# \[PATCH\] bcache: lock in btree\_flush\_write() to avoid races - Subsystems: BCACHE (BLOCK LAYER CACHE), THE REST - Maintainers: Coly Li, Kent Overstreet, Linus Torvalds - List: linux-block - 3 messages, 3 authors - Page: https://ratatoskr.run/linux-block/2018/01/6378317/t ## \[PATCH\] bcache: lock in btree\_flush\_write() to avoid races - From: \ - Date: 2018-01-24 06:52:57 - Message: https://ratatoskr.run/linux-block/2018/01/6378317.md ```text From: Tang Junhui 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 --- 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 \ - Date: 2018-01-24 09:13:39 - Message: https://ratatoskr.run/linux-block/2018/01/6378316.md ```text On 24/01/2018 2:54 PM, tang.junhui@zte.com.cn wrote: > From: Tang Junhui > > 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 > --- > 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 \ - Date: 2018-01-24 10:30:53 - Message: https://ratatoskr.run/linux-block/2018/01/6378313.md ```text On 24.01.2018 08:54, tang.junhui@zte.com.cn wrote: > From: Tang Junhui > > 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 > --- > 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. > 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) { > ```