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

[Jens Axboe <axboe@kernel.dk>]

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@kernel.dk> 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