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

[Andreas Gruenbacher <agruenba@redhat.com>]

On Thu, Mar 10, 2022 at 6:13 PM David Hildenbrand <david@redhat.com> wrote:
>
> 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 practice. Not sure how we could return that
> information the best way to the caller ("the currently faulted
> in/present page ends at address X").

Yes, this applies to fault_in_iov_iter_readable() as well, as it is
based on fault_in_readable(). It's probably not a super urgent
optimization as the buffers faulted in are immediately accessed.

Thanks,
Andreas

> For the time being, the idea LGTM.
>
> --
> Thanks,
>
> David / dhildenb
>