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@xxxxxxxxx> 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.
