Subject: Re: Linux 5.1-rc5

From: Martin Schwidefsky <[email-masked]>
Date: Thu, 18 Apr 2019 10:02:18 +0200

On Wed, 17 Apr 2019 09:57:01 -0700
Linus Torvalds <[email-masked]> wrote:

> On Wed, Apr 17, 2019 at 1:02 AM Martin Schwidefsky
> <[email-masked]> 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.