On 12/11/20 10:29 AM, Al Viro wrote: > On Fri, Dec 11, 2020 at 09:21:20AM -0800, Linus Torvalds wrote: >> On Fri, Dec 11, 2020 at 7:57 AM Jens Axboe <axboe@xxxxxxxxx> wrote: >>> >>> On 12/10/20 7:35 PM, Al Viro wrote: >>>> _IF_ for some theoretical exercise you want to do "lookup without dropping >>>> out of RCU", just add a flag that has unlazy_walk() fail. With -ECHILD. >>>> Strip it away in complete_walk() and have path_init() with that flag >>>> and without LOOKUP_RCU fail with -EAGAIN. All there is to it. >>> >>> Thanks Al, that makes for an easier implementation. I like that suggestion, >>> boils it down to just three hunks (see below). >> >> Ooh. Yes, very nice. > > Except for the wrong order in path_init() - the check should go _before_ > if (!*s) > flags &= ~LOOKUP_RCU; > for obvious reasons. Oops yes, I fixed that up. > Again, that part is trivial - what to do with > do_open()/open_last_lookups() is where the nastiness will be. > Basically, it makes sure we bail out in cases when lookup itself > would've blocked, but it does *not* bail out when equally heavy (and > unlikely) blocking sources hit past the complete_walk(). Which makes > it rather useless for the caller, unless we get logics added to that > part as well. And _that_ I want to see before we commit to anything. A few items can be handled by just disallowing them upfront - like O_TRUNC with RESOLVE_NONBLOCK. I'd prefer to do that instead of sprinkling trylock variants in places where they kind of end up being nonsensical anyway (eg O_TRUNC with RESOLVE_NONBLOCK just doesn't make sense). Outside of that, doesn't look like to me that do_open() needs any further massaging. open_last_lookups() needs to deal with non-blocking mnt_want_write(), but that looks pretty trivial, and trylock on the inode. So all seems pretty doable. Which makes me think I must be missing something? -- Jens Axboe
