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

[Coly Li <colyli@suse.de>]

On 24/01/2018 2:54 PM, tang.junhui@zte.com.cn wrote:
> From: Tang Junhui <tang.junhui@zte.com.cn>
> 
> 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@zte.com.cn>
> ---
>  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