Re: Buffered I/O broken on s390x with page faults disabled (gfs2)

[David Hildenbrand <david@redhat.com>]

On 08.03.22 20:27, Linus Torvalds wrote:
> On Tue, Mar 8, 2022 at 9:40 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Hmm. The futex code actually uses "fixup_user_fault()" for this case.
>> Maybe fault_in_safe_writeable() should do that?
> 
> .. paging all the bits back in, I'm reminded that one of the worries
> was "what about large areas".
> 
> But I really think that the solution should be that we limit the size
> of fault_in_safe_writeable() to just a couple of pages.
> 
> Even if you were to fault in gigabytes, page-out can undo it anyway,
> so there is no semantic reason why that function should ever do more
> than a few pages to make sure. There's already even a comment about
> how there's no guarantee that the pages will stay.
> 
> Side note: the current GUP-based fault_in_safe_writeable() is buggy in
> another way anyway: it doesn't work right for stack extending
> accesses.
> 
> So I think the fix for this all might be something like the attached
> (TOTALLY UNTESTED)!
> 
> Comments? Andreas, mind (carefully - maybe it is totally broken and
> does unspeakable acts to your pets) testing this?

I'm late to the party, I agree with the "stack extending accesses" issue
and that using fixup_user_fault() looks "cleaner" than FOLL_TOUCH.


I'm just going to point out that fixup_user_fault() on a per-page basis
is sub-optimal, especially when dealing with something that's PMD- or
PUD-mapped (THP, hugetlb, ...). In contrast, GUP is optimized for that.

So that might be something interesting to look into optimizing in the
future, if relevant in pactice. Not sure how we could return that
information the best way to the caller ("the currently faulted
in/present page ends at address X").

For the time being, the idea LGTM.

-- 
Thanks,

David / dhildenb