linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: viro@zeniv.linux.org.uk, matthew@wil.cx, bfields@fieldses.org
Cc: dhowells@redhat.com, sage@inktank.com, smfrench@gmail.com,
	swhiteho@redhat.com, Trond.Myklebust@netapp.com,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-afs@lists.infradead.org, ceph-devel@vger.kernel.org,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	cluster-devel@redhat.com, linux-nfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, piastryyy@gmail.com
Subject: [PATCH v2 12/14] locks: give the blocked_hash its own spinlock
Date: Tue, 11 Jun 2013 07:09:06 -0400	[thread overview]
Message-ID: <1370948948-31784-13-git-send-email-jlayton@redhat.com> (raw)
In-Reply-To: <1370948948-31784-1-git-send-email-jlayton@redhat.com>

There's no reason we have to protect the blocked_hash and file_lock_list
with the same spinlock. With the tests I have, breaking it in two gives
a barely measurable performance benefit, but it seems reasonable to make
this locking as granular as possible.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 Documentation/filesystems/Locking |   16 ++++++++--------
 fs/locks.c                        |   25 +++++++++++++------------
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index ee351ac..8d8d040 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -359,20 +359,20 @@ prototypes:
 
 locking rules:
 
-			inode->i_lock	file_lock_lock	may block
-lm_compare_owner:	yes		maybe		no
-lm_owner_key		yes		yes		no
-lm_notify:		yes		no		no
-lm_grant:		no		no		no
-lm_break:		yes		no		no
-lm_change		yes		no		no
+			inode->i_lock	blocked_hash_lock	may block
+lm_compare_owner:	yes		maybe			no
+lm_owner_key		yes		yes			no
+lm_notify:		yes		no			no
+lm_grant:		no		no			no
+lm_break:		yes		no			no
+lm_change		yes		no			no
 
 	->lm_compare_owner and ->lm_owner_key are generally called with
 *an* inode->i_lock held. It may not be the i_lock of the inode
 associated with either file_lock argument! This is the case with deadlock
 detection, since the code has to chase down the owners of locks that may
 be entirely unrelated to the one on which the lock is being acquired.
-For deadlock detection however, the file_lock_lock is also held. The
+For deadlock detection however, the blocked_hash_lock is also held. The
 fact that these locks are held ensures that the file_locks do not
 disappear out from under you while doing the comparison or generating an
 owner key.
diff --git a/fs/locks.c b/fs/locks.c
index 11e7784..8124fc1 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -162,12 +162,11 @@ int lease_break_time = 45;
  */
 #define BLOCKED_HASH_BITS	7
 
+static DEFINE_SPINLOCK(blocked_hash_lock);
 static DEFINE_HASHTABLE(blocked_hash, BLOCKED_HASH_BITS);
 
-static HLIST_HEAD(file_lock_list);
-
-/* Protects the file_lock_list and the blocked_hash */
 static DEFINE_SPINLOCK(file_lock_lock);
+static HLIST_HEAD(file_lock_list);
 
 static struct kmem_cache *filelock_cache __read_mostly;
 
@@ -505,9 +504,9 @@ __locks_delete_global_blocked(struct file_lock *waiter)
 static inline void
 locks_delete_global_blocked(struct file_lock *waiter)
 {
-	spin_lock(&file_lock_lock);
+	spin_lock(&blocked_hash_lock);
 	__locks_delete_global_blocked(waiter);
-	spin_unlock(&file_lock_lock);
+	spin_unlock(&blocked_hash_lock);
 }
 
 static inline void
@@ -581,14 +580,14 @@ static void locks_wake_up_blocks(struct file_lock *blocker)
 
 /*
  * Wake up processes blocked waiting for blocker. In the FL_POSIX case, we must
- * also take the global file_lock_lock and dequeue it from the global blocked
- * list as we wake the processes.
+ * also take the global blocked_hash_lock and dequeue it from the global
+ * blocked list as we wake the processes.
  *
  * Must be called with the inode->i_lock of the blocker held!
  */
 static void locks_wake_up_posix_blocks(struct file_lock *blocker)
 {
-	spin_lock(&file_lock_lock);
+	spin_lock(&blocked_hash_lock);
 	while (!list_empty(&blocker->fl_block)) {
 		struct file_lock *waiter;
 
@@ -601,7 +600,7 @@ static void locks_wake_up_posix_blocks(struct file_lock *blocker)
 		else
 			wake_up(&waiter->fl_wait);
 	}
-	spin_unlock(&file_lock_lock);
+	spin_unlock(&blocked_hash_lock);
 }
 /* Insert file lock fl into an inode's lock list at the position indicated
  * by pos. At the same time add the lock to the global file lock list.
@@ -754,7 +753,7 @@ static struct file_lock *what_owner_is_waiting_for(struct file_lock *block_fl)
 	return NULL;
 }
 
-/* Must be called with the file_lock_lock held! */
+/* Must be called with the blocked_hash_lock held! */
 static int posix_locks_deadlock(struct file_lock *caller_fl,
 				struct file_lock *block_fl)
 {
@@ -898,13 +897,13 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			if (!(request->fl_flags & FL_SLEEP))
 				goto out;
 			error = -EDEADLK;
-			spin_lock(&file_lock_lock);
+			spin_lock(&blocked_hash_lock);
 			if (likely(!posix_locks_deadlock(request, fl))) {
 				error = FILE_LOCK_DEFERRED;
 				locks_insert_block(fl, request);
 				locks_insert_global_blocked(request);
 			}
-			spin_unlock(&file_lock_lock);
+			spin_unlock(&blocked_hash_lock);
 			goto out;
   		}
   	}
