[PATCH] bcache: lock in btree_flush_write() to avoid races

Subsystems: bcache (block layer cache), the rest

STALE3112d

3 messages, 3 authors, 2018-01-24 · open the first message on its own page

[PATCH] bcache: lock in btree_flush_write() to avoid races

From: <hidden>
Date: 2018-01-24 06:52:57 

From: Tang Junhui <redacted>

In btree_flush_write(), two places need to take a locker to
avoid races:

Firstly, we need take rcu read locker to protect the bucket_hash
traverse, since hlist_for_each_entry_rcu() must be called under
the protection of rcu read locker.

Secondly, we need take b->write_lock locker to protect journal
of the btree node, otherwise, the btree node may have been
written, and the journal have been assign to NULL.

Signed-off-by: Tang Junhui <redacted>
---
 drivers/md/bcache/journal.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 02a98dd..505f9f3 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -375,7 +375,9 @@ static void btree_flush_write(struct cache_set *c)
 retry:
 	best = NULL;
 
-	for_each_cached_btree(b, c, i)
+	rcu_read_lock();
+	for_each_cached_btree(b, c, i) {
+		mutex_lock(&b->write_lock);
 		if (btree_current_write(b)->journal) {
 			if (!best)
 				best = b;
@@ -385,6 +387,9 @@ static void btree_flush_write(struct cache_set *c)
 				best = b;
 			}
 		}
+		mutex_unlock(&b->write_lock);
+	}
+	rcu_read_unlock();
 
 	b = best;
 	if (b) {
-- 
1.8.3.1

Re: [PATCH] bcache: lock in btree_flush_write() to avoid races

From: Coly Li <hidden>
Date: 2018-01-24 09:13:39 

On 24/01/2018 2:54 PM, tang.junhui@zte.com.cn wrote:
quoted hunk
From: Tang Junhui <redacted>

In btree_flush_write(), two places need to take a locker to
avoid races:

Firstly, we need take rcu read locker to protect the bucket_hash
traverse, since hlist_for_each_entry_rcu() must be called under
the protection of rcu read locker.

Secondly, we need take b->write_lock locker to protect journal
of the btree node, otherwise, the btree node may have been
written, and the journal have been assign to NULL.

Signed-off-by: Tang Junhui <redacted>
---
 drivers/md/bcache/journal.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 02a98dd..505f9f3 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -375,7 +375,9 @@ static void btree_flush_write(struct cache_set *c)
 retry:
 	best = NULL;
 
-	for_each_cached_btree(b, c, i)
+	rcu_read_lock();
+	for_each_cached_btree(b, c, i) {
+		mutex_lock(&b->write_lock);
 		if (btree_current_write(b)->journal) {
 			if (!best)
 				best = b;
@@ -385,6 +387,9 @@ static void btree_flush_write(struct cache_set *c)
 				best = b;
 			}
 		}
+		mutex_unlock(&b->write_lock);
+	}
+	rcu_read_unlock();
 
 	b = best;
 	if (b) {
Hi Junhui,

Do you run into some real problem ? The patch looks good to me at first
glance, but I need time to dig it more before I provide my response.

Thanks.

Coly Li

Re: [PATCH] bcache: lock in btree_flush_write() to avoid races

From: Nikolay Borisov <hidden>
Date: 2018-01-24 10:30:53 


On 24.01.2018 08:54, tang.junhui@zte.com.cn wrote:
quoted hunk
From: Tang Junhui <redacted>

In btree_flush_write(), two places need to take a locker to
avoid races:

Firstly, we need take rcu read locker to protect the bucket_hash
traverse, since hlist_for_each_entry_rcu() must be called under
the protection of rcu read locker.

Secondly, we need take b->write_lock locker to protect journal
of the btree node, otherwise, the btree node may have been
written, and the journal have been assign to NULL.

Signed-off-by: Tang Junhui <redacted>
---
 drivers/md/bcache/journal.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c
index 02a98dd..505f9f3 100644
--- a/drivers/md/bcache/journal.c
+++ b/drivers/md/bcache/journal.c
@@ -375,7 +375,9 @@ static void btree_flush_write(struct cache_set *c)
 retry:
 	best = NULL;
 
-	for_each_cached_btree(b, c, i)
+	rcu_read_lock();
+	for_each_cached_btree(b, c, i) {
+		mutex_lock(&b->write_lock);

You can't sleep in rcu read critical section, yet here you take mutex
which can sleep under rcu_read_lock.
quoted hunk
 		if (btree_current_write(b)->journal) {
 			if (!best)
 				best = b;
@@ -385,6 +387,9 @@ static void btree_flush_write(struct cache_set *c)
 				best = b;
 			}
 		}
+		mutex_unlock(&b->write_lock);
+	}
+	rcu_read_unlock();
 
 	b = best;
 	if (b) {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help