On Fri, 28 Mar 2014 17:43:22 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote: > On Wed, Mar 19, 2014 at 04:45:57PM -0400, Jeff Layton wrote: > > It's not really feasible to do deadlock detection with FL_FILE_PVT > > locks since they aren't owned by a single task, per-se. Deadlock > > detection also tends to be rather expensive so just skip it for > > these sorts of locks. > > Yay! > > > Also, add a FIXME comment about adding more limited deadlock detection > > that just applies to ro -> rw upgrades, per Andy's request. > > > > Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/locks.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/fs/locks.c b/fs/locks.c > > index f8cd6d7de161..8c5bc07c360f 100644 > > --- a/fs/locks.c > > +++ b/fs/locks.c > > @@ -564,7 +564,7 @@ static void __locks_insert_block(struct file_lock *blocker, > > BUG_ON(!list_empty(&waiter->fl_block)); > > waiter->fl_next = blocker; > > list_add_tail(&waiter->fl_block, &blocker->fl_block); > > - if (IS_POSIX(blocker)) > > + if (IS_POSIX(blocker) && !IS_FILE_PVT(blocker)) > > locks_insert_global_blocked(waiter); > > } > > > > @@ -757,8 +757,16 @@ EXPORT_SYMBOL(posix_test_lock); > > * Note: the above assumption may not be true when handling lock > > * requests from a broken NFS client. It may also fail in the presence > > * of tasks (such as posix threads) sharing the same open file table. > > - * > > * To handle those cases, we just bail out after a few iterations. > > + * > > + * For FL_FILE_PVT locks, the owner is the filp, not the files_struct. > > + * Because the owner is not even nominally tied to a thread of > > + * execution, the deadlock detection below can't reasonably work well. Just > > + * skip it for those. > > + * > > + * In principle, we could do a more limited deadlock detection on FL_FILE_PVT > > + * locks that just checks for the case where two tasks are attempting to > > + * upgrade from read to write locks on the same inode. > > */ > > > > #define MAX_DEADLK_ITERATIONS 10 > > @@ -781,6 +789,13 @@ static int posix_locks_deadlock(struct file_lock *caller_fl, > > { > > int i = 0; > > > > + /* > > + * This deadlock detector can't reasonably detect deadlocks with > > + * FL_FILE_PVT locks, since they aren't owned by a process, per-se. > > + */ > > + if (IS_FILE_PVT(caller_fl)) > > + return 0; > > + > > This takes care of deadlock detection at the time that you apply a > file_private lock. What happens when you're doing deadlock detection > before applying a traditional posix lock and happen to run across a > file_private lock? > > Hm, I guess the posix_same_owner() always fails in that case? > > OK, ACK. > > --b. > Note that we don't ever insert FP locks into the hashtable, so they're aren't taken into account at all for deadlock detection. FWIW, as a later cleanup, I'd also like to ensure that we never take the global spinlock for fplocks as well, but I think that can wait until after this set has been merged. > > while ((block_fl = what_owner_is_waiting_for(block_fl))) { > > if (i++ > MAX_DEADLK_ITERATIONS) > > return 0; > > -- > > 1.8.5.3 > > -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html