On Thu, Feb 27, 2020 at 9:57 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Arjun, > > On Thu, Feb 27, 2020 at 6:45 PM Arjun Roy <arjunroy@xxxxxxxxxx> wrote: > > On Thu, Feb 27, 2020 at 9:13 AM Arjun Roy <arjunroy@xxxxxxxxxx> wrote: > > > On Thu, Feb 27, 2020 at 1:03 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > > > On Thu, Feb 27, 2020 at 5:12 AM Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote: > > > > > After merging the akpm tree, today's linux-next build (sparc defconfig) > > > > > failed like this: > > > > > > > > > > In file included from include/linux/list.h:9:0, > > > > > from include/linux/smp.h:12, > > > > > from include/linux/kernel_stat.h:5, > > > > > from mm/memory.c:42: > > > > > mm/memory.c: In function 'insert_pages': > > > > > mm/memory.c:1523:41: error: implicit declaration of function 'pte_index'; did you mean 'page_index'? [-Werror=implicit-function-declaration] > > > > > remaining_pages_total, PTRS_PER_PTE - pte_index(addr)); > > > > > ^ > > > > > include/linux/kernel.h:842:40: note: in definition of macro '__typecheck' > > > > > (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) > > > > > ^ > > > > > include/linux/kernel.h:866:24: note: in expansion of macro '__safe_cmp' > > > > > __builtin_choose_expr(__safe_cmp(x, y), \ > > > > > ^~~~~~~~~~ > > > > > include/linux/kernel.h:934:27: note: in expansion of macro '__careful_cmp' > > > > > #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) > > > > > ^~~~~~~~~~~~~ > > > > > mm/memory.c:1522:26: note: in expansion of macro 'min_t' > > > > > pages_to_write_in_pmd = min_t(unsigned long, > > > > > ^~~~~ > > > > > > > > Same issue on m68k, as per a report from kisskb. > > > > > > > > > Caused by patch > > > > > > > > > > "mm/memory.c: add vm_insert_pages()" > > > > > > > > > > sparc32 does not implement pte_index at all :-( > > > > > > > > Seems like about only half of the architectures do. > > > > > > > > > > :/ I begin to suspect the only sane way to make this work is to have a > > > per-arch header defined method, returning a bool saying whether > > > pte_index() is meaningful or not on that arch, and early on in > > > vm_insert_pages() if that bool returns true, to just call > > > vm_insert_page() in a loop. > > > > > > > So, here is what I propose: something like the following macro in a > > per-arch header: > > > > #define PTE_INDEX_DEFINED 1 // or 0 if it is not > > pte_index is already a #define on architectures where it exists, so > you can just use that. > > > In mm/memory.c, another macro: > > > > #ifndef PTE_INDEX_DEFINED > > #define PTE_INDEX_DEFINED 0 > > #endifndef > > No need for the above... > > > And inside vm_insert_pages: > > > > int vm_insert_pages() { > > > > #if PTE_INDEX_DEFINED > > ... if you use "#ifdef" here. > Sounds good, thanks. I'll cook up a patch and send it along. -Arjun > > > > // The existing method > > > > #else > > > > for (i=0; i<n; ++i) > > vm_insert_page(i) > > > > #endif > > } > > > > That way: > > 1. No playing whack-a-mole with different architectures > > 2. Architecture that knows pte_index is meaningful works can define > > this explicitly > > 3. Can remove the sparc patches modifying pte_index that Stephen and I > > contributed. > > > > If that sounds acceptable I can cook a patch. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > 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