linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce
@ 2020-04-15 11:42 Zhiqiang Liu
  2020-04-20  6:55 ` Zhiqiang Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Zhiqiang Liu @ 2020-04-15 11:42 UTC (permalink / raw)
  To: colyli, kmo, linux-bcache, linux-kernel
  Cc: wubo (T), Mingfangsen, Yanxiaodan, linfeilong

From: Zhiqiang Liu <liuzhiqiang26@huawei.com>

coccicheck reports:
  drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417

After obtaining new_nodes[i]->write_lock, we may go to out_nocoalesce
tag without releasing lock. Here, we add a new tag
'out_unlock_nocoalesce' before out_nocoalesce tag to release
new_nodes[i]->write_lock.

Fixes: 2a285686c1 ("bcache: btree locking rework")
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
 drivers/md/bcache/btree.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index fa872df4e770..cad8b0b97e33 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1447,7 +1447,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
 			if (__set_blocks(n1, n1->keys + n2->keys,
 					 block_bytes(b->c)) >
 			    btree_blocks(new_nodes[i]))
-				goto out_nocoalesce;
+				goto out_unlock_nocoalesce;

 			keys = n2->keys;
 			/* Take the key of the node we're getting rid of */
@@ -1476,7 +1476,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,

 		if (__bch_keylist_realloc(&keylist,
 					  bkey_u64s(&new_nodes[i]->key)))
-			goto out_nocoalesce;
+			goto out_unlock_nocoalesce;

 		bch_btree_node_write(new_nodes[i], &cl);
 		bch_keylist_add(&keylist, &new_nodes[i]->key);
@@ -1522,6 +1522,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
 	/* Invalidated our iterator */
 	return -EINTR;

+out_unlock_nocoalesce:
+	for (i = 0; i < nodes; i++)
+		mutex_unlock(&new_nodes[i]->write_lock);
+
 out_nocoalesce:
 	closure_sync(&cl);

-- 
2.19.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce
  2020-04-15 11:42 bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce Zhiqiang Liu
@ 2020-04-20  6:55 ` Zhiqiang Liu
  2020-04-20  7:10   ` Coly Li
  0 siblings, 1 reply; 4+ messages in thread
From: Zhiqiang Liu @ 2020-04-20  6:55 UTC (permalink / raw)
  To: colyli, kmo, linux-bcache, linux-kernel
  Cc: wubo (T), Mingfangsen, Yanxiaodan, linfeilong

friendly ping...

On 2020/4/15 19:42, Zhiqiang Liu wrote:
> From: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> 
> coccicheck reports:
>   drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417
> 
> After obtaining new_nodes[i]->write_lock, we may go to out_nocoalesce
> tag without releasing lock. Here, we add a new tag
> 'out_unlock_nocoalesce' before out_nocoalesce tag to release
> new_nodes[i]->write_lock.
> 
> Fixes: 2a285686c1 ("bcache: btree locking rework")
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
>  drivers/md/bcache/btree.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index fa872df4e770..cad8b0b97e33 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -1447,7 +1447,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>  			if (__set_blocks(n1, n1->keys + n2->keys,
>  					 block_bytes(b->c)) >
>  			    btree_blocks(new_nodes[i]))
> -				goto out_nocoalesce;
> +				goto out_unlock_nocoalesce;
> 
>  			keys = n2->keys;
>  			/* Take the key of the node we're getting rid of */
> @@ -1476,7 +1476,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
> 
>  		if (__bch_keylist_realloc(&keylist,
>  					  bkey_u64s(&new_nodes[i]->key)))
> -			goto out_nocoalesce;
> +			goto out_unlock_nocoalesce;
> 
>  		bch_btree_node_write(new_nodes[i], &cl);
>  		bch_keylist_add(&keylist, &new_nodes[i]->key);
> @@ -1522,6 +1522,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>  	/* Invalidated our iterator */
>  	return -EINTR;
> 
> +out_unlock_nocoalesce:
> +	for (i = 0; i < nodes; i++)
> +		mutex_unlock(&new_nodes[i]->write_lock);
> +
>  out_nocoalesce:
>  	closure_sync(&cl);
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce
  2020-04-20  6:55 ` Zhiqiang Liu
@ 2020-04-20  7:10   ` Coly Li
  2020-04-20  7:48     ` Zhiqiang Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Coly Li @ 2020-04-20  7:10 UTC (permalink / raw)
  To: Zhiqiang Liu
  Cc: kmo, linux-bcache, linux-kernel, wubo (T),
	Mingfangsen, Yanxiaodan, linfeilong

On 2020/4/20 14:55, Zhiqiang Liu wrote:
> friendly ping...
> 

The patch is good. Just it would be better if the commit log can be more
accurate and understandable for other readers who are not very familiar
with bcache code.

Thanks.


Coly Li


