Message-ID: <c04323d5-6dcd-4391-81bb-94ee580ae98f@arm.com>
Date: Tue, 15 Oct 2024 12:16:12 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Pingfan Liu <piliu@...hat.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 15/10/2024 04:04, Pingfan Liu wrote:
> 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.

Yes agreed. I spent some time thinking about how much of this could be automated
(i.e. with Cochinelle or otherwise), but concluded that it's very difficult. As
a result, all of the patches in this series are manually created.

> 
>> 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.

Yes, hope so!

> 
> Thanks,
> 
> Pingfan
> 

Powered by blists - more mailing lists