| Message ID | 20240923142533.1197982-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: Make SPLIT_PTE_PTLOCKS depend on the existence of NR_CPUS | expand |
On 23.09.24 16:25, Guenter Roeck wrote: > SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates > to true if there is no NR_CPUS configuration option (such as for m68k). > This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. > This in turn causes the m68k "q800" machine to crash in qemu. Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... Thanks for debugging and fixing! Acked-by: David Hildenbrand <david@redhat.com>
Hi Guenter, kernel test robot noticed the following build errors: [auto build test ERROR on akpm-mm/mm-everything] url: https://github.com/intel-lab-lkp/linux/commits/Guenter-Roeck/mm-Make-SPLIT_PTE_PTLOCKS-depend-on-the-existence-of-NR_CPUS/20240923-222628 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20240923142533.1197982-1-linux%40roeck-us.net patch subject: [PATCH] mm: Make SPLIT_PTE_PTLOCKS depend on the existence of NR_CPUS config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240924/202409240416.eEfELHN9-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240924/202409240416.eEfELHN9-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409240416.eEfELHN9-lkp@intel.com/ All errors (new ones prefixed by >>): arch/s390/mm/gmap.c: In function '__gmap_segment_gaddr': >> arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'; did you mean 'pmd_pgtable'? [-Wimplicit-function-declaration] 357 | page = pmd_pgtable_page((pmd_t *) entry); | ^~~~~~~~~~~~~~~~ | pmd_pgtable >> arch/s390/mm/gmap.c:357:14: error: assignment to 'struct page *' from 'int' makes pointer from integer without a cast [-Wint-conversion] 357 | page = pmd_pgtable_page((pmd_t *) entry); | ^ vim +357 arch/s390/mm/gmap.c 1e133ab296f3ff Martin Schwidefsky 2016-03-08 343 1e133ab296f3ff Martin Schwidefsky 2016-03-08 344 /** 1e133ab296f3ff Martin Schwidefsky 2016-03-08 345 * __gmap_segment_gaddr - find virtual address from segment pointer 1e133ab296f3ff Martin Schwidefsky 2016-03-08 346 * @entry: pointer to a segment table entry in the guest address space 1e133ab296f3ff Martin Schwidefsky 2016-03-08 347 * 1e133ab296f3ff Martin Schwidefsky 2016-03-08 348 * Returns the virtual address in the guest address space for the segment 1e133ab296f3ff Martin Schwidefsky 2016-03-08 349 */ 1e133ab296f3ff Martin Schwidefsky 2016-03-08 350 static unsigned long __gmap_segment_gaddr(unsigned long *entry) 1e133ab296f3ff Martin Schwidefsky 2016-03-08 351 { 1e133ab296f3ff Martin Schwidefsky 2016-03-08 352 struct page *page; 7e25de77bc5ea5 Anshuman Khandual 2022-11-25 353 unsigned long offset; 1e133ab296f3ff Martin Schwidefsky 2016-03-08 354 1e133ab296f3ff Martin Schwidefsky 2016-03-08 355 offset = (unsigned long) entry / sizeof(unsigned long); 1e133ab296f3ff Martin Schwidefsky 2016-03-08 356 offset = (offset & (PTRS_PER_PMD - 1)) * PMD_SIZE; 7e25de77bc5ea5 Anshuman Khandual 2022-11-25 @357 page = pmd_pgtable_page((pmd_t *) entry); 1e133ab296f3ff Martin Schwidefsky 2016-03-08 358 return page->index + offset; 1e133ab296f3ff Martin Schwidefsky 2016-03-08 359 } 1e133ab296f3ff Martin Schwidefsky 2016-03-08 360
Hi Guenter,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
url: https://github.com/intel-lab-lkp/linux/commits/Guenter-Roeck/mm-Make-SPLIT_PTE_PTLOCKS-depend-on-the-existence-of-NR_CPUS/20240923-222628
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20240923142533.1197982-1-linux%40roeck-us.net
patch subject: [PATCH] mm: Make SPLIT_PTE_PTLOCKS depend on the existence of NR_CPUS
config: s390-defconfig (https://download.01.org/0day-ci/archive/20240924/202409240546.SJwj9tUj-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 8663a75fa2f31299ab8d1d90288d9df92aadee88)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240924/202409240546.SJwj9tUj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409240546.SJwj9tUj-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/s390/mm/gmap.c:12:
In file included from include/linux/pagewalk.h:5:
In file included from include/linux/mm.h:2198:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> arch/s390/mm/gmap.c:357:9: error: call to undeclared function 'pmd_pgtable_page'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
357 | page = pmd_pgtable_page((pmd_t *) entry);
| ^
>> arch/s390/mm/gmap.c:357:7: error: incompatible integer to pointer conversion assigning to 'struct page *' from 'int' [-Wint-conversion]
357 | page = pmd_pgtable_page((pmd_t *) entry);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4 warnings and 2 errors generated.
vim +/pmd_pgtable_page +357 arch/s390/mm/gmap.c
1e133ab296f3ff Martin Schwidefsky 2016-03-08 343
1e133ab296f3ff Martin Schwidefsky 2016-03-08 344 /**
1e133ab296f3ff Martin Schwidefsky 2016-03-08 345 * __gmap_segment_gaddr - find virtual address from segment pointer
1e133ab296f3ff Martin Schwidefsky 2016-03-08 346 * @entry: pointer to a segment table entry in the guest address space
1e133ab296f3ff Martin Schwidefsky 2016-03-08 347 *
1e133ab296f3ff Martin Schwidefsky 2016-03-08 348 * Returns the virtual address in the guest address space for the segment
1e133ab296f3ff Martin Schwidefsky 2016-03-08 349 */
1e133ab296f3ff Martin Schwidefsky 2016-03-08 350 static unsigned long __gmap_segment_gaddr(unsigned long *entry)
1e133ab296f3ff Martin Schwidefsky 2016-03-08 351 {
1e133ab296f3ff Martin Schwidefsky 2016-03-08 352 struct page *page;
7e25de77bc5ea5 Anshuman Khandual 2022-11-25 353 unsigned long offset;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 354
1e133ab296f3ff Martin Schwidefsky 2016-03-08 355 offset = (unsigned long) entry / sizeof(unsigned long);
1e133ab296f3ff Martin Schwidefsky 2016-03-08 356 offset = (offset & (PTRS_PER_PMD - 1)) * PMD_SIZE;
7e25de77bc5ea5 Anshuman Khandual 2022-11-25 @357 page = pmd_pgtable_page((pmd_t *) entry);
1e133ab296f3ff Martin Schwidefsky 2016-03-08 358 return page->index + offset;
1e133ab296f3ff Martin Schwidefsky 2016-03-08 359 }
1e133ab296f3ff Martin Schwidefsky 2016-03-08 360
On 9/23/24 08:23, David Hildenbrand wrote: > On 23.09.24 16:25, Guenter Roeck wrote: >> SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates >> to true if there is no NR_CPUS configuration option (such as for m68k). >> This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. >> This in turn causes the m68k "q800" machine to crash in qemu. > > Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... > > Thanks for debugging and fixing! > > Acked-by: David Hildenbrand <david@redhat.com> > Apparently it wasn't that simple :-(. 0-day reports a build failure with s390 builds. arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'. Turns out that depends on NR_CPUS && NR_CPUS >= 4 doesn't work and disables SPLIT_PTE_PTLOCKS even if NR_CPUS _is_ defined. I have no idea how to declare the dependency correctly. Sorry, I did not expect that. Guenter
On 9/23/24 15:08, Guenter Roeck wrote: > On 9/23/24 08:23, David Hildenbrand wrote: >> On 23.09.24 16:25, Guenter Roeck wrote: >>> SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates >>> to true if there is no NR_CPUS configuration option (such as for m68k). >>> This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. >>> This in turn causes the m68k "q800" machine to crash in qemu. >> >> Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... >> >> Thanks for debugging and fixing! >> >> Acked-by: David Hildenbrand <david@redhat.com> >> > > Apparently it wasn't that simple :-(. 0-day reports a build failure > with s390 builds. > > arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'. > > Turns out that > depends on NR_CPUS && NR_CPUS >= 4 > > doesn't work and disables SPLIT_PTE_PTLOCKS even if NR_CPUS _is_ defined. > I have no idea how to declare the dependency correctly. > Sorry, I did not expect that. > The only solution I found was to define NR_CPUS for m68k. That seems to be the only architecture not defining it, so hopefully that is an acceptable solution. I'll send v2 of the patch shortly. Guenter
Hi Günter, CC kbuild I have two comments... On Tue, Sep 24, 2024 at 1:52 AM Guenter Roeck <linux@roeck-us.net> wrote: > On 9/23/24 15:08, Guenter Roeck wrote: > > On 9/23/24 08:23, David Hildenbrand wrote: > >> On 23.09.24 16:25, Guenter Roeck wrote: > >>> SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates > >>> to true if there is no NR_CPUS configuration option (such as for m68k). > >>> This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. > >>> This in turn causes the m68k "q800" machine to crash in qemu. Should this be fixed in Kconfig (too)? > >> Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... > >> > >> Thanks for debugging and fixing! > >> > >> Acked-by: David Hildenbrand <david@redhat.com> > >> > > > > Apparently it wasn't that simple :-(. 0-day reports a build failure > > with s390 builds. > > > > arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'. > > > > Turns out that > > depends on NR_CPUS && NR_CPUS >= 4 > > > > doesn't work and disables SPLIT_PTE_PTLOCKS even if NR_CPUS _is_ defined. > > I have no idea how to declare the dependency correctly. > > Sorry, I did not expect that. > > The only solution I found was to define NR_CPUS for m68k. That seems to be > the only architecture not defining it, so hopefully that is an acceptable > solution. I'll send v2 of the patch shortly. My first thought was to agree, as m68k is indeed the only architecture that does not define NR_CPUS. Upon closer look, most architectures have NR_CPUS depend on SMP, hence I assume the issue could happen for those too (although I didn't manage to create such a config on anything but m68k)? So the simple solution would be to add a dependency on SMP to SPLIT_PTE_PTLOCKS. BTW, the list of excluded architectures looks fragile to me: config SPLIT_PTE_PTLOCKS def_bool y depends on MMU depends on NR_CPUS >= 4 depends on !ARM || CPU_CACHE_VIPT depends on !PARISC || PA20 depends on !SPARC32 If this can't be handled in a generic way, perhaps this should be changed from opt-out to opt-in (i.e. select gate symbol in arch-specific Kconfig)? Gr{oetje,eeting}s, Geert
On 24.09.24 09:45, Geert Uytterhoeven wrote: > Hi Günter, > > CC kbuild > > I have two comments... > > On Tue, Sep 24, 2024 at 1:52 AM Guenter Roeck <linux@roeck-us.net> wrote: >> On 9/23/24 15:08, Guenter Roeck wrote: >>> On 9/23/24 08:23, David Hildenbrand wrote: >>>> On 23.09.24 16:25, Guenter Roeck wrote: >>>>> SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates >>>>> to true if there is no NR_CPUS configuration option (such as for m68k). >>>>> This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. >>>>> This in turn causes the m68k "q800" machine to crash in qemu. > > Should this be fixed in Kconfig (too)? > >>>> Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... >>>> >>>> Thanks for debugging and fixing! >>>> >>>> Acked-by: David Hildenbrand <david@redhat.com> >>>> >>> >>> Apparently it wasn't that simple :-(. 0-day reports a build failure >>> with s390 builds. >>> >>> arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'. >>> >>> Turns out that >>> depends on NR_CPUS && NR_CPUS >= 4 >>> >>> doesn't work and disables SPLIT_PTE_PTLOCKS even if NR_CPUS _is_ defined. >>> I have no idea how to declare the dependency correctly. >>> Sorry, I did not expect that. >> >> The only solution I found was to define NR_CPUS for m68k. That seems to be >> the only architecture not defining it, so hopefully that is an acceptable >> solution. I'll send v2 of the patch shortly. > > My first thought was to agree, as m68k is indeed the only architecture > that does not define NR_CPUS. Upon closer look, most architectures > have NR_CPUS depend on SMP, hence I assume the issue could happen for > those too (although I didn't manage to create such a config on anything I recall that I played the same thing, convincing me that having no CONFIG_NR_CPUS on !SMP would actually do the right thing. Apparently it doesn't for m68k at least. > but m68k)? So the simple solution would be to add a dependency on > SMP to SPLIT_PTE_PTLOCKS. That will probably work for now. CONFIG_NR_CPUS should be cleaned up at some point to sort out the FIXME I commented in v2. Having kconfig set CONFIG_NR_CPUS=1 without SMP would be easiest, but it's probably not that easy. > > BTW, the list of excluded architectures looks fragile to me: > > config SPLIT_PTE_PTLOCKS > def_bool y > depends on MMU > depends on NR_CPUS >= 4 > depends on !ARM || CPU_CACHE_VIPT > depends on !PARISC || PA20 > depends on !SPARC32 > > If this can't be handled in a generic way, perhaps this should be > changed from opt-out to opt-in (i.e. select gate symbol in arch-specific > Kconfig)? Yes, as stated in my commit: More cleanups would be reasonable (like the arch-specific "depends on" for CONFIG_SPLIT_PTE_PTLOCKS), but we'll leave that for another day.
On 9/24/24 00:45, Geert Uytterhoeven wrote: > Hi Günter, > > CC kbuild > > I have two comments... > > On Tue, Sep 24, 2024 at 1:52 AM Guenter Roeck <linux@roeck-us.net> wrote: >> On 9/23/24 15:08, Guenter Roeck wrote: >>> On 9/23/24 08:23, David Hildenbrand wrote: >>>> On 23.09.24 16:25, Guenter Roeck wrote: >>>>> SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates >>>>> to true if there is no NR_CPUS configuration option (such as for m68k). >>>>> This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. >>>>> This in turn causes the m68k "q800" machine to crash in qemu. > > Should this be fixed in Kconfig (too)? > I don't know. I thought that was intentional, though I don't understand the logic. I didn't find a documentation that would explain how boolean dependencies of integer objects are supposed to be handled. My expectation that it would be similar to C was obviously wrong. >>>> Oh, that's why my compile tests still worked ... I even removed the additional NR_CPUS check, assuming it's not required ... >>>> >>>> Thanks for debugging and fixing! >>>> >>>> Acked-by: David Hildenbrand <david@redhat.com> >>>> >>> >>> Apparently it wasn't that simple :-(. 0-day reports a build failure >>> with s390 builds. >>> >>> arch/s390/mm/gmap.c:357:16: error: implicit declaration of function 'pmd_pgtable_page'. >>> >>> Turns out that >>> depends on NR_CPUS && NR_CPUS >= 4 >>> >>> doesn't work and disables SPLIT_PTE_PTLOCKS even if NR_CPUS _is_ defined. >>> I have no idea how to declare the dependency correctly. >>> Sorry, I did not expect that. >> >> The only solution I found was to define NR_CPUS for m68k. That seems to be >> the only architecture not defining it, so hopefully that is an acceptable >> solution. I'll send v2 of the patch shortly. > > My first thought was to agree, as m68k is indeed the only architecture > that does not define NR_CPUS. Upon closer look, most architectures > have NR_CPUS depend on SMP, hence I assume the issue could happen for > those too (although I didn't manage to create such a config on anything > but m68k)? So the simple solution would be to add a dependency on > SMP to SPLIT_PTE_PTLOCKS. > Makes sense. I'll send v3. Guenter
diff --git a/mm/Kconfig b/mm/Kconfig index 09aebca1cae3..20fe60389cf5 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -595,7 +595,7 @@ config ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE config SPLIT_PTE_PTLOCKS def_bool y depends on MMU - depends on NR_CPUS >= 4 + depends on NR_CPUS && NR_CPUS >= 4 depends on !ARM || CPU_CACHE_VIPT depends on !PARISC || PA20 depends on !SPARC32
SPLIT_PTE_PTLOCKS already depends on "NR_CPUS >= 4", but that evaluates to true if there is no NR_CPUS configuration option (such as for m68k). This results in CONFIG_SPLIT_PTE_PTLOCKS=y for mac_defconfig. This in turn causes the m68k "q800" machine to crash in qemu. Adding an explicit dependency on the existence of NR_CPUS fixes the problem. Fixes: 394290cba966 ("mm: turn USE_SPLIT_PTE_PTLOCKS / USE_SPLIT_PTE_PTLOCKS into Kconfig options") Cc: David Hildenbrand <david@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- mm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)