forked from luck/tmp_suning_uos_patched
locks: remove conditional lock release in middle of flock_lock_file
As Linus pointed out: Say we have an existing flock, and now do a new one that conflicts. I see what looks like three separate bugs. - We go through the first loop, find a lock of another type, and delete it in preparation for replacing it - we *drop* the lock context spinlock. - BUG #1? So now there is no lock at all, and somebody can come in and see that unlocked state. Is that really valid? - another thread comes in while the first thread dropped the lock context lock, and wants to add its own lock. It doesn't see the deleted or pending locks, so it just adds it - the first thread gets the context spinlock again, and adds the lock that replaced the original - BUG #2? So now there are *two* locks on the thing, and the next time you do an unlock (or when you close the file), it will only remove/replace the first one. ...remove the "drop the spinlock" code in the middle of this function as it has always been suspicious. This should eliminate the potential race that can leave two locks for the same struct file on the list. He also pointed out another thing as a bug -- namely that you flock_lock_file removes the lock from the list unconditionally when doing a lock upgrade, without knowing whether it'll be able to set the new lock. Bruce pointed out that this is expected behavior and may help prevent certain deadlock situations. We may want to revisit that at some point, but it's probably best that we do so in the context of a different patchset. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com>
This commit is contained in:
parent
c4e136cda1
commit
267f112858
10
fs/locks.c
10
fs/locks.c
@ -901,16 +901,6 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
|
||||
goto out;
|
||||
}
|
||||
|
||||
/*
|
||||
* If a higher-priority process was blocked on the old file lock,
|
||||
* give it the opportunity to lock the file.
|
||||
*/
|
||||
if (found) {
|
||||
spin_unlock(&ctx->flc_lock);
|
||||
cond_resched();
|
||||
spin_lock(&ctx->flc_lock);
|
||||
}
|
||||
|
||||
find_conflict:
|
||||
list_for_each_entry(fl, &ctx->flc_flock, fl_list) {
|
||||
if (!flock_locks_conflict(request, fl))
|
||||
|
Loading…
Reference in New Issue
Block a user