@@ -2309,10 +2308,12 @@ static int locks_show(struct seq_file *f, void *v)
 
 	lock_get_status(f, fl, *((loff_t *)f->private), "");
 
+	spin_lock(&blocked_hash_lock);
 	hash_for_each(blocked_hash, bkt, bfl, fl_link) {
 		if (bfl->fl_next == fl)
 			lock_get_status(f, bfl, *((loff_t *)f->private), " ->");
 	}
+	spin_unlock(&blocked_hash_lock);
 
 	return 0;
 }
-- 
1.7.1


  parent reply	other threads:[~2013-06-11 11:09 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 11:08 [PATCH v2 00/14] locks: scalability improvements for file locking Jeff Layton
2013-06-11 11:08 ` [PATCH v2 01/14] cifs: use posix_unblock_lock instead of locks_delete_block Jeff Layton
2013-06-11 11:08 ` [PATCH v2 02/14] locks: make generic_add_lease and generic_delete_lease static Jeff Layton
2013-06-11 11:08 ` [PATCH v2 03/14] locks: comment cleanups and clarifications Jeff Layton
2013-06-11 11:08 ` [PATCH v2 04/14] locks: make "added" in __posix_lock_file a bool Jeff Layton
2013-06-11 11:08 ` [PATCH v2 05/14] locks: encapsulate the fl_link list handling Jeff Layton
2013-06-11 11:09 ` [PATCH v2 06/14] locks: don't walk inode->i_flock list in locks_show Jeff Layton
2013-06-13 19:45   ` J. Bruce Fields
2013-06-13 20:26     ` Jeff Layton
     [not found]       ` <51BB040C.3050101@samba.org>
2013-06-15 11:05         ` Jeff Layton
2013-06-15 15:04           ` Simo
2013-06-11 11:09 ` [PATCH v2 07/14] locks: convert to i_lock to protect i_flock list Jeff Layton
2013-06-13 14:41   ` J. Bruce Fields
2013-06-13 15:09     ` Jeff Layton
2013-06-11 11:09 ` [PATCH v2 08/14] locks: ensure that deadlock detection is atomic with respect to blocked_list modification Jeff Layton
2013-06-11 11:09 ` [PATCH v2 09/14] locks: convert fl_link to a hlist_node Jeff Layton
2013-06-11 11:09 ` [PATCH v2 10/14] locks: turn the blocked_list into a hashtable Jeff Layton
2013-06-13 14:50   ` J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 11/14] locks: add a new "lm_owner_key" lock operation Jeff Layton
2013-06-13 15:00   ` J. Bruce Fields
2013-06-11 11:09 ` Jeff Layton [this message]
2013-06-13 15:02   ` [PATCH v2 12/14] locks: give the blocked_hash its own spinlock J. Bruce Fields
2013-06-13 15:18     ` Jeff Layton
2013-06-13 15:20       ` J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 13/14] seq_file: add seq_list_*_percpu helpers Jeff Layton
2013-06-13 15:27   ` J. Bruce Fields
2013-06-11 11:09 ` [PATCH v2 14/14] locks: move file_lock_list to a set of percpu hlist_heads and convert file_lock_lock to an lglock Jeff Layton
2013-06-13 15:37   ` J. Bruce Fields
2013-06-11 16:04 ` [PATCH v2 00/14] locks: scalability improvements for file locking J. Bruce Fields
2013-06-11 16:35   ` Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1370948948-31784-13-git-send-email-jlayton@redhat.com \
    --to=jlayton@redhat.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=cluster-devel@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=piastryyy@gmail.com \
    --cc=sage@inktank.com \
    --cc=samba-technical@lists.samba.org \
    --cc=smfrench@gmail.com \
    --cc=swhiteho@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).