| Message ID | 1389702712-26638-3-git-send-email-miaox@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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); } /*
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(-)