blocked_lock_lock and file_lock_lglock are used to protect file_lock's
fl_link, fl_block, fl_next, blocked_hash and the percpu
file_lock_list.
Let's use blocked_lock_lock only to protect blocked_hash since it is a
global lock.
Whenever we insert a new lock we are going to grab besides the
flc_lock also the corresponding file_lock_lglock. The global
blocked_lock_lock is only used when blocked_hash is involved.
Since we already use fl_link_cpu to remember which percpu
file_lock_list is referencing to a blocker we just going to use it as
well for all waiters.
Note fl_list is protected by flc_lock. It's easy to get confused...
Signed-off-by: Daniel Wagner <daniel.wagner@xxxxxxxxxxxx>
Cc: Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
---
fs/locks.c | 72 ++++++++++++++++++++++++++++++++++----------------------------
1 file changed, 39 insertions(+), 33 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 0c37d68..661e58b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -162,6 +162,20 @@ int lease_break_time = 45;
* keep a list on each CPU, with each list protected by its own spinlock via
* the file_lock_lglock. Note that alterations to the list also require that
* the relevant flc_lock is held.
+ *
+ * In addition, it also protects the fl->fl_block list, and the fl->fl_next
+ * pointer for file_lock structures that are acting as lock requests (in
+ * contrast to those that are acting as records of acquired locks).
+ *
+ * file_lock structures acting as lock requests (waiters) use the same
+ * spinlock as the those acting as lock holder (blocker). E.g. the
+ * blocker is initially added to the file_lock_list living on CPU 0,
+ * all waiters on that blocker are serialized via CPU 0 (see
+ * fl_link_cpu usage).
+ *
+ * In particular, adding an entry to the fl_block list requires that you hold
+ * both the flc_lock and the blocked_lock_lock (acquired in that order).
+ * Deleting an entry from the list however only requires the file_lock_gllock.
*/
DEFINE_STATIC_LGLOCK(file_lock_lglock);
static DEFINE_PER_CPU(struct hlist_head, file_lock_list);
@@ -183,19 +197,6 @@ static DEFINE_HASHTABLE(blocked_hash, BLOCKED_HASH_BITS);
/*
* This lock protects the blocked_hash. Generally, if you're accessing it, you
* want to be holding this lock.
- *
- * In addition, it also protects the fl->fl_block list, and the fl->fl_next
- * pointer for file_lock structures that are acting as lock requests (in
- * contrast to those that are acting as records of acquired locks).
- *
- * Note that when we acquire this lock in order to change the above fields,
- * we often hold the flc_lock as well. In certain cases, when reading the fields
- * protected by this lock, we can skip acquiring it iff we already hold the
- * flc_lock.
- *
- * In particular, adding an entry to the fl_block list requires that you hold
- * both the flc_lock and the blocked_lock_lock (acquired in that order).
- * Deleting an entry from the list however only requires the file_lock_lock.
*/
static DEFINE_SPINLOCK(blocked_lock_lock);
@@ -607,7 +608,7 @@ static void locks_delete_global_blocked(struct file_lock *waiter)
/* Remove waiter from blocker's block list.
* When blocker ends up pointing to itself then the list is empty.
*
- * Must be called with blocked_lock_lock held.
+ * Must be called with file_lock_lglock held.
*/
static void __locks_delete_block(struct file_lock *waiter)
{
@@ -617,7 +618,7 @@ static void __locks_delete_block(struct file_lock *waiter)
/* Posix block variant of __locks_delete_block.
*
- * Must be called with blocked_lock_lock held.
+ * Must be called with file_lock_lglock held.
*/
static void __locks_delete_posix_block(struct file_lock *waiter)
{
@@ -627,16 +628,18 @@ static void __locks_delete_posix_block(struct file_lock *waiter)
static void locks_delete_block(struct file_lock *waiter)
{
- spin_lock(&blocked_lock_lock);
+ lg_local_lock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
__locks_delete_block(waiter);
- spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
}
static void locks_delete_posix_block(struct file_lock *waiter)
{
+ lg_local_lock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
spin_lock(&blocked_lock_lock);
__locks_delete_posix_block(waiter);
spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
}
/* Insert waiter into blocker's block list.
@@ -644,22 +647,23 @@ static void locks_delete_posix_block(struct file_lock *waiter)
* the order they blocked. The documentation doesn't require this but
* it seems like the reasonable thing to do.
*
- * Must be called with both the flc_lock and blocked_lock_lock held. The
- * fl_block list itself is protected by the blocked_lock_lock, but by ensuring
+ * Must be called with both the flc_lock and file_lock_lglock held. The
+ * fl_block list itself is protected by the file_lock_lglock, but by ensuring
* that the flc_lock is also held on insertions we can avoid taking the
- * blocked_lock_lock in some cases when we see that the fl_block list is empty.
+ * file_lock_lglock in some cases when we see that the fl_block list is empty.
*/
static void __locks_insert_block(struct file_lock *blocker,
struct file_lock *waiter)
{
BUG_ON(!list_empty(&waiter->fl_block));
+ waiter->fl_link_cpu = blocker->fl_link_cpu;
waiter->fl_next = blocker;
list_add_tail(&waiter->fl_block, &blocker->fl_block);
}
/* Posix block variant of __locks_insert_block.
*
- * Must be called with flc_lock and blocked_lock_lock held.
+ * Must be called with flc_lock and file_lock_lglock held.
*/
static void __locks_insert_posix_block(struct file_lock *blocker,
struct file_lock *waiter)
@@ -672,9 +676,9 @@ static void __locks_insert_posix_block(struct file_lock *blocker,
static void locks_insert_block(struct file_lock *blocker,
struct file_lock *waiter)
{
- spin_lock(&blocked_lock_lock);
+ lg_local_lock_cpu(&file_lock_lglock, blocker->fl_link_cpu);
__locks_insert_block(blocker, waiter);
- spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, blocker->fl_link_cpu);
}
/*
@@ -685,31 +689,33 @@ static void locks_insert_block(struct file_lock *blocker,
static void locks_wake_up_blocks(struct file_lock *blocker)
{
/*
- * Avoid taking global lock if list is empty. This is safe since new
+ * Avoid taking lock if list is empty. This is safe since new
* blocked requests are only added to the list under the flc_lock, and
* the flc_lock is always held here. Note that removal from the fl_block
* list does not require the flc_lock, so we must recheck list_empty()
- * after acquiring the blocked_lock_lock.
+ * after acquiring the file_lock_lglock.
*/
if (list_empty(&blocker->fl_block))
return;
- spin_lock(&blocked_lock_lock);
+ lg_local_lock_cpu(&file_lock_lglock, blocker->fl_link_cpu);
while (!list_empty(&blocker->fl_block)) {
struct file_lock *waiter;
waiter = list_first_entry(&blocker->fl_block,
struct file_lock, fl_block);
- if (IS_POSIX(blocker) && !IS_OFDLCK(blocker))
+ if (IS_POSIX(blocker) && !IS_OFDLCK(blocker)) {
+ spin_lock(&blocked_lock_lock);
__locks_delete_posix_block(waiter);
- else
+ spin_unlock(&blocked_lock_lock);
+ } else
__locks_delete_block(waiter);
if (waiter->fl_lmops && waiter->fl_lmops->lm_notify)
waiter->fl_lmops->lm_notify(waiter);
else
wake_up(&waiter->fl_wait);
}
- spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, blocker->fl_link_cpu);
}
static void
@@ -1010,12 +1016,14 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
* locks list must be done while holding the same lock!
*/
error = -EDEADLK;
+ lg_local_lock_cpu(&file_lock_lglock, fl->fl_link_cpu);
spin_lock(&blocked_lock_lock);
if (likely(!posix_locks_deadlock(request, fl))) {
error = FILE_LOCK_DEFERRED;
__locks_insert_posix_block(fl, request);
}
spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, fl->fl_link_cpu);
goto out;
}
}
@@ -2497,12 +2505,14 @@ posix_unblock_lock(struct file_lock *waiter)
{
int status = 0;
+ lg_local_lock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
spin_lock(&blocked_lock_lock);
if (waiter->fl_next)
__locks_delete_posix_block(waiter);
else
status = -ENOENT;
spin_unlock(&blocked_lock_lock);
+ lg_local_unlock_cpu(&file_lock_lglock, waiter->fl_link_cpu);
return status;
}
EXPORT_SYMBOL(posix_unblock_lock);
@@ -2629,13 +2639,11 @@ static int locks_show(struct seq_file *f, void *v)
}
static void *locks_start(struct seq_file *f, loff_t *pos)
- __acquires(&blocked_lock_lock)
{
struct locks_iterator *iter = f->private;
iter->li_pos = *pos + 1;
lg_global_lock(&file_lock_lglock);
- spin_lock(&blocked_lock_lock);
return seq_hlist_start_percpu(&file_lock_list, &iter->li_cpu, *pos);
}
@@ -2648,9 +2656,7 @@ static void *locks_next(struct seq_file *f, void *v, loff_t *pos)
}
static void locks_stop(struct seq_file *f, void *v)
- __releases(&blocked_lock_lock)
{
- spin_unlock(&blocked_lock_lock);
lg_global_unlock(&file_lock_lglock);
}
--
2.1.0
--
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
