On 24/01/2018 5:45 PM, tang.junhui@xxxxxxxxxx wrote: > From: Tang Junhui <tang.junhui@xxxxxxxxxx> > >> On 24/01/2018 2:54 PM, tang.junhui@xxxxxxxxxx wrote: >>> From: Tang Junhui <tang.junhui@xxxxxxxxxx> >>> >>> 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 <tang.junhui@xxxxxxxxxx> >>> --- >>> 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. > > No real problem run out, I find it by code reading, I think we'd better > lock it for safety. I see, then I will take it as a lower priority and finish my current task firstly. I will provide my review comment 1 week later, if no one else does it then. Thanks. Coly Li