From: Ryan Roberts <ryan.roberts@arm.com>
To: Pingfan Liu <piliu@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>,
"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
Andreas Larsson <andreas@gaisler.com>,
Andrew Morton <akpm@linux-foundation.org>,
Anshuman Khandual <anshuman.khandual@arm.com>,
Anton Ivanov <anton.ivanov@cambridgegreys.com>,
Ard Biesheuvel <ardb@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Borislav Petkov <bp@alien8.de>,
Catalin Marinas <catalin.marinas@arm.com>,
Chris Zankel <chris@zankel.net>,
Dave Hansen <dave.hansen@linux.intel.com>,
David Hildenbrand <david@redhat.com>,
Dinh Nguyen <dinguyen@kernel.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Greg Marsden <greg.marsden@oracle.com>,
Helge Deller <deller@gmx.de>, Huacai Chen <chenhuacai@kernel.org>,
Ingo Molnar <mingo@redhat.com>,
Ivan Ivanov <ivan.ivanov@suse.com>,
Johannes Berg <johannes@sipsolutions.net>,
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
Jonas Bonn <jonas@southpole.se>,
Kalesh Singh <kaleshsingh@google.com>,
Marc Zyngier <maz@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Matthias Brugger <mbrugger@suse.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Miroslav Benes <mbenes@suse.cz>, Rich Felker <dalias@libc.org>,
Richard Weinberger <richard@nod.at>,
Stafford Horne <shorne@gmail.com>,
Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Thomas Gleixner <tglx@linutronix.de>,
Will Deacon <will@kernel.org>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
x86@kernel.org, linux-alpha@vger.kernel.org,
linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-m68k@lists.linux-m68k.org,
linux-mips@vger.kernel.org, linux-mm@kvack.org,
linux-openrisc@vger.kernel.org, linux-parisc@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org,
linux-sh@vger.kernel.org, linux-snps-arc@lists.infradead.org,
linux-um@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
loongarch@lists.linux.dev, sparclinux@vger.kernel.org
Subject: Re: [RFC PATCH v1 01/57] mm: Add macros ahead of supporting boot-time page size selection
Date: Tue, 15 Oct 2024 12:16:12 +0100 [thread overview]
Message-ID: <c04323d5-6dcd-4391-81bb-94ee580ae98f@arm.com> (raw)
In-Reply-To: <CAF+s44QbdPBN-8EcPiWiZgYgZY4v8RK-wA0VEaVXbfnc9_HQ9Q@mail.gmail.com>
On 15/10/2024 04:04, Pingfan Liu wrote:
> On Mon, Oct 14, 2024 at 10:07 PM Ryan Roberts <ryan.roberts@arm.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
>
next prev parent reply other threads:[~2024-10-15 11:16 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20241014105514.3206191-1-ryan.roberts@arm.com>
2024-10-14 10:58 ` [RFC PATCH v1 01/57] mm: Add macros ahead of supporting boot-time page size selection Ryan Roberts
2024-10-14 13:54 ` Pingfan Liu
2024-10-14 14:07 ` Ryan Roberts
2024-10-15 3:04 ` Pingfan Liu
2024-10-15 11:16 ` Ryan Roberts [this message]
2024-10-16 14:36 ` Ryan Roberts
2024-10-30 8:45 ` Ryan Roberts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c04323d5-6dcd-4391-81bb-94ee580ae98f@arm.com \
--to=ryan.roberts@arm.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=andreas@gaisler.com \
--cc=anshuman.khandual@arm.com \
--cc=anton.ivanov@cambridgegreys.com \
--cc=ardb@kernel.org \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=chenhuacai@kernel.org \
--cc=chris@zankel.net \
--cc=dalias@libc.org \
--cc=dave.hansen@linux.intel.com \
--cc=davem@davemloft.net \
--cc=david@redhat.com \
--cc=deller@gmx.de \
--cc=dinguyen@kernel.org \
--cc=geert@linux-m68k.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=greg.marsden@oracle.com \
--cc=ivan.ivanov@suse.com \
--cc=jcmvbkbc@gmail.com \
--cc=johannes@sipsolutions.net \
--cc=jonas@southpole.se \
--cc=kaleshsingh@google.com \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-csky@vger.kernel.org \
--cc=linux-hexagon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-openrisc@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux-snps-arc@lists.infradead.org \
--cc=linux-um@lists.infradead.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=loongarch@lists.linux.dev \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=mbenes@suse.cz \
--cc=mbrugger@suse.com \
--cc=mingo@redhat.com \
--cc=piliu@redhat.com \
--cc=richard@nod.at \
--cc=shorne@gmail.com \
--cc=sparclinux@vger.kernel.org \
--cc=stefan.kristiansson@saunalahti.fi \
--cc=tglx@linutronix.de \
--cc=tsbogend@alpha.franken.de \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).