Re: [PATCH 1/2] fs: add support for LOOKUP_NONBLOCK

[Al Viro <viro@zeniv.linux.org.uk>]

On Thu, Dec 10, 2020 at 02:06:39PM -0700, Jens Axboe wrote:
> On 12/10/20 1:53 PM, Linus Torvalds wrote:
> > On Thu, Dec 10, 2020 at 12:01 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> io_uring always punts opens to async context, since there's no control
> >> over whether the lookup blocks or not. Add LOOKUP_NONBLOCK to support
> >> just doing the fast RCU based lookups, which we know will not block. If
> >> we can do a cached path resolution of the filename, then we don't have
> >> to always punt lookups for a worker.
> > 
> > Ok, this looks much better to me just from the name change.
> > 
> > Half of the patch is admittedly just to make sure it now returns the
> > right error from unlazy_walk (rather than knowing it's always
> > -ECHILD), and that could be its own thing, but I'm not sure it's even
> > worth splitting up. The only reason to do it would be to perhaps make
> > it really clear which part is the actual change, and which is just
> > that error handling cleanup.
> > 
> > So it looks fine to me, but I will leave this all to Al.
> 
> I did consider doing a prep patch just making the error handling clearer
> and get rid of the -ECHILD assumption, since it's pretty odd and not
> even something I'd expect to see in there. Al, do you want a prep patch
> to do that to make the change simpler/cleaner?

No, I do not.  Why bother returning anything other than -ECHILD, when
you can just have path_init() treat you flag sans LOOKUP_RCU as "fail
with -EAGAIN now" and be done with that?

What's the point propagating that thing when we are going to call the
non-RCU variant next if we get -ECHILD?

And that still doesn't answer the questions about the difference between
->d_revalidate() and ->get_link() (for the latter you keep the call in
RCU mode, for the former you generate that -EAGAIN crap).  Or between
->d_revalidate() and ->permission(), for that matter.

Finally, I really wonder what is that for; if you are in conditions when
you really don't want to risk going to sleep, you do *NOT* want to
do mnt_want_write().  Or ->open().  Or truncate().  Or, for Cthulhu
sake, IMA hash calculation.

So how hard are your "we don't want to block here" requirements?  Because
the stuff you do after complete_walk() can easily be much longer than
everything else.