[V2,2/4] Btrfs: don't get the lock when adding a csum into a ordered extent
diff mbox

Message ID 1389702712-26638-3-git-send-email-miaox@cn.fujitsu.com
State Accepted
Headers show

Commit Message

Miao Xie Jan. 14, 2014, 12:31 p.m. UTC
We are sure that:
- one ordered extent just has one csum calculation worker, and no one
  access the csum list during the csum calculation except the worker.
- we don't change the list and free the csum until no one reference
  to the ordered extent
So it is safe to add csum without the lock.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/ordered-data.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

Comments

Josef Bacik Jan. 28, 2014, 4 p.m. UTC | #1
On 01/14/2014 07:31 AM, Miao Xie wrote:
> We are sure that:
> - one ordered extent just has one csum calculation worker, and no one
>    access the csum list during the csum calculation except the worker.
> - we don't change the list and free the csum until no one reference
>    to the ordered extent
> So it is safe to add csum without the lock.
>
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> ---
>   fs/btrfs/ordered-data.c |    5 -----
>   1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index e4c3d56..396c6d1 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -280,16 +280,11 @@ void btrfs_add_ordered_sum(struct inode *inode,
>   			   struct btrfs_ordered_extent *entry,
>   			   struct btrfs_ordered_sum *sum)
>   {
> -	struct btrfs_ordered_inode_tree *tree;
> -
> -	tree = &BTRFS_I(inode)->ordered_tree;
> -	spin_lock_irq(&tree->lock);
>   	list_add_tail(&sum->list, &entry->list);
>   	WARN_ON(entry->csum_bytes_left < sum->len);
>   	entry->csum_bytes_left -= sum->len;
>   	if (entry->csum_bytes_left == 0)
>   		wake_up(&entry->wait);
> -	spin_unlock_irq(&tree->lock);
>   }
>   
>   /*
This blew up in xfstests so one of your assumptions is incorrect. Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Miao Xie Jan. 29, 2014, 2:25 a.m. UTC | #2
On 	tue, 28 Jan 2014 11:00:54 -0500, Josef Bacik wrote:
> 
> On 01/14/2014 07:31 AM, Miao Xie wrote:
>> We are sure that:
>> - one ordered extent just has one csum calculation worker, and no one
>>    access the csum list during the csum calculation except the worker.
>> - we don't change the list and free the csum until no one reference
>>    to the ordered extent
>> So it is safe to add csum without the lock.
>>
>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
>> ---
>>   fs/btrfs/ordered-data.c |    5 -----
>>   1 files changed, 0 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
>> index e4c3d56..396c6d1 100644
>> --- a/fs/btrfs/ordered-data.c
>> +++ b/fs/btrfs/ordered-data.c
>> @@ -280,16 +280,11 @@ void btrfs_add_ordered_sum(struct inode *inode,
>>                  struct btrfs_ordered_extent *entry,
>>                  struct btrfs_ordered_sum *sum)
>>   {
>> -    struct btrfs_ordered_inode_tree *tree;
>> -
>> -    tree = &BTRFS_I(inode)->ordered_tree;
>> -    spin_lock_irq(&tree->lock);
>>       list_add_tail(&sum->list, &entry->list);
>>       WARN_ON(entry->csum_bytes_left < sum->len);
>>       entry->csum_bytes_left -= sum->len;
>>       if (entry->csum_bytes_left == 0)
>>           wake_up(&entry->wait);
>> -    spin_unlock_irq(&tree->lock);
>>   }
>>     /*
> This blew up in xfstests so one of your assumptions is incorrect. Thanks,

I think it is because there is a bug in the other place. Please tell me
the case that can reproduce the problem.

Thanks
Miao

> 
> Josef
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josef Bacik Jan. 29, 2014, 6:43 p.m. UTC | #3
On 01/28/2014 09:25 PM, Miao Xie wrote:
> On 	tue, 28 Jan 2014 11:00:54 -0500, Josef Bacik wrote:
>> On 01/14/2014 07:31 AM, Miao Xie wrote:
>>> We are sure that:
>>> - one ordered extent just has one csum calculation worker, and no one
>>>     access the csum list during the csum calculation except the worker.
>>> - we don't change the list and free the csum until no one reference
>>>     to the ordered extent
>>> So it is safe to add csum without the lock.
>>>
>>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
>>> ---
>>>    fs/btrfs/ordered-data.c |    5 -----
>>>    1 files changed, 0 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
>>> index e4c3d56..396c6d1 100644
>>> --- a/fs/btrfs/ordered-data.c
>>> +++ b/fs/btrfs/ordered-data.c
>>> @@ -280,16 +280,11 @@ void btrfs_add_ordered_sum(struct inode *inode,
>>>                   struct btrfs_ordered_extent *entry,
>>>                   struct btrfs_ordered_sum *sum)
>>>    {
>>> -    struct btrfs_ordered_inode_tree *tree;
>>> -
>>> -    tree = &BTRFS_I(inode)->ordered_tree;
>>> -    spin_lock_irq(&tree->lock);
>>>        list_add_tail(&sum->list, &entry->list);
>>>        WARN_ON(entry->csum_bytes_left < sum->len);
>>>        entry->csum_bytes_left -= sum->len;
>>>        if (entry->csum_bytes_left == 0)
>>>            wake_up(&entry->wait);
>>> -    spin_unlock_irq(&tree->lock);
>>>    }
>>>      /*
>> This blew up in xfstests so one of your assumptions is incorrect. Thanks,
> I think it is because there is a bug in the other place. Please tell me
> the case that can reproduce the problem.
>
It was one of the btrfs tests, I think btrfs/005 maybe?  Turn on slab 
debugging, that seemed to make a difference.  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index e4c3d56..396c6d1 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -280,16 +280,11 @@  void btrfs_add_ordered_sum(struct inode *inode,
 			   struct btrfs_ordered_extent *entry,
 			   struct btrfs_ordered_sum *sum)
 {
-	struct btrfs_ordered_inode_tree *tree;
-
-	tree = &BTRFS_I(inode)->ordered_tree;
-	spin_lock_irq(&tree->lock);
 	list_add_tail(&sum->list, &entry->list);
 	WARN_ON(entry->csum_bytes_left < sum->len);
 	entry->csum_bytes_left -= sum->len;
 	if (entry->csum_bytes_left == 0)
 		wake_up(&entry->wait);
-	spin_unlock_irq(&tree->lock);
 }
 
 /*