lkml.org 
[lkml]   [2015]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL] please pull file-locking related changes for v3.20
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


\
 
 \ /
  Last update: 2015-02-17 21:21    [W:0.053 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site