Re: [PATCH] filemap: Handle error return from __filemap_get_folio()

[Peter Xu <peterx@redhat.com>]

On Wed, May 10, 2023 at 04:44:59PM -0500, Linus Torvalds wrote:
> On Wed, May 10, 2023 at 4:33 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > We'd still keep the RETRY bit as a "this did not complete, you need to
> > retry", but at least the whole secondary meaning of "oh, and if it
> > isn't set, you need to release the lock you took" would go away.
> 
> "unless VM_FAULT_COMPLETED is set, in which case everything was fine,
> and you shouldn't release the lock because we already released it".
> 
> I completely forgot about that wart that came in last year.
> 
> I think that if we made handle_mm_fault() always unlock, that thing
> would go away entirely, since "0" would now just mean the same thing.
> 
> Is there really any case that *wants* to keep the mmap lock held, and
> couldn't just always re-take it if it needs to do another page
> (possibly retry, but the retry case obviously already has that issue)?

FAULT_FLAG_RETRY_NOWAIT?

> Certainly nothing wants the vma lock, so it's only the "mmap_sem" case
> that would be an issue.

You're definitely right that the gup path is broken which I didn't notice
when reading...  I know I shouldn't review such a still slightly involved
patch during travel. :(

I still think maybe we have chance to generalize at least the fault path,
I'd still start with something like having just 2-3 archs having a shared
routine handle only some part of the fault path (I remember there was a
previous discussion previously, but I didn't follow up much from there..).

So even if we still need duplicates over many archs, we'll start to have
something we can use as a baseline in fault path.  Does it sound a sane
thing to consider as a start, or maybe not?

The other question - considering RETRY_NOWAIT being there - do we still
want to have something like what Johannes proposed first to fix the problem
(with all arch and gup fixed)?  I'd think yes, but I could missed something.

Thanks,

-- 
Peter Xu