Re: linux-next: build failure on powerpc 8xx with 16k pages
[+Arnd since I think we spoke about this on IRC once] On Thu, Jun 04, 2020 at 02:35:14PM +, Christophe Leroy wrote: > Now I get the same issue at > >CC mm/mincore.o > In file included from ./include/asm-generic/bug.h:5:0, > from ./arch/powerpc/include/asm/bug.h:109, > from ./include/linux/bug.h:5, > from ./include/linux/mmdebug.h:5, > from ./include/linux/mm.h:9, > from ./include/linux/pagemap.h:8, > from mm/mincore.c:11: > In function 'huge_ptep_get', > inlined from 'mincore_hugetlb' at mm/mincore.c:35:20: > ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_218' > declared with attribute error: Unsupported access size for > {READ,WRITE}_ONCE(). > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ^ > ./include/linux/compiler.h:373:4: note: in definition of macro > '__compiletime_assert' > prefix ## suffix();\ > ^ > ./include/linux/compiler.h:392:2: note: in expansion of macro > '_compiletime_assert' > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ^ > ./include/linux/compiler.h:405:2: note: in expansion of macro > 'compiletime_assert' > compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ > ^ > ./include/linux/compiler.h:291:2: note: in expansion of macro > 'compiletime_assert_rwonce_type' > compiletime_assert_rwonce_type(x);\ > ^ > ./include/asm-generic/hugetlb.h:125:9: note: in expansion of macro > 'READ_ONCE' > return READ_ONCE(*ptep); > ^ > make[2]: *** [mm/mincore.o] Error 1 > > I guess for this one I have to implement platform specific huge_ptep_get() Yeah, or bite the bullet and introduce proper accessors for all these things: pte_read() pmd_read() pud_read() etc with the default implementation pointing at READ_ONCE(), but allowing an architecture override. It's a big job because mm/ would need repainting, but it would have the benefit of being able to remove aggregate types from READ_ONCE() entirely and using a special accessor just for the page-table types. That might also mean that we could have asm-generic versions of things like ptep_get_and_clear() that work for architectures with hardware update and need atomic rmw. But I'm getting ahead of myself. Will
Re: linux-next: build failure on powerpc 8xx with 16k pages
On 06/04/2020 12:00 PM, Peter Zijlstra wrote: On Thu, Jun 04, 2020 at 12:17:23PM +0100, Will Deacon wrote: Hi, [+Peter] On Thu, Jun 04, 2020 at 10:48:03AM +, Christophe Leroy wrote: Using mpc885_ads_defconfig with CONFIG_PPC_16K_PAGES instead of CONFIG_PPC_4K_PAGES, getting the following build failure: CC mm/gup.o In file included from ./include/linux/kernel.h:11:0, from mm/gup.c:2: In function 'gup_hugepte.constprop', inlined from 'gup_huge_pd.isra.78' at mm/gup.c:2465:8: ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_257' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE(). _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:373:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^ ./include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:405:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/linux/compiler.h:291:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^ mm/gup.c:2428:8: note: in expansion of macro 'READ_ONCE' pte = READ_ONCE(*ptep); ^ In function 'gup_get_pte', inlined from 'gup_pte_range' at mm/gup.c:2228:9, inlined from 'gup_pmd_range' at mm/gup.c:2613:15, inlined from 'gup_pud_range' at mm/gup.c:2641:15, inlined from 'gup_p4d_range' at mm/gup.c:2666:15, inlined from 'gup_pgd_range' at mm/gup.c:2694:15, inlined from 'internal_get_user_pages_fast' at mm/gup.c:2785:3: At first glance, this looks like a real bug in the 16k page code -- you're loading the pte non-atomically on the fast GUP path and so you're prone to tearing, which probably isn't what you want. For a short-term hack, I'd suggest having CONFIG_HAVE_FAST_GUP depend on !CONFIG_PPC_16K_PAGES, but if you want to support this them you'll need to rework your pte_t so that it can be loaded atomically. Looking at commit 55c8fc3f49302, they're all the exact same value, so what they could do is grow another special gup_get_pte() variant that just loads the first value. Also, per that very same commit, there's a distinct lack of WRITE_ONCE() in the pte_update() / __set_pte_at() paths for much of Power. Thanks for the idea. Now I get the same issue at CC mm/mincore.o In file included from ./include/asm-generic/bug.h:5:0, from ./arch/powerpc/include/asm/bug.h:109, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/mm.h:9, from ./include/linux/pagemap.h:8, from mm/mincore.c:11: In function 'huge_ptep_get', inlined from 'mincore_hugetlb' at mm/mincore.c:35:20: ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_218' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE(). _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:373:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^ ./include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:405:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/linux/compiler.h:291:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^ ./include/asm-generic/hugetlb.h:125:9: note: in expansion of macro 'READ_ONCE' return READ_ONCE(*ptep); ^ make[2]: *** [mm/mincore.o] Error 1 I guess for this one I have to implement platform specific huge_ptep_get() Christophe
Re: linux-next: build failure on powerpc 8xx with 16k pages
On 06/04/2020 11:17 AM, Will Deacon wrote: Hi, [+Peter] On Thu, Jun 04, 2020 at 10:48:03AM +, Christophe Leroy wrote: Using mpc885_ads_defconfig with CONFIG_PPC_16K_PAGES instead of CONFIG_PPC_4K_PAGES, getting the following build failure: CC mm/gup.o In file included from ./include/linux/kernel.h:11:0, from mm/gup.c:2: In function 'gup_hugepte.constprop', inlined from 'gup_huge_pd.isra.78' at mm/gup.c:2465:8: ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_257' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE(). _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:373:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^ ./include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:405:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/linux/compiler.h:291:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^ mm/gup.c:2428:8: note: in expansion of macro 'READ_ONCE' pte = READ_ONCE(*ptep); ^ In function 'gup_get_pte', inlined from 'gup_pte_range' at mm/gup.c:2228:9, inlined from 'gup_pmd_range' at mm/gup.c:2613:15, inlined from 'gup_pud_range' at mm/gup.c:2641:15, inlined from 'gup_p4d_range' at mm/gup.c:2666:15, inlined from 'gup_pgd_range' at mm/gup.c:2694:15, inlined from 'internal_get_user_pages_fast' at mm/gup.c:2785:3: At first glance, this looks like a real bug in the 16k page code -- you're loading the pte non-atomically on the fast GUP path and so you're prone to tearing, which probably isn't what you want. For a short-term hack, I'd suggest having CONFIG_HAVE_FAST_GUP depend on !CONFIG_PPC_16K_PAGES, but if you want to support this them you'll need to rework your pte_t so that it can be loaded atomically. What do you mean by *rework* pte_t ? pte are 32 bits words in size and are spread every 4 words in memory. Therefore pte_t has to be 128 bits because unlike huge_pte handling which always use huge_pte_offset() in loops, many many places in the kernel do pte++, so we need the pte type to be the size of the interval from one pte to the next one. Christophe
Re: linux-next: build failure on powerpc 8xx with 16k pages
On Thu, Jun 04, 2020 at 12:17:23PM +0100, Will Deacon wrote: > Hi, [+Peter] > > On Thu, Jun 04, 2020 at 10:48:03AM +, Christophe Leroy wrote: > > Using mpc885_ads_defconfig with CONFIG_PPC_16K_PAGES instead of > > CONFIG_PPC_4K_PAGES, getting the following build failure: > > > > CC mm/gup.o > > In file included from ./include/linux/kernel.h:11:0, > > from mm/gup.c:2: > > In function 'gup_hugepte.constprop', > > inlined from 'gup_huge_pd.isra.78' at mm/gup.c:2465:8: > > ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_257' > > declared with attribute error: Unsupported access size for > > {READ,WRITE}_ONCE(). > > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > > ^ > > ./include/linux/compiler.h:373:4: note: in definition of macro > > '__compiletime_assert' > > prefix ## suffix();\ > > ^ > > ./include/linux/compiler.h:392:2: note: in expansion of macro > > '_compiletime_assert' > > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > > ^ > > ./include/linux/compiler.h:405:2: note: in expansion of macro > > 'compiletime_assert' > > compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ > > ^ > > ./include/linux/compiler.h:291:2: note: in expansion of macro > > 'compiletime_assert_rwonce_type' > > compiletime_assert_rwonce_type(x);\ > > ^ > > mm/gup.c:2428:8: note: in expansion of macro 'READ_ONCE' > > pte = READ_ONCE(*ptep); > > ^ > > In function 'gup_get_pte', > > inlined from 'gup_pte_range' at mm/gup.c:2228:9, > > inlined from 'gup_pmd_range' at mm/gup.c:2613:15, > > inlined from 'gup_pud_range' at mm/gup.c:2641:15, > > inlined from 'gup_p4d_range' at mm/gup.c:2666:15, > > inlined from 'gup_pgd_range' at mm/gup.c:2694:15, > > inlined from 'internal_get_user_pages_fast' at mm/gup.c:2785:3: > > At first glance, this looks like a real bug in the 16k page code -- you're > loading the pte non-atomically on the fast GUP path and so you're prone to > tearing, which probably isn't what you want. For a short-term hack, I'd > suggest having CONFIG_HAVE_FAST_GUP depend on !CONFIG_PPC_16K_PAGES, but if > you want to support this them you'll need to rework your pte_t so that it > can be loaded atomically. Looking at commit 55c8fc3f49302, they're all the exact same value, so what they could do is grow another special gup_get_pte() variant that just loads the first value. Also, per that very same commit, there's a distinct lack of WRITE_ONCE() in the pte_update() / __set_pte_at() paths for much of Power.
Re: linux-next: build failure on powerpc 8xx with 16k pages
Hi, [+Peter] On Thu, Jun 04, 2020 at 10:48:03AM +, Christophe Leroy wrote: > Using mpc885_ads_defconfig with CONFIG_PPC_16K_PAGES instead of > CONFIG_PPC_4K_PAGES, getting the following build failure: > > CC mm/gup.o > In file included from ./include/linux/kernel.h:11:0, > from mm/gup.c:2: > In function 'gup_hugepte.constprop', > inlined from 'gup_huge_pd.isra.78' at mm/gup.c:2465:8: > ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_257' > declared with attribute error: Unsupported access size for > {READ,WRITE}_ONCE(). > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ^ > ./include/linux/compiler.h:373:4: note: in definition of macro > '__compiletime_assert' > prefix ## suffix();\ > ^ > ./include/linux/compiler.h:392:2: note: in expansion of macro > '_compiletime_assert' > _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) > ^ > ./include/linux/compiler.h:405:2: note: in expansion of macro > 'compiletime_assert' > compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ > ^ > ./include/linux/compiler.h:291:2: note: in expansion of macro > 'compiletime_assert_rwonce_type' > compiletime_assert_rwonce_type(x);\ > ^ > mm/gup.c:2428:8: note: in expansion of macro 'READ_ONCE' > pte = READ_ONCE(*ptep); > ^ > In function 'gup_get_pte', > inlined from 'gup_pte_range' at mm/gup.c:2228:9, > inlined from 'gup_pmd_range' at mm/gup.c:2613:15, > inlined from 'gup_pud_range' at mm/gup.c:2641:15, > inlined from 'gup_p4d_range' at mm/gup.c:2666:15, > inlined from 'gup_pgd_range' at mm/gup.c:2694:15, > inlined from 'internal_get_user_pages_fast' at mm/gup.c:2785:3: At first glance, this looks like a real bug in the 16k page code -- you're loading the pte non-atomically on the fast GUP path and so you're prone to tearing, which probably isn't what you want. For a short-term hack, I'd suggest having CONFIG_HAVE_FAST_GUP depend on !CONFIG_PPC_16K_PAGES, but if you want to support this them you'll need to rework your pte_t so that it can be loaded atomically. Will
linux-next: build failure on powerpc 8xx with 16k pages
Hi all, Using mpc885_ads_defconfig with CONFIG_PPC_16K_PAGES instead of CONFIG_PPC_4K_PAGES, getting the following build failure: CC mm/gup.o In file included from ./include/linux/kernel.h:11:0, from mm/gup.c:2: In function 'gup_hugepte.constprop', inlined from 'gup_huge_pd.isra.78' at mm/gup.c:2465:8: ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_257' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE(). _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:373:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^ ./include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:405:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/linux/compiler.h:291:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^ mm/gup.c:2428:8: note: in expansion of macro 'READ_ONCE' pte = READ_ONCE(*ptep); ^ In function 'gup_get_pte', inlined from 'gup_pte_range' at mm/gup.c:2228:9, inlined from 'gup_pmd_range' at mm/gup.c:2613:15, inlined from 'gup_pud_range' at mm/gup.c:2641:15, inlined from 'gup_p4d_range' at mm/gup.c:2666:15, inlined from 'gup_pgd_range' at mm/gup.c:2694:15, inlined from 'internal_get_user_pages_fast' at mm/gup.c:2785:3: ./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_254' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE(). _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:373:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^ ./include/linux/compiler.h:392:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ./include/linux/compiler.h:405:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/linux/compiler.h:291:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^ mm/gup.c:2199:9: note: in expansion of macro 'READ_ONCE' return READ_ONCE(*ptep); ^ make[2]: *** [mm/gup.o] Error 1 Bisected to: 2ab3a0a02905 (HEAD, refs/bisect/bad) READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses Christophe