Message-ID: <CAF+s44QbdPBN-8EcPiWiZgYgZY4v8RK-wA0VEaVXbfnc9_HQ9Q@mail.gmail.com>
Date: Tue, 15 Oct 2024 11:04:21 +0800
From: Pingfan Liu <piliu@...hat.com>
To: Ryan Roberts <ryan.roberts@....com>
Cc: "David S. Miller" <davem@...emloft.net>, 
	"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>, 
	Andreas Larsson <andreas@...sler.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Anshuman Khandual <anshuman.khandual@....com>, Anton Ivanov <anton.ivanov@...bridgegreys.com>, 
	Ard Biesheuvel <ardb@...nel.org>, Arnd Bergmann <arnd@...db.de>, Borislav Petkov <bp@...en8.de>, 
	Catalin Marinas <catalin.marinas@....com>, Chris Zankel <chris@...kel.net>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, David Hildenbrand <david@...hat.com>, 
	Dinh Nguyen <dinguyen@...nel.org>, Geert Uytterhoeven <geert@...ux-m68k.org>, 
	Greg Marsden <greg.marsden@...cle.com>, Helge Deller <deller@....de>, 
	Huacai Chen <chenhuacai@...nel.org>, Ingo Molnar <mingo@...hat.com>, 
	Ivan Ivanov <ivan.ivanov@...e.com>, Johannes Berg <johannes@...solutions.net>, 
	John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>, Jonas Bonn <jonas@...thpole.se>, 
	Kalesh Singh <kaleshsingh@...gle.com>, Marc Zyngier <maz@...nel.org>, 
	Mark Rutland <mark.rutland@....com>, Matthias Brugger <mbrugger@...e.com>, 
	Max Filippov <jcmvbkbc@...il.com>, Miroslav Benes <mbenes@...e.cz>, Rich Felker <dalias@...c.org>, 
	Richard Weinberger <richard@....at>, Stafford Horne <shorne@...il.com>, 
	Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>, 
	Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Thomas Gleixner <tglx@...utronix.de>, 
	Will Deacon <will@...nel.org>, Yoshinori Sato <ysato@...rs.sourceforge.jp>, x86@...nel.org, 
	linux-alpha@...r.kernel.org, linux-arch@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-csky@...r.kernel.org, 
	linux-hexagon@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org, 
	linux-mm@...ck.org, linux-openrisc@...r.kernel.org, 
	linux-parisc@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	linux-s390@...r.kernel.org, linux-sh@...r.kernel.org, 
	linux-snps-arc@...ts.infradead.org, linux-um@...ts.infradead.org, 
	linuxppc-dev@...ts.ozlabs.org, loongarch@...ts.linux.dev, 
	sparclinux@...r.kernel.org
Subject: Re: [RFC PATCH v1 01/57] mm: Add macros ahead of supporting boot-time
 page size selection

On Mon, Oct 14, 2024 at 10:07 PM Ryan Roberts <ryan.roberts@....com> wrote:
>
> On 14/10/2024 14:54, Pingfan Liu wrote:
> > Hello Ryan,
> >
> > On Mon, Oct 14, 2024 at 11:58:08AM +0100, Ryan Roberts wrote:
> >> arm64 can support multiple base page sizes. Instead of selecting a page
> >> size at compile time, as is done today, we will make it possible to
> >> select the desired page size on the command line.
> >>
> >> In this case PAGE_SHIFT and it's derivatives, PAGE_SIZE and PAGE_MASK
> >> (as well as a number of other macros related to or derived from
> >> PAGE_SHIFT, but I'm not worrying about those yet), are no longer
> >> compile-time constants. So the code base needs to cope with that.
> >>
> >> As a first step, introduce MIN and MAX variants of these macros, which
> >> express the range of possible page sizes. These are always compile-time
> >> constants and can be used in many places where PAGE_[SHIFT|SIZE|MASK]
> >> were previously used where a compile-time constant is required.
> >> (Subsequent patches will do that conversion work). When the arch/build
> >> doesn't support boot-time page size selection, the MIN and MAX variants
> >> are equal and everything resolves as it did previously.
> >>
> >
> > MIN and MAX appear to construct a boundary, but it may be not enough.
> > Please see the following comment inline.
> >
> >> Additionally, introduce DEFINE_GLOBAL_PAGE_SIZE_VAR[_CONST]() which wrap
> >> global variable defintions so that for boot-time page size selection
> >> builds, the variable being wrapped is initialized at boot-time, instead
> >> of compile-time. This is done by defining a function to do the
> >> assignment, which has the "constructor" attribute. Constructor is
> >> preferred over initcall, because when compiling a module, the module is
> >> limited to a single initcall but constructors are unlimited. For
> >> built-in code, constructors are now called earlier to guarrantee that
> >> the variables are initialized by the time they are used. Any arch that
> >> wants to enable boot-time page size selection will need to select
> >> CONFIG_CONSTRUCTORS.
> >>
> >> These new macros need to be available anywhere PAGE_SHIFT and friends
> >> are available. Those are defined via asm/page.h (although some arches
> >> have a sub-include that defines them). Unfortunately there is no
> >> reliable asm-generic header we can easily piggy-back on, so let's define
> >> a new one, pgtable-geometry.h, which we include near where each arch
> >> defines PAGE_SHIFT. Ugh.
> >>
> >> -------
> >>
> >> Most of the problems that need to be solved over the next few patches
> >> fall into these broad categories, which are all solved with the help of
> >> these new macros:
> >>
> >> 1. Assignment of values derived from PAGE_SIZE in global variables
> >>
> >>   For boot-time page size builds, we must defer the initialization of
> >>   these variables until boot-time, when the page size is known. See
> >>   DEFINE_GLOBAL_PAGE_SIZE_VAR[_CONST]() as described above.
> >>
> >> 2. Define static storage in units related to PAGE_SIZE
> >>
> >>   This static storage will be defined according to PAGE_SIZE_MAX.
> >>
> >> 3. Define size of struct so that it is related to PAGE_SIZE
> >>
> >>   The struct often contains an array that is sized to fill the page. In
> >>   this case, use a flexible array with dynamic allocation. In other
> >>   cases, the struct fits exactly over a page, which is a header (e.g.
> >>   swap file header). In this case, remove the padding, and manually
> >>   determine the struct pointer within the page.
> >>
> >
> > About two years ago, I tried to do similar thing in your series, but ran
> > into problem at this point, or maybe not exactly as the point you list
> > here. I consider this as the most challenged part.
> >
> > The scenario is
> > struct X {
> >       a[size_a];
> >       b[size_b];
> >       c;
> > };
> >
> > Where size_a = f(PAGE_SHIFT), size_b=g(PAGE_SHIFT). One of f() and g()
> > is proportional to PAGE_SHIFT, the other is inversely proportional.
> >
> > How can you fix the reference of X.a and X.b?
>
> If you need to allocate static memory, then in this scenario, assuming f() is
> proportional and g() is inversely-proportional, then I guess you need
> size_a=f(PAGE_SIZE_MAX) and size_b=g(PAGE_SIZE_MIN). Or if you can allocate the

My point is that such stuff can not be handled by scripts
automatically and needs manual intervention.

> memory dynamically, then make a and b pointers to dynamically allocated buffers.
>

This seems a better way out.

> Is there a specific place in the source where this pattern is used today? It
> might be easier to discuss in the context of the code if so.
>

No such code at hand. Just throw out the potential issue and be
curious about it which frustrates me.
I hope people can reach an agreement on it and turn this useful series
into reality.

Thanks,

Pingfan

Powered by blists - more mailing lists