Date:   Mon, 27 Dec 2021 18:17:49 +0900
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     Vasily Averin <vvs@virtuozzo.com>
Cc:     Eric Van Hensbergen <ericvh@gmail.com>,
        Latchesar Ionkov <lucho@ionkov.net>, kernel@openvz.org,
        v9fs-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        "J. Bruce Fields" <bfields@fieldses.org>
Subject: Re: [PATCH] v9fs: handle async processing of F_SETLK with FL_SLEEP
 flag
Message-ID: <YcmEvWm5dB4/d224@codewreck.org>
References: <YcXfm6U/6+Xmv7be@codewreck.org>
 <5ec8d1c6-e410-e297-0d3a-037c7de30b6b@virtuozzo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <5ec8d1c6-e410-e297-0d3a-037c7de30b6b@virtuozzo.com>
Precedence: bulk

Vasily Averin wrote on Mon, Dec 27, 2021 at 10:42:48AM +0300:
> > Should we also check fl->fl_flags & (FL_POSIX|FL_FLOCK) like
> > locks_lock_file_wait does, to call either posix_lock_file or ... there
> > doesn't seem to be an exported flock_lock_file equivalent in the other
> > case, so back to wait variant there?
> > (or rephrasing the question, what happens if the lock is a FL_FLOCK lock
> > and we call posix_lock_file on it? Or are we guaranted that if FL_SLEEP
> > is set we're about posix locks?)
> 
> SETLK with FL_SLEEP flag can be set by kernel export threads for posix locks only.

I see.
Would it make sense to add a WARN_ON or similar in case that ever
changes in the future?

I'd be more comfortable with one given it'd be quite a sneaky bug when
it does (locks will still appear to work, just wrong kind of locks...).
I assume that if it ever does all filesystems will be reviewed for
it... But sometimes it's best to make sure.

> > Well, it depends on what you have in mind with blocking.
> > A synchronous RPC to some server which might never reply if it doesn't
> > feel like it (bug or whatever) is very much blocking in my opinion.
> 
> The main goal is to avoid deadlock of server threads.
> It is acceptable to sleep or process such a request for a very long time,
> however it is unacceptable to wait for another command to remove our lock,
> because there can be no available working threads to process this command:
> all of them can be busy by processing of blocking locks. 

Ok, that makes sense to me.

I'm happy with the current patch except for my paranoia on that fcntl
lock check, let me know what you think about it and I'll apply either
this or a new version.

Thanks!
-- 
Dominique