Re: [GIT PULL] please pull file-locking related changes for v3.20

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Feb 17, 2015 at 11:41:40AM -0800, Linus Torvalds wrote:
> On Tue, Feb 17, 2015 at 11:27 AM, Jeff Layton <jlayton@poochiereds.net> wrote:
> >
> > What about this instead then?
> 
> No. Really.
> 
> > - leave the "drop the spinlock" thing in place in flock_lock_file for
> >   v3.20
> 
> No. The whole concept of "drop the lock in the middle" is *BROKEN*.
> It's seriously crap. It's not just a bug, it's a really fundamentally
> wrong thing to do.
> 
> > - change locks_remove_flock to just walk the list and delete any locks
> >   associated with the filp being closed
> 
> No. That's still wrong. You can have two people holding a write-lock.
> Seriously. That's *shit*.
> 
> The "drop the spinlock in the middle" must go. There's not even any
> reason for it. Just get rid of it. There can be no deadlock if you get
> rid of it, because
> 
>  - we hold the flc_lock over the whole event, so we can never see any
> half-way state
> 
>  - if we actually decide to sleep (due to conflicting locks) and
> return FILE_LOCK_DEFERRED, we will drop the lock before actually
> sleeping, so nobody else will be deadlocking on this file lock. So any
> *other* person who tries to do an upgrade will not sleep, because the
> pending upgrade will have moved to the blocking list (that whole
> "locks_insert_block" part.

Whoops, you're right, I was forgetting that wait happens up in
flock_lock_file_wait(), OK.

--b.

> Ergo, either we'll upgrade the lock (atomically, within flc_lock), or
> we will drop the lock (possibly moving it to the blocking list). I
> don't see a deadlock.
> 
> I think your (and mine - but mine had the more fundamental problem of
> never setting "old_fl" correctly at all) patch had a deadlock because
> you didn't actually remove the old lock when you returned
> FILE_LOCK_DEFERRED.
> 
> But I think the correct minimal patch is actually to just remove the
> "if (found)" statement.
> 
>                        Linus