On Wed, 17 Apr 2019 09:57:01 -0700 Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Apr 17, 2019 at 1:02 AM Martin Schwidefsky > <schwidefsky@xxxxxxxxxx> wrote: > > > > Grumpf, that does *not* work. For gup the table entries may be read only > > once. Now I remember why I open-coded p4d_offset, pud_offset and pmd_offset > > in arch/s390/mm/gup.c, to avoid to read the table entries twice. > > It will be hard to use the common gup code after all. > > Hmm. The common gup code generally should do the "read only once" > thing too (since by definition the gup-fast case is done without > locking), although it's probably the case that most architectures > simply don't care. > > What would it require for the generic code to work for s390? The problematic lines in the generic gup code are these three: 1845: pmdp = pmd_offset(&pud, addr); 1888: pudp = pud_offset(&p4d, addr); 1916: p4dp = p4d_offset(&pgd, addr); Passing the pointer of a *copy* of a page table entry to pxd_offset() does not work with the page table folding on s390. The pxd_offset() function on s390 have to make a choice, either return the dereferenced value behind the passed pointer (that works) or return the original page table pointer if the table level is folded (that does not work). To fix this we would need three new helpers pmd_offset_orig, pud_offset_orig and p4d_offset_orig, their generic definition would look like this: #define p4d_offset_orig(pgdp, pgd, address) p4d_offset(&pgd, address) #define pud_offset_orig(p4dp, p4d, address) pud_offset(&p4d, address) #define pmd_offset_orig(pudp, pud, address) pmd_offset(&pud, address) For the s390 definition see the following branch: git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git generic-gup A quick test with this branch shows everything working normally. Keeping my fingers crossed that I did not miss anything. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