> On 2020/4/15 19:42, Zhiqiang Liu wrote:
>> From: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>>
>> coccicheck reports:
>>   drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417
>>
>> After obtaining new_nodes[i]->write_lock, we may go to out_nocoalesce
>> tag without releasing lock. Here, we add a new tag
>> 'out_unlock_nocoalesce' before out_nocoalesce tag to release
>> new_nodes[i]->write_lock.
>>
>> Fixes: 2a285686c1 ("bcache: btree locking rework")
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> ---
>>  drivers/md/bcache/btree.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
>> index fa872df4e770..cad8b0b97e33 100644
>> --- a/drivers/md/bcache/btree.c
>> +++ b/drivers/md/bcache/btree.c
>> @@ -1447,7 +1447,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>  			if (__set_blocks(n1, n1->keys + n2->keys,
>>  					 block_bytes(b->c)) >
>>  			    btree_blocks(new_nodes[i]))
>> -				goto out_nocoalesce;
>> +				goto out_unlock_nocoalesce;
>>
>>  			keys = n2->keys;
>>  			/* Take the key of the node we're getting rid of */
>> @@ -1476,7 +1476,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>
>>  		if (__bch_keylist_realloc(&keylist,
>>  					  bkey_u64s(&new_nodes[i]->key)))
>> -			goto out_nocoalesce;
>> +			goto out_unlock_nocoalesce;
>>
>>  		bch_btree_node_write(new_nodes[i], &cl);
>>  		bch_keylist_add(&keylist, &new_nodes[i]->key);
>> @@ -1522,6 +1522,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>  	/* Invalidated our iterator */
>>  	return -EINTR;
>>
>> +out_unlock_nocoalesce:
>> +	for (i = 0; i < nodes; i++)
>> +		mutex_unlock(&new_nodes[i]->write_lock);
>> +
>>  out_nocoalesce:
>>  	closure_sync(&cl);
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce
  2020-04-20  7:10   ` Coly Li
@ 2020-04-20  7:48     ` Zhiqiang Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Zhiqiang Liu @ 2020-04-20  7:48 UTC (permalink / raw)
  To: Coly Li
  Cc: kmo, linux-bcache, linux-kernel, wubo (T),
	Mingfangsen, Yanxiaodan, linfeilong



On 2020/4/20 15:10, Coly Li wrote:
> On 2020/4/20 14:55, Zhiqiang Liu wrote:
>> friendly ping...
>>
> 
> The patch is good. Just it would be better if the commit log can be more
> accurate and understandable for other readers who are not very familiar
> with bcache code.
> 
> Thanks.
> 
> 
> Coly Li

Thanks for your reply. I will try my best to rewrite the commit log.

Regards.
Zhiqiang Liu.

> 
> 
>> On 2020/4/15 19:42, Zhiqiang Liu wrote:
>>> From: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>>>
>>> coccicheck reports:
>>>   drivers/md//bcache/btree.c:1538:1-7: preceding lock on line 1417
>>>
>>> After obtaining new_nodes[i]->write_lock, we may go to out_nocoalesce
>>> tag without releasing lock. Here, we add a new tag
>>> 'out_unlock_nocoalesce' before out_nocoalesce tag to release
>>> new_nodes[i]->write_lock.
>>>
>>> Fixes: 2a285686c1 ("bcache: btree locking rework")
>>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>>> ---
>>>  drivers/md/bcache/btree.c | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
>>> index fa872df4e770..cad8b0b97e33 100644
>>> --- a/drivers/md/bcache/btree.c
>>> +++ b/drivers/md/bcache/btree.c
>>> @@ -1447,7 +1447,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>>  			if (__set_blocks(n1, n1->keys + n2->keys,
>>>  					 block_bytes(b->c)) >
>>>  			    btree_blocks(new_nodes[i]))
>>> -				goto out_nocoalesce;
>>> +				goto out_unlock_nocoalesce;
>>>
>>>  			keys = n2->keys;
>>>  			/* Take the key of the node we're getting rid of */
>>> @@ -1476,7 +1476,7 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>>
>>>  		if (__bch_keylist_realloc(&keylist,
>>>  					  bkey_u64s(&new_nodes[i]->key)))
>>> -			goto out_nocoalesce;
>>> +			goto out_unlock_nocoalesce;
>>>
>>>  		bch_btree_node_write(new_nodes[i], &cl);
>>>  		bch_keylist_add(&keylist, &new_nodes[i]->key);
>>> @@ -1522,6 +1522,10 @@ static int btree_gc_coalesce(struct btree *b, struct btree_op *op,
>>>  	/* Invalidated our iterator */
>>>  	return -EINTR;
>>>
>>> +out_unlock_nocoalesce:
>>> +	for (i = 0; i < nodes; i++)
>>> +		mutex_unlock(&new_nodes[i]->write_lock);
>>> +
>>>  out_nocoalesce:
>>>  	closure_sync(&cl);
>>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-04-20  7:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15 11:42 bcache: unlock new_nodes[i]->write_lock in btree_gc_coalesce Zhiqiang Liu
2020-04-20  6:55 ` Zhiqiang Liu
2020-04-20  7:10   ` Coly Li
2020-04-20  7:48     ` Zhiqiang Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).