Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Tue, 2024-02-27 at 18:16 +, Christophe Leroy wrote: > > > Why doing a full init of the struct when all fields are re- > > > written a few > > > lines after ? > > > > It's a nice change for robustness and makes future changes easier. > > It's > > not actually wasteful since the compiler will throw away all > > redundant > > stores. > > Well, I tend to dislike default init at declaration because it often > hides missed real init. When a field is not initialized GCC should > emit > a Warning, at least when built with W=2 which sets > -Wmissing-field-initializers ? Sorry, I'm not following where you are going with this. There aren't any struct vm_unmapped_area_info users that use initializers today, so that warning won't apply in this case. Meanwhile, designated style struct initialization (which would zero new members) is very common, as well as not get anything checked by that warning. Anything with this many members is probably going to use the designated style. If we are optimizing to avoid bugs, the way this struct is used today is not great. It is essentially being used as an argument passer. Normally when a function signature changes, but a caller is missed, of course the compiler will notice loudly. But not here. So I think probably zero initializing it is safer than being setup to pass garbage. I'm trying to figure out what to do here. If I changed it so that just powerpc set the new field manually, then the convention across the kernel would be for everything to be default zero, and future other new parameters could have a greater chance of turning into garbage on powerpc. Since it could be easy to miss that powerpc was special. Would you prefer it? Or maybe I could try a new vm_unmapped_area() that takes the extra argument separately? The old callers could call the old function and not need any arch updates. It all seems strange though, because automatic zero initializing struct members is so common in the kernel. But it also wouldn't add the cleanup Kees was pointing out. Hmm. Any preference? Or maybe am I missing your point and talking nonsense?
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
Le 27/02/2024 à 19:07, Kees Cook a écrit : > On Tue, Feb 27, 2024 at 07:02:59AM +, Christophe Leroy wrote: >> >> >> Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : >>> Future changes will need to add a field to struct vm_unmapped_area_info. >>> This would cause trouble for any archs that don't initialize the >>> struct. Currently every user sets each field, so if new fields are >>> added, the core code parsing the struct will see garbage in the new >>> field. >>> >>> It could be possible to initialize the new field for each arch to 0, but >>> instead simply inialize the field with a C99 struct inializing syntax. >> >> Why doing a full init of the struct when all fields are re-written a few >> lines after ? > > It's a nice change for robustness and makes future changes easier. It's > not actually wasteful since the compiler will throw away all redundant > stores. Well, I tend to dislike default init at declaration because it often hides missed real init. When a field is not initialized GCC should emit a Warning, at least when built with W=2 which sets -Wmissing-field-initializers ? > >> If I take the exemple of powerpc function slice_find_area_bottomup(): >> >> struct vm_unmapped_area_info info; >> >> info.flags = 0; >> info.length = len; >> info.align_mask = PAGE_MASK & ((1ul << pshift) - 1); >> info.align_offset = 0; > > But one cleanup that is possible from explicitly zero-initializing the > whole structure would be dropping all the individual "= 0" assignments. > :) > Sure if we decide to go that direction all those 0 assignments void.
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Tue, Feb 27, 2024 at 07:02:59AM +, Christophe Leroy wrote: > > > Le 26/02/2024 à 20:09, Rick Edgecombe a écrit : > > Future changes will need to add a field to struct vm_unmapped_area_info. > > This would cause trouble for any archs that don't initialize the > > struct. Currently every user sets each field, so if new fields are > > added, the core code parsing the struct will see garbage in the new > > field. > > > > It could be possible to initialize the new field for each arch to 0, but > > instead simply inialize the field with a C99 struct inializing syntax. > > Why doing a full init of the struct when all fields are re-written a few > lines after ? It's a nice change for robustness and makes future changes easier. It's not actually wasteful since the compiler will throw away all redundant stores. > If I take the exemple of powerpc function slice_find_area_bottomup(): > > struct vm_unmapped_area_info info; > > info.flags = 0; > info.length = len; > info.align_mask = PAGE_MASK & ((1ul << pshift) - 1); > info.align_offset = 0; But one cleanup that is possible from explicitly zero-initializing the whole structure would be dropping all the individual "= 0" assignments. :) -- Kees Cook
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
On Tue, Feb 27, 2024, at 16:44, Christophe Leroy wrote: > Le 27/02/2024 à 16:40, Arnd Bergmann a écrit : >> On Mon, Feb 26, 2024, at 17:55, Samuel Holland wrote: > > > For 256K pages, powerpc has the following help. I think you should have > it too: > > The kernel will only be able to run applications that have been > compiled with '-zmax-page-size' set to 256K (the default is 64K) using > binutils later than 2.17.50.0.3, or by patching the ELF_MAXPAGESIZE > definition from 0x1 to 0x4 in older versions. I don't think we need to mention pre-2.18 binutils any more, but the rest seems useful, changed the text now to config PAGE_SIZE_256KB bool "256KiB pages" depends on HAVE_PAGE_SIZE_256KB help 256KiB pages have little practical value due to their extreme memory usage. The kernel will only be able to run applications that have been compiled with '-zmax-page-size' set to 256KiB (the default is 64KiB or 4KiB on most architectures). Arnd
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
Le 27/02/2024 à 16:40, Arnd Bergmann a écrit : > On Mon, Feb 26, 2024, at 17:55, Samuel Holland wrote: >> On 2024-02-26 10:14 AM, Arnd Bergmann wrote: >>> >>> +config HAVE_PAGE_SIZE_4KB >>> + bool >>> + >>> +config HAVE_PAGE_SIZE_8KB >>> + bool >>> + >>> +config HAVE_PAGE_SIZE_16KB >>> + bool >>> + >>> +config HAVE_PAGE_SIZE_32KB >>> + bool >>> + >>> +config HAVE_PAGE_SIZE_64KB >>> + bool >>> + >>> +config HAVE_PAGE_SIZE_256KB >>> + bool >>> + >>> +choice >>> + prompt "MMU page size" >> >> Should this have some generic help text (at least a warning about >> compatibility)? > > Good point. I've added some of this now, based on the mips > text with some generalizations for other architectures: > > config PAGE_SIZE_4KB > bool "4KiB pages" > depends on HAVE_PAGE_SIZE_4KB > help >This option select the standard 4KiB Linux page size and the only >available option on many architectures. Using 4KiB page size will >minimize memory consumption and is therefore recommended for low >memory systems. >Some software that is written for x86 systems makes incorrect >assumptions about the page size and only runs on 4KiB pages. > > config PAGE_SIZE_8KB > bool "8KiB pages" > depends on HAVE_PAGE_SIZE_8KB > help >This option is the only supported page size on a few older >processors, and can be slightly faster than 4KiB pages. > > config PAGE_SIZE_16KB > bool "16KiB pages" > depends on HAVE_PAGE_SIZE_16KB > help >This option is usually a good compromise between memory >consumption and performance for typical desktop and server >workloads, often saving a level of page table lookups compared >to 4KB pages as well as reducing TLB pressure and overhead of >per-page operations in the kernel at the expense of a larger >page cache. > > config PAGE_SIZE_32KB > bool "32KiB pages" > depends on HAVE_PAGE_SIZE_32KB >Using 32KiB page size will result in slightly higher performance >kernel at the price of higher memory consumption compared to >16KiB pages. This option is available only on cnMIPS cores. >Note that you will need a suitable Linux distribution to >support this. > > config PAGE_SIZE_64KB > bool "64KiB pages" > depends on HAVE_PAGE_SIZE_64KB >Using 64KiB page size will result in slightly higher performance >kernel at the price of much higher memory consumption compared to >4KiB or 16KiB pages. >This is not suitable for general-purpose workloads but the >better performance may be worth the cost for certain types of >supercomputing or database applications that work mostly with >large in-memory data rather than small files. > > config PAGE_SIZE_256KB > bool "256KiB pages" > depends on HAVE_PAGE_SIZE_256KB > help >256KB pages have little practical value due to their extreme >memory usage. For 256K pages, powerpc has the following help. I think you should have it too: The kernel will only be able to run applications that have been compiled with '-zmax-page-size' set to 256K (the default is 64K) using binutils later than 2.17.50.0.3, or by patching the ELF_MAXPAGESIZE definition from 0x1 to 0x4 in older versions.
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
On Tue, Feb 27, 2024, at 09:45, Geert Uytterhoeven wrote:
>
>> +config PAGE_SIZE_4KB
>> + bool "4KB pages"
>
> Now you got rid of the 4000-byte ("4kB") pages and friends, please
> do not replace these by Kelvin-bytes, and use the official binary
> prefixes => "4 KiB".
>
Done, thanks.
Arnd
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
On Mon, Feb 26, 2024, at 20:02, Christophe Leroy wrote: > Le 26/02/2024 à 17:14, Arnd Bergmann a écrit : >> From: Arnd Bergmann > > That's a nice re-factor. > > The only drawback I see is that we are loosing several interesting > arch-specific comments/help text. Don't know if there could be an easy > way to keep them. This is what I have now, trying to write it as generic as possible while still giving useful advice: config PAGE_SIZE_4KB bool "4KiB pages" depends on HAVE_PAGE_SIZE_4KB help This option select the standard 4KiB Linux page size and the only available option on many architectures. Using 4KiB page size will minimize memory consumption and is therefore recommended for low memory systems. Some software that is written for x86 systems makes incorrect assumptions about the page size and only runs on 4KiB pages. config PAGE_SIZE_8KB bool "8KiB pages" depends on HAVE_PAGE_SIZE_8KB help This option is the only supported page size on a few older processors, and can be slightly faster than 4KiB pages. config PAGE_SIZE_16KB bool "16KiB pages" depends on HAVE_PAGE_SIZE_16KB help This option is usually a good compromise between memory consumption and performance for typical desktop and server workloads, often saving a level of page table lookups compared to 4KB pages as well as reducing TLB pressure and overhead of per-page operations in the kernel at the expense of a larger page cache. config PAGE_SIZE_32KB bool "32KiB pages" depends on HAVE_PAGE_SIZE_32KB Using 32KiB page size will result in slightly higher performance kernel at the price of higher memory consumption compared to 16KiB pages. This option is available only on cnMIPS cores. Note that you will need a suitable Linux distribution to support this. config PAGE_SIZE_64KB bool "64KiB pages" depends on HAVE_PAGE_SIZE_64KB Using 64KiB page size will result in slightly higher performance kernel at the price of much higher memory consumption compared to 4KiB or 16KiB pages. This is not suitable for general-purpose workloads but the better performance may be worth the cost for certain types of supercomputing or database applications that work mostly with large in-memory data rather than small files. config PAGE_SIZE_256KB bool "256KiB pages" depends on HAVE_PAGE_SIZE_256KB help 256KB pages have little practical value due to their extreme memory usage. Let me know if you think some of this should be adapted further. >> >> +#define PAGE_SHIFT CONFIG_PAGE_SHIFT >> #define PAGE_SIZE (1UL << PAGE_SHIFT) >> #define PAGE_MASK (~((1 << PAGE_SHIFT) - 1)) >> > > Could we move PAGE_SIZE and PAGE_MASK in a generic/core header instead > of having it duplicated for each arch ? Yes, but I'm leaving this for a follow-up series, since I had to stop somewhere and there is always room for cleanup up headers further ;-) Arnd
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
On Mon, Feb 26, 2024, at 17:55, Samuel Holland wrote: > On 2024-02-26 10:14 AM, Arnd Bergmann wrote: >> >> +config HAVE_PAGE_SIZE_4KB >> +bool >> + >> +config HAVE_PAGE_SIZE_8KB >> +bool >> + >> +config HAVE_PAGE_SIZE_16KB >> +bool >> + >> +config HAVE_PAGE_SIZE_32KB >> +bool >> + >> +config HAVE_PAGE_SIZE_64KB >> +bool >> + >> +config HAVE_PAGE_SIZE_256KB >> +bool >> + >> +choice >> +prompt "MMU page size" > > Should this have some generic help text (at least a warning about > compatibility)? Good point. I've added some of this now, based on the mips text with some generalizations for other architectures: config PAGE_SIZE_4KB bool "4KiB pages" depends on HAVE_PAGE_SIZE_4KB help This option select the standard 4KiB Linux page size and the only available option on many architectures. Using 4KiB page size will minimize memory consumption and is therefore recommended for low memory systems. Some software that is written for x86 systems makes incorrect assumptions about the page size and only runs on 4KiB pages. config PAGE_SIZE_8KB bool "8KiB pages" depends on HAVE_PAGE_SIZE_8KB help This option is the only supported page size on a few older processors, and can be slightly faster than 4KiB pages. config PAGE_SIZE_16KB bool "16KiB pages" depends on HAVE_PAGE_SIZE_16KB help This option is usually a good compromise between memory consumption and performance for typical desktop and server workloads, often saving a level of page table lookups compared to 4KB pages as well as reducing TLB pressure and overhead of per-page operations in the kernel at the expense of a larger page cache. config PAGE_SIZE_32KB bool "32KiB pages" depends on HAVE_PAGE_SIZE_32KB Using 32KiB page size will result in slightly higher performance kernel at the price of higher memory consumption compared to 16KiB pages. This option is available only on cnMIPS cores. Note that you will need a suitable Linux distribution to support this. config PAGE_SIZE_64KB bool "64KiB pages" depends on HAVE_PAGE_SIZE_64KB Using 64KiB page size will result in slightly higher performance kernel at the price of much higher memory consumption compared to 4KiB or 16KiB pages. This is not suitable for general-purpose workloads but the better performance may be worth the cost for certain types of supercomputing or database applications that work mostly with large in-memory data rather than small files. config PAGE_SIZE_256KB bool "256KiB pages" depends on HAVE_PAGE_SIZE_256KB help 256KB pages have little practical value due to their extreme memory usage. >> diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig >> index a880ee067d2e..aac46ee1a000 100644 >> --- a/arch/hexagon/Kconfig >> +++ b/arch/hexagon/Kconfig >> @@ -8,6 +8,11 @@ config HEXAGON >> select ARCH_HAS_SYNC_DMA_FOR_DEVICE >> select ARCH_NO_PREEMPT >> select DMA_GLOBAL_POOL >> +select FRAME_POINTER > > Looks like a paste error. > Fixed, thanks! I think that happened during a rebase. >> #ifdef CONFIG_PAGE_SIZE_1MB >> -#define PAGE_SHIFT 20 >> #define HEXAGON_L1_PTE_SIZE __HVM_PDE_S_1MB >> #endif > > The corresponding Kconfig option does not exist (and did not exist before this > patch). Yes, I noticed that as well. It's clearly harmless. Arnd
Re: [PATCH v2 5/9] mm: Initialize struct vm_unmapped_area_info
On Tue, 2024-02-27 at 07:02 +, Christophe Leroy wrote: > > It could be possible to initialize the new field for each arch to > > 0, but > > instead simply inialize the field with a C99 struct inializing > > syntax. > > Why doing a full init of the struct when all fields are re-written a > few > lines after ? > > If I take the exemple of powerpc function slice_find_area_bottomup(): > > struct vm_unmapped_area_info info; > > info.flags = 0; > info.length = len; > info.align_mask = PAGE_MASK & ((1ul << pshift) - 1); > info.align_offset = 0; > > For me it looks better to just add: > > info.new_field = 0; /* or whatever value it needs to have */ Hi, Thanks for taking a look. Yes, I guess that should have some justification. I was thinking of two reasons: 1. No future additions of optional parameters would need to make tree wide changes like this. 2. The change is easier to review and get correct because the necessary context is within a single line. For example, in that function some of members are set within a while loop. The place you pointed seems to be the correct one, but a diff that had the new field set after: info.high_limit = addr; ...would look correct too, but not be. What is the concern with C99 initialization? FWIW, the full series also removes an indirect branch, and probably is a net win for performance in this path.
Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures
On Tue, Feb 27, 2024, at 12:12, Geert Uytterhoeven wrote:
> On Tue, Feb 27, 2024 at 11:59 AM Arnd Bergmann wrote:
>> On Tue, Feb 27, 2024, at 09:54, Geert Uytterhoeven wrote:
>> I was a bit unsure about how to best do this since there
>> is not really a need for a fixed page size on nommu kernels,
>> whereas the three MMU configs clearly tie the page size to
>> the MMU rather than the platform.
>>
>> There should be no reason for coldfire to have a different
>> page size from dragonball if neither of them actually uses
>> hardware pages, so one of them could be changed later.
>
> Indeed, in theory, PAGE_SIZE doesn't matter for nommu, but the concept
> of pages is used all over the place in Linux.
>
> I'm mostly worried about some Coldfire code relying on the actual value
> of PAGE_SIZE in some other context. e.g. for configuring non-cacheable
> regions.
Right, any change here would have to be carefully tested. I would
expect that a 4K page size would reduce memory consumption even on
NOMMU systems that should have the same tradeoffs for representing
files in the page cache and in mem_map[].
> And does this impact running nommu binaries on a system with MMU?
> I.e. if nommu binaries were built with a 4 KiB PAGE_SIZE, do they
> still run on MMU systems with an 8 KiB PAGE_SIZE (coldfire and sun3),
> or are there some subtleties to take into account?
As far as I understand, binaries have to be built and linked for
the largest page size they can run on, so running them on a kernel
with smaller page size usually works.
One notable exception is sys_mmap2(), which on most architectures
takes units of 4KiB but on m68k is actually written to take
PAGE_SIZE units. As Al pointed out in f8b7256096a2 ("Unify
sys_mmap*"), it has always been wrong on sun3, presumably
because users of that predate modern glibc. Running coldfire
nommu binaries on coldfire mmu kernels would run into the same
bug if either of them changes PAGE_SIZE. If you can run
coldfire nommu binaries on classic m68k, that is already
broken in the same way.
Arnd
Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures
On Mon, Feb 26, 2024 at 05:14:13PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > Most architectures only support a single hardcoded page size. In order > to ensure that each one of these sets the corresponding Kconfig symbols, > change over the PAGE_SHIFT definition to the common one and allow > only the hardware page size to be selected. > > Signed-off-by: Arnd Bergmann > --- ... > arch/s390/Kconfig | 1 + > arch/s390/include/asm/page.h | 2 +- ... > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index fe565f3a3a91..b61c74c10050 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -199,6 +199,7 @@ config S390 > select HAVE_MOD_ARCH_SPECIFIC > select HAVE_NMI > select HAVE_NOP_MCOUNT > + select HAVE_PAGE_SIZE_4KB > select HAVE_PCI > select HAVE_PERF_EVENTS > select HAVE_PERF_REGS > diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h > index 73b9c3bf377f..ded9548d11d9 100644 > --- a/arch/s390/include/asm/page.h > +++ b/arch/s390/include/asm/page.h > @@ -11,7 +11,7 @@ > #include > #include > > -#define _PAGE_SHIFT 12 > +#define _PAGE_SHIFT CONFIG_PAGE_SHIFT Acked-by: Heiko Carstens
Re: [PATCH 2/4] arch: simplify architecture specific page size configuration
On 2/26/24 17:14, Arnd Bergmann wrote: From: Arnd Bergmann arc, arm64, parisc and powerpc all have their own Kconfig symbols in place of the common CONFIG_PAGE_SIZE_4KB symbols. Change these so the common symbols are the ones that are actually used, while leaving the arhcitecture specific ones as the user visible place for configuring it, to avoid breaking user configs. Signed-off-by: Arnd Bergmann --- arch/arc/Kconfig | 3 +++ arch/arc/include/uapi/asm/page.h | 6 ++ arch/arm64/Kconfig| 29 + arch/arm64/include/asm/page-def.h | 2 +- arch/parisc/Kconfig | 3 +++ arch/parisc/include/asm/page.h| 10 +- Acked-by: Helge Deller # parisc Thanks for the cleanups! Helge
Re: [PATCH 4/4] vdso: avoid including asm/page.h
On Mon, Feb 26, 2024 at 05:14:14PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The recent change to the vdso_data_store broke building compat VDSO
> on at least arm64 because it includes headers outside of the include/vdso/
> namespace:
>
> In file included from arch/arm64/include/asm/lse.h:5,
> from arch/arm64/include/asm/cmpxchg.h:14,
> from arch/arm64/include/asm/atomic.h:16,
> from include/linux/atomic.h:7,
> from include/asm-generic/bitops/atomic.h:5,
> from arch/arm64/include/asm/bitops.h:25,
> from include/linux/bitops.h:68,
> from arch/arm64/include/asm/memory.h:209,
> from arch/arm64/include/asm/page.h:46,
> from include/vdso/datapage.h:22,
> from lib/vdso/gettimeofday.c:5,
> from :
> arch/arm64/include/asm/atomic_ll_sc.h:298:9: error: unknown type name 'u128'
> 298 | u128 full;
>
> Use an open-coded page size calculation based on the new CONFIG_PAGE_SHIFT
> Kconfig symbol instead.
>
> Reported-by: Linux Kernel Functional Testing
> Fixes: a0d2fcd62ac2 ("vdso/ARM: Make union vdso_data_store available for all
> architectures")
> Link:
> https://lore.kernel.org/lkml/ca+g9fytrxxm_ko9fnpz3xarxhv7ud_yqp-teupqrnrhu+_0...@mail.gmail.com/
> Signed-off-by: Arnd Bergmann
Acked-by: Catalin Marinas
Re: [PATCH 2/4] arch: simplify architecture specific page size configuration
On Mon, Feb 26, 2024 at 05:14:12PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > arc, arm64, parisc and powerpc all have their own Kconfig symbols > in place of the common CONFIG_PAGE_SIZE_4KB symbols. Change these > so the common symbols are the ones that are actually used, while > leaving the arhcitecture specific ones as the user visible > place for configuring it, to avoid breaking user configs. > > Signed-off-by: Arnd Bergmann For arm64: Acked-by: Catalin Marinas
Re: [PATCH 4/4] vdso: avoid including asm/page.h
Christophe Leroy writes: > Le 26/02/2024 à 17:14, Arnd Bergmann a écrit : >> From: Arnd Bergmann >> >> The recent change to the vdso_data_store broke building compat VDSO >> on at least arm64 because it includes headers outside of the include/vdso/ >> namespace: > > I understand that powerpc64 also has an issue, see > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20231221120410.2226678-1-...@ellerman.id.au/ Yeah, and that patch would silently conflict with this series, which is not ideal. I could delay merging my patch above until after this series goes in, mine only fixes a fairly obscure build warning. cheers
Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures
Hi Arnd,
CC Greg
On Tue, Feb 27, 2024 at 11:59 AM Arnd Bergmann wrote:
> On Tue, Feb 27, 2024, at 09:54, Geert Uytterhoeven wrote:
> >> diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu
> >> index 9dcf245c9cbf..c777a129768a 100644
> >> --- a/arch/m68k/Kconfig.cpu
> >> +++ b/arch/m68k/Kconfig.cpu
> >> @@ -30,6 +30,7 @@ config COLDFIRE
> >> select GENERIC_CSUM
> >> select GPIOLIB
> >> select HAVE_LEGACY_CLK
> >> + select HAVE_PAGE_SIZE_8KB if !MMU
> >
> > if you would drop the !MMU-dependency here.
> >
> >>
> >> endchoice
> >>
> >> @@ -45,6 +46,7 @@ config M68000
> >> select GENERIC_CSUM
> >> select CPU_NO_EFFICIENT_FFS
> >> select HAVE_ARCH_HASH
> >> + select HAVE_PAGE_SIZE_4KB
> >
> > Perhaps replace this by
> >
> > config M68KCLASSIC
> > bool "Classic M68K CPU family support"
> > select HAVE_ARCH_PFN_VALID
> > + select HAVE_PAGE_SIZE_4KB if !MMU
> >
> > so it covers all 680x0 CPUs without MMU?
>
> I was a bit unsure about how to best do this since there
> is not really a need for a fixed page size on nommu kernels,
> whereas the three MMU configs clearly tie the page size to
> the MMU rather than the platform.
>
> There should be no reason for coldfire to have a different
> page size from dragonball if neither of them actually uses
> hardware pages, so one of them could be changed later.
Indeed, in theory, PAGE_SIZE doesn't matter for nommu, but the concept
of pages is used all over the place in Linux.
I'm mostly worried about some Coldfire code relying on the actual value
of PAGE_SIZE in some other context. e.g. for configuring non-cacheable
regions.
And does this impact running nommu binaries on a system with MMU?
I.e. if nommu binaries were built with a 4 KiB PAGE_SIZE, do they
still run on MMU systems with an 8 KiB PAGE_SIZE (coldfire and sun3),
or are there some subtleties to take into account?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures
On Tue, Feb 27, 2024, at 09:54, Geert Uytterhoeven wrote: > Hi Arnd, >> diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu >> index 9dcf245c9cbf..c777a129768a 100644 >> --- a/arch/m68k/Kconfig.cpu >> +++ b/arch/m68k/Kconfig.cpu >> @@ -30,6 +30,7 @@ config COLDFIRE >> select GENERIC_CSUM >> select GPIOLIB >> select HAVE_LEGACY_CLK >> + select HAVE_PAGE_SIZE_8KB if !MMU > > if you would drop the !MMU-dependency here. > >> >> endchoice >> >> @@ -45,6 +46,7 @@ config M68000 >> select GENERIC_CSUM >> select CPU_NO_EFFICIENT_FFS >> select HAVE_ARCH_HASH >> + select HAVE_PAGE_SIZE_4KB > > Perhaps replace this by > > config M68KCLASSIC > bool "Classic M68K CPU family support" > select HAVE_ARCH_PFN_VALID > + select HAVE_PAGE_SIZE_4KB if !MMU > > so it covers all 680x0 CPUs without MMU? I was a bit unsure about how to best do this since there is not really a need for a fixed page size on nommu kernels, whereas the three MMU configs clearly tie the page size to the MMU rather than the platform. There should be no reason for coldfire to have a different page size from dragonball if neither of them actually uses hardware pages, so one of them could be changed later. Let me know if that makes sense to you, or you still prefer me to change it like you suggested. Arnd
Re: [PATCH 3/4] arch: define CONFIG_PAGE_SIZE_*KB on all architectures
Hi Arnd,
On Mon, Feb 26, 2024 at 5:15 PM Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Most architectures only support a single hardcoded page size. In order
> to ensure that each one of these sets the corresponding Kconfig symbols,
> change over the PAGE_SHIFT definition to the common one and allow
> only the hardware page size to be selected.
>
> Signed-off-by: Arnd Bergmann
Thanks for your patch!
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -84,12 +84,15 @@ config MMU
>
> config MMU_MOTOROLA
> bool
> + select HAVE_PAGE_SIZE_4KB
>
> config MMU_COLDFIRE
> + select HAVE_PAGE_SIZE_8KB
I think you can do without this...
> bool
>
> config MMU_SUN3
> bool
> + select HAVE_PAGE_SIZE_8KB
> depends on MMU && !MMU_MOTOROLA && !MMU_COLDFIRE
>
> config ARCH_SUPPORTS_KEXEC
> diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu
> index 9dcf245c9cbf..c777a129768a 100644
> --- a/arch/m68k/Kconfig.cpu
> +++ b/arch/m68k/Kconfig.cpu
> @@ -30,6 +30,7 @@ config COLDFIRE
> select GENERIC_CSUM
> select GPIOLIB
> select HAVE_LEGACY_CLK
> + select HAVE_PAGE_SIZE_8KB if !MMU
if you would drop the !MMU-dependency here.
>
> endchoice
>
> @@ -45,6 +46,7 @@ config M68000
> select GENERIC_CSUM
> select CPU_NO_EFFICIENT_FFS
> select HAVE_ARCH_HASH
> + select HAVE_PAGE_SIZE_4KB
Perhaps replace this by
config M68KCLASSIC
bool "Classic M68K CPU family support"
select HAVE_ARCH_PFN_VALID
+ select HAVE_PAGE_SIZE_4KB if !MMU
so it covers all 680x0 CPUs without MMU?
> select LEGACY_TIMER_TICK
> help
> The Freescale (was Motorola) 68000 CPU is the first generation of
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/4] arch: consolidate existing CONFIG_PAGE_SIZE_*KB definitions
Hi Arnd,
On Mon, Feb 26, 2024 at 5:14 PM Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> These four architectures define the same Kconfig symbols for configuring
> the page size. Move the logic into a common place where it can be shared
> with all other architectures.
>
> Signed-off-by: Arnd Bergmann
Thanks for your patch!
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> +config PAGE_SIZE_4KB
> + bool "4KB pages"
Now you got rid of the 4000-byte ("4kB") pages and friends, please
do not replace these by Kelvin-bytes, and use the official binary
prefixes => "4 KiB".
> + depends on HAVE_PAGE_SIZE_4KB
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
