From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f71.google.com (mail-pg0-f71.google.com [74.125.83.71]) by kanga.kvack.org (Postfix) with ESMTP id E594F6B0271 for ; Tue, 5 Dec 2017 14:54:04 -0500 (EST) Received: by mail-pg0-f71.google.com with SMTP id m17so914489pgu.19 for ; Tue, 05 Dec 2017 11:54:04 -0800 (PST) Received: from bombadil.infradead.org (bombadil.infradead.org. [65.50.211.133]) by mx.google.com with ESMTPS id k4si547871pls.297.2017.12.05.11.54.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Dec 2017 11:54:03 -0800 (PST) Date: Tue, 5 Dec 2017 11:54:00 -0800 From: Matthew Wilcox Subject: Re: [PATCH] dax: fix potential overflow on 32bit machine Message-ID: <20171205195400.GC26021@bombadil.infradead.org> References: <20171205033210.38338-1-yi.zhang@huawei.com> <20171205052407.GA20757@bombadil.infradead.org> <20171205170709.GA21010@linux.intel.com> <20171205173713.GA26021@bombadil.infradead.org> <20171205191928.GB21010@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171205191928.GB21010@linux.intel.com> Sender: owner-linux-mm@kvack.org List-ID: To: Ross Zwisler , "zhangyi (F)" , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, mawilcox@microsoft.com, viro@zeniv.linux.org.uk, miaoxie@huawei.com On Tue, Dec 05, 2017 at 12:19:28PM -0700, Ross Zwisler wrote: > On Tue, Dec 05, 2017 at 09:37:13AM -0800, Matthew Wilcox wrote: > > On Tue, Dec 05, 2017 at 10:07:09AM -0700, Ross Zwisler wrote: > > > > /* The 'colour' (ie low bits) within a PMD of a page offset. */ > > > > #define PG_PMD_COLOUR ((PMD_SIZE >> PAGE_SHIFT) - 1) > > > > +#define PG_PMD_NR (PMD_SIZE >> PAGE_SHIFT) > > > > > > I wonder if it's confusing that PG_PMD_COLOUR is a mask, but PG_PMD_NR is a > > > count? Would "PAGES_PER_PMD" be clearer, in the spirit of > > > PTRS_PER_{PGD,PMD,PTE}? > > > > Maybe. I don't think that 'NR' can ever be confused with a mask. > > I went with PG_PMD_NR because I didn't want to use HPAGE_PMD_NR, but > > in retrospect I just needed to go to sleep and leave thinking about > > hard problems like naming things for the morning. I decided to call it > > 'colour' rather than 'mask' originally because I got really confused with > > PMD_MASK masking off the low bits. If you ask 'What colour is this page > > within the PMD', you know you're talking about the low bits. > > > > I actually had cause to define PMD_ORDER in a separate unrelated patch > > I was working on this morning. How does this set of definitions grab you? > > > > #define PMD_ORDER (PMD_SHIFT - PAGE_SHIFT) > > #define PMD_PAGES (1UL << PMD_ORDER) > > #define PMD_PAGE_COLOUR (PMD_PAGES - 1) > > > > and maybe put them in linux/mm.h so everybody can see them? > > Yep, I personally like these better, and putting them in a global header seems > like the right way to go. Ugh. ARM, MIPS and PARISC all define a rather interesting PMD_ORDER. I'm going to have to rename them first. > > > Also, can we use the same define both in fs/dax.c and in mm/truncate.c, > > > instead of the latter using HPAGE_PMD_NR? > > > > I'm OK with the latter using HPAGE_PMD_NR because it's explicitly "is > > this a huge page?" But I'd kind of like to get rid of a lot of the HPAGE_* > > definitions, so > > I would also like to get rid of them if possible, but quick grep makes me > think that unfortunately they may not be entirely equivalent to other defines > we have? > > i.e: > > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 13 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 14 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 15 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 16 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 17 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 18 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 19 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 20 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 21 > arch/metag/include/asm/page.h:# define HPAGE_SHIFT 22 > > this arch has no PMD_SHIFT definition... > > I'm not really familiar with the HPAGE defines, though, so maybe it's not as > complex as it seems. I think it's more complex than it seems. Some of the HPAGE definitions (like the ones you've found) are for hugetlbfs hugepages which are a bit different from the transparent hugepages. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org