| Message ID | 20180629082725.12934-1-ethanlien@synology.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29.06.2018 11:27, Ethan Lien wrote: > We use percpu_counter to reduce lock contention of frequently updated > counter. But the default 32 batch size is suitable only for inc/dec > update, not for sector size update. The end result is we still acquire > spin lock for every percpu_counter_add(). So use customized batch size > for total_bytes_pinned as we already done for dirty_metadata_bytes > and delalloc_bytes. Also we fix dirty_metadata_bytes to use > __percpu_counter_compare of customized batch so we get precise value of > dirty_metadata_bytes. Do you have any numbers to prove this is problematic? Perhaps a fio workload + lockdep contention numbers before/after the patch ? > > Signed-off-by: Ethan Lien <ethanlien@synology.com> > --- > fs/btrfs/ctree.h | 1 + > fs/btrfs/disk-io.c | 10 ++++++---- > fs/btrfs/extent-tree.c | 43 +++++++++++++++++++++++++++--------------- > 3 files changed, 35 insertions(+), 19 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 118346aceea9..df682a521635 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -422,6 +422,7 @@ struct btrfs_space_info { > * time the transaction commits. > */ > struct percpu_counter total_bytes_pinned; > + s32 total_bytes_pinned_batch; > > struct list_head list; > /* Protected by the spinlock 'lock'. */ > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 205092dc9390..dfed08e70ec1 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -961,8 +961,9 @@ static int btree_writepages(struct address_space *mapping, > > fs_info = BTRFS_I(mapping->host)->root->fs_info; > /* this is a bit racy, but that's ok */ > - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, > - BTRFS_DIRTY_METADATA_THRESH); > + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, > + BTRFS_DIRTY_METADATA_THRESH, > + fs_info->dirty_metadata_batch); > if (ret < 0) > return 0; > } > @@ -4150,8 +4151,9 @@ static void __btrfs_btree_balance_dirty(struct btrfs_fs_info *fs_info, > if (flush_delayed) > btrfs_balance_delayed_items(fs_info); > > - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, > - BTRFS_DIRTY_METADATA_THRESH); > + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, > + BTRFS_DIRTY_METADATA_THRESH, > + fs_info->dirty_metadata_batch); > if (ret > 0) { > balance_dirty_pages_ratelimited(fs_info->btree_inode->i_mapping); > } This hunk needs to be in a separate patch. And the last sentence which relates to it needs to be expanded further to explain the problem. > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 3d9fe58c0080..a067c047904c 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -758,7 +758,8 @@ static void add_pinned_bytes(struct btrfs_fs_info *fs_info, s64 num_bytes, > > space_info = __find_space_info(fs_info, flags); > ASSERT(space_info); > - percpu_counter_add(&space_info->total_bytes_pinned, num_bytes); > + percpu_counter_add_batch(&space_info->total_bytes_pinned, num_bytes, > + space_info->total_bytes_pinned_batch); > } > > /* > @@ -2598,8 +2599,9 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans, > flags = BTRFS_BLOCK_GROUP_METADATA; > space_info = __find_space_info(fs_info, flags); > ASSERT(space_info); > - percpu_counter_add(&space_info->total_bytes_pinned, > - -head->num_bytes); > + percpu_counter_add_batch(&space_info->total_bytes_pinned, > + -head->num_bytes, > + space_info->total_bytes_pinned_batch); > > if (head->is_data) { > spin_lock(&delayed_refs->lock); > @@ -4035,6 +4037,10 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags) > INIT_LIST_HEAD(&space_info->ro_bgs); > INIT_LIST_HEAD(&space_info->tickets); > INIT_LIST_HEAD(&space_info->priority_tickets); > + if (flags & BTRFS_BLOCK_GROUP_DATA) > + space_info->total_bytes_pinned_batch = info->delalloc_batch; > + else > + space_info->total_bytes_pinned_batch = info->dirty_metadata_batch; > > ret = kobject_init_and_add(&space_info->kobj, &space_info_ktype, > info->space_info_kobj, "%s", > @@ -4309,9 +4315,10 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes) > * allocation, and no removed chunk in current transaction, > * don't bother committing the transaction. > */ > - have_pinned_space = percpu_counter_compare( > + have_pinned_space = __percpu_counter_compare( > &data_sinfo->total_bytes_pinned, > - used + bytes - data_sinfo->total_bytes); > + used + bytes - data_sinfo->total_bytes, > + data_sinfo->total_bytes_pinned_batch); > spin_unlock(&data_sinfo->lock); > > /* commit the current transaction and try again */ > @@ -4912,8 +4919,9 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, > return 0; > > /* See if there is enough pinned space to make this reservation */ > - if (percpu_counter_compare(&space_info->total_bytes_pinned, > - bytes) >= 0) > + if (__percpu_counter_compare(&space_info->total_bytes_pinned, > + bytes, > + space_info->total_bytes_pinned_batch) >= 0) > goto commit; > > /* > @@ -4930,8 +4938,9 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, > bytes -= delayed_rsv->size; > spin_unlock(&delayed_rsv->lock); > > - if (percpu_counter_compare(&space_info->total_bytes_pinned, > - bytes) < 0) { > + if (__percpu_counter_compare(&space_info->total_bytes_pinned, > + bytes, > + space_info->total_bytes_pinned_batch) < 0) { > return -ENOSPC; > } > > @@ -6268,8 +6277,9 @@ static int update_block_group(struct btrfs_trans_handle *trans, > trace_btrfs_space_reservation(info, "pinned", > cache->space_info->flags, > num_bytes, 1); > - percpu_counter_add(&cache->space_info->total_bytes_pinned, > - num_bytes); > + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, > + num_bytes, > + cache->space_info->total_bytes_pinned_batch); > set_extent_dirty(info->pinned_extents, > bytenr, bytenr + num_bytes - 1, > GFP_NOFS | __GFP_NOFAIL); > @@ -6347,7 +6357,8 @@ static int pin_down_extent(struct btrfs_fs_info *fs_info, > > trace_btrfs_space_reservation(fs_info, "pinned", > cache->space_info->flags, num_bytes, 1); > - percpu_counter_add(&cache->space_info->total_bytes_pinned, num_bytes); > + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, > + num_bytes, cache->space_info->total_bytes_pinned_batch); > set_extent_dirty(fs_info->pinned_extents, bytenr, > bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL); > return 0; > @@ -6711,7 +6722,8 @@ static int unpin_extent_range(struct btrfs_fs_info *fs_info, > trace_btrfs_space_reservation(fs_info, "pinned", > space_info->flags, len, 0); > space_info->max_extent_size = 0; > - percpu_counter_add(&space_info->total_bytes_pinned, -len); > + percpu_counter_add_batch(&space_info->total_bytes_pinned, > + -len, space_info->total_bytes_pinned_batch); > if (cache->ro) { > space_info->bytes_readonly += len; > readonly = true; > @@ -10764,8 +10776,9 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) > > space_info->bytes_pinned -= block_group->pinned; > space_info->bytes_readonly += block_group->pinned; > - percpu_counter_add(&space_info->total_bytes_pinned, > - -block_group->pinned); > + percpu_counter_add_batch(&space_info->total_bytes_pinned, > + -block_group->pinned, > + space_info->total_bytes_pinned_batch); > block_group->pinned = 0; > > spin_unlock(&block_group->lock); > -- 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
Nikolay Borisov 於 2018-06-29 16:43 寫到: > On 29.06.2018 11:27, Ethan Lien wrote: >> We use percpu_counter to reduce lock contention of frequently updated >> counter. But the default 32 batch size is suitable only for inc/dec >> update, not for sector size update. The end result is we still acquire >> spin lock for every percpu_counter_add(). So use customized batch size >> for total_bytes_pinned as we already done for dirty_metadata_bytes >> and delalloc_bytes. Also we fix dirty_metadata_bytes to use >> __percpu_counter_compare of customized batch so we get precise value >> of >> dirty_metadata_bytes. > > Do you have any numbers to prove this is problematic? Perhaps a fio > workload + lockdep contention numbers before/after the patch ? > I think a contented path may come from heavily cow metadata and short data extents, where we call add_pinned_bytes() frequently in releasing old blocks. I'll try if I can find out a test to show the difference. >> >> Signed-off-by: Ethan Lien <ethanlien@synology.com> >> --- >> fs/btrfs/ctree.h | 1 + >> fs/btrfs/disk-io.c | 10 ++++++---- >> fs/btrfs/extent-tree.c | 43 >> +++++++++++++++++++++++++++--------------- >> 3 files changed, 35 insertions(+), 19 deletions(-) >> >> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h >> index 118346aceea9..df682a521635 100644 >> --- a/fs/btrfs/ctree.h >> +++ b/fs/btrfs/ctree.h >> @@ -422,6 +422,7 @@ struct btrfs_space_info { >> * time the transaction commits. >> */ >> struct percpu_counter total_bytes_pinned; >> + s32 total_bytes_pinned_batch; >> >> struct list_head list; >> /* Protected by the spinlock 'lock'. */ >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index 205092dc9390..dfed08e70ec1 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -961,8 +961,9 @@ static int btree_writepages(struct address_space >> *mapping, >> >> fs_info = BTRFS_I(mapping->host)->root->fs_info; >> /* this is a bit racy, but that's ok */ >> - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, >> - BTRFS_DIRTY_METADATA_THRESH); >> + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, >> + BTRFS_DIRTY_METADATA_THRESH, >> + fs_info->dirty_metadata_batch); >> if (ret < 0) >> return 0; >> } >> @@ -4150,8 +4151,9 @@ static void __btrfs_btree_balance_dirty(struct >> btrfs_fs_info *fs_info, >> if (flush_delayed) >> btrfs_balance_delayed_items(fs_info); >> >> - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, >> - BTRFS_DIRTY_METADATA_THRESH); >> + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, >> + BTRFS_DIRTY_METADATA_THRESH, >> + fs_info->dirty_metadata_batch); >> if (ret > 0) { >> balance_dirty_pages_ratelimited(fs_info->btree_inode->i_mapping); >> } > > This hunk needs to be in a separate patch. And the last sentence which > relates to it needs to be expanded further to explain the problem. > OK I'll prepare a separate patch. >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 3d9fe58c0080..a067c047904c 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -758,7 +758,8 @@ static void add_pinned_bytes(struct btrfs_fs_info >> *fs_info, s64 num_bytes, >> >> space_info = __find_space_info(fs_info, flags); >> ASSERT(space_info); >> - percpu_counter_add(&space_info->total_bytes_pinned, num_bytes); >> + percpu_counter_add_batch(&space_info->total_bytes_pinned, num_bytes, >> + space_info->total_bytes_pinned_batch); >> } >> >> /* >> @@ -2598,8 +2599,9 @@ static int cleanup_ref_head(struct >> btrfs_trans_handle *trans, >> flags = BTRFS_BLOCK_GROUP_METADATA; >> space_info = __find_space_info(fs_info, flags); >> ASSERT(space_info); >> - percpu_counter_add(&space_info->total_bytes_pinned, >> - -head->num_bytes); >> + percpu_counter_add_batch(&space_info->total_bytes_pinned, >> + -head->num_bytes, >> + space_info->total_bytes_pinned_batch); >> >> if (head->is_data) { >> spin_lock(&delayed_refs->lock); >> @@ -4035,6 +4037,10 @@ static int create_space_info(struct >> btrfs_fs_info *info, u64 flags) >> INIT_LIST_HEAD(&space_info->ro_bgs); >> INIT_LIST_HEAD(&space_info->tickets); >> INIT_LIST_HEAD(&space_info->priority_tickets); >> + if (flags & BTRFS_BLOCK_GROUP_DATA) >> + space_info->total_bytes_pinned_batch = info->delalloc_batch; >> + else >> + space_info->total_bytes_pinned_batch = info->dirty_metadata_batch; >> >> ret = kobject_init_and_add(&space_info->kobj, &space_info_ktype, >> info->space_info_kobj, "%s", >> @@ -4309,9 +4315,10 @@ int btrfs_alloc_data_chunk_ondemand(struct >> btrfs_inode *inode, u64 bytes) >> * allocation, and no removed chunk in current transaction, >> * don't bother committing the transaction. >> */ >> - have_pinned_space = percpu_counter_compare( >> + have_pinned_space = __percpu_counter_compare( >> &data_sinfo->total_bytes_pinned, >> - used + bytes - data_sinfo->total_bytes); >> + used + bytes - data_sinfo->total_bytes, >> + data_sinfo->total_bytes_pinned_batch); >> spin_unlock(&data_sinfo->lock); >> >> /* commit the current transaction and try again */ >> @@ -4912,8 +4919,9 @@ static int may_commit_transaction(struct >> btrfs_fs_info *fs_info, >> return 0; >> >> /* See if there is enough pinned space to make this reservation */ >> - if (percpu_counter_compare(&space_info->total_bytes_pinned, >> - bytes) >= 0) >> + if (__percpu_counter_compare(&space_info->total_bytes_pinned, >> + bytes, >> + space_info->total_bytes_pinned_batch) >= 0) >> goto commit; >> >> /* >> @@ -4930,8 +4938,9 @@ static int may_commit_transaction(struct >> btrfs_fs_info *fs_info, >> bytes -= delayed_rsv->size; >> spin_unlock(&delayed_rsv->lock); >> >> - if (percpu_counter_compare(&space_info->total_bytes_pinned, >> - bytes) < 0) { >> + if (__percpu_counter_compare(&space_info->total_bytes_pinned, >> + bytes, >> + space_info->total_bytes_pinned_batch) < 0) { >> return -ENOSPC; >> } >> >> @@ -6268,8 +6277,9 @@ static int update_block_group(struct >> btrfs_trans_handle *trans, >> trace_btrfs_space_reservation(info, "pinned", >> cache->space_info->flags, >> num_bytes, 1); >> - percpu_counter_add(&cache->space_info->total_bytes_pinned, >> - num_bytes); >> + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, >> + num_bytes, >> + cache->space_info->total_bytes_pinned_batch); >> set_extent_dirty(info->pinned_extents, >> bytenr, bytenr + num_bytes - 1, >> GFP_NOFS | __GFP_NOFAIL); >> @@ -6347,7 +6357,8 @@ static int pin_down_extent(struct btrfs_fs_info >> *fs_info, >> >> trace_btrfs_space_reservation(fs_info, "pinned", >> cache->space_info->flags, num_bytes, 1); >> - percpu_counter_add(&cache->space_info->total_bytes_pinned, >> num_bytes); >> + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, >> + num_bytes, cache->space_info->total_bytes_pinned_batch); >> set_extent_dirty(fs_info->pinned_extents, bytenr, >> bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL); >> return 0; >> @@ -6711,7 +6722,8 @@ static int unpin_extent_range(struct >> btrfs_fs_info *fs_info, >> trace_btrfs_space_reservation(fs_info, "pinned", >> space_info->flags, len, 0); >> space_info->max_extent_size = 0; >> - percpu_counter_add(&space_info->total_bytes_pinned, -len); >> + percpu_counter_add_batch(&space_info->total_bytes_pinned, >> + -len, space_info->total_bytes_pinned_batch); >> if (cache->ro) { >> space_info->bytes_readonly += len; >> readonly = true; >> @@ -10764,8 +10776,9 @@ void btrfs_delete_unused_bgs(struct >> btrfs_fs_info *fs_info) >> >> space_info->bytes_pinned -= block_group->pinned; >> space_info->bytes_readonly += block_group->pinned; >> - percpu_counter_add(&space_info->total_bytes_pinned, >> - -block_group->pinned); >> + percpu_counter_add_batch(&space_info->total_bytes_pinned, >> + -block_group->pinned, >> + space_info->total_bytes_pinned_batch); >> block_group->pinned = 0; >> >> spin_unlock(&block_group->lock); >> > -- > 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
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 118346aceea9..df682a521635 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -422,6 +422,7 @@ struct btrfs_space_info { * time the transaction commits. */ struct percpu_counter total_bytes_pinned; + s32 total_bytes_pinned_batch; struct list_head list; /* Protected by the spinlock 'lock'. */ diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 205092dc9390..dfed08e70ec1 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -961,8 +961,9 @@ static int btree_writepages(struct address_space *mapping, fs_info = BTRFS_I(mapping->host)->root->fs_info; /* this is a bit racy, but that's ok */ - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, - BTRFS_DIRTY_METADATA_THRESH); + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, + BTRFS_DIRTY_METADATA_THRESH, + fs_info->dirty_metadata_batch); if (ret < 0) return 0; } @@ -4150,8 +4151,9 @@ static void __btrfs_btree_balance_dirty(struct btrfs_fs_info *fs_info, if (flush_delayed) btrfs_balance_delayed_items(fs_info); - ret = percpu_counter_compare(&fs_info->dirty_metadata_bytes, - BTRFS_DIRTY_METADATA_THRESH); + ret = __percpu_counter_compare(&fs_info->dirty_metadata_bytes, + BTRFS_DIRTY_METADATA_THRESH, + fs_info->dirty_metadata_batch); if (ret > 0) { balance_dirty_pages_ratelimited(fs_info->btree_inode->i_mapping); } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 3d9fe58c0080..a067c047904c 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -758,7 +758,8 @@ static void add_pinned_bytes(struct btrfs_fs_info *fs_info, s64 num_bytes, space_info = __find_space_info(fs_info, flags); ASSERT(space_info); - percpu_counter_add(&space_info->total_bytes_pinned, num_bytes); + percpu_counter_add_batch(&space_info->total_bytes_pinned, num_bytes, + space_info->total_bytes_pinned_batch); } /* @@ -2598,8 +2599,9 @@ static int cleanup_ref_head(struct btrfs_trans_handle *trans, flags = BTRFS_BLOCK_GROUP_METADATA; space_info = __find_space_info(fs_info, flags); ASSERT(space_info); - percpu_counter_add(&space_info->total_bytes_pinned, - -head->num_bytes); + percpu_counter_add_batch(&space_info->total_bytes_pinned, + -head->num_bytes, + space_info->total_bytes_pinned_batch); if (head->is_data) { spin_lock(&delayed_refs->lock); @@ -4035,6 +4037,10 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags) INIT_LIST_HEAD(&space_info->ro_bgs); INIT_LIST_HEAD(&space_info->tickets); INIT_LIST_HEAD(&space_info->priority_tickets); + if (flags & BTRFS_BLOCK_GROUP_DATA) + space_info->total_bytes_pinned_batch = info->delalloc_batch; + else + space_info->total_bytes_pinned_batch = info->dirty_metadata_batch; ret = kobject_init_and_add(&space_info->kobj, &space_info_ktype, info->space_info_kobj, "%s", @@ -4309,9 +4315,10 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes) * allocation, and no removed chunk in current transaction, * don't bother committing the transaction. */ - have_pinned_space = percpu_counter_compare( + have_pinned_space = __percpu_counter_compare( &data_sinfo->total_bytes_pinned, - used + bytes - data_sinfo->total_bytes); + used + bytes - data_sinfo->total_bytes, + data_sinfo->total_bytes_pinned_batch); spin_unlock(&data_sinfo->lock); /* commit the current transaction and try again */ @@ -4912,8 +4919,9 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, return 0; /* See if there is enough pinned space to make this reservation */ - if (percpu_counter_compare(&space_info->total_bytes_pinned, - bytes) >= 0) + if (__percpu_counter_compare(&space_info->total_bytes_pinned, + bytes, + space_info->total_bytes_pinned_batch) >= 0) goto commit; /* @@ -4930,8 +4938,9 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, bytes -= delayed_rsv->size; spin_unlock(&delayed_rsv->lock); - if (percpu_counter_compare(&space_info->total_bytes_pinned, - bytes) < 0) { + if (__percpu_counter_compare(&space_info->total_bytes_pinned, + bytes, + space_info->total_bytes_pinned_batch) < 0) { return -ENOSPC; } @@ -6268,8 +6277,9 @@ static int update_block_group(struct btrfs_trans_handle *trans, trace_btrfs_space_reservation(info, "pinned", cache->space_info->flags, num_bytes, 1); - percpu_counter_add(&cache->space_info->total_bytes_pinned, - num_bytes); + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, + num_bytes, + cache->space_info->total_bytes_pinned_batch); set_extent_dirty(info->pinned_extents, bytenr, bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL); @@ -6347,7 +6357,8 @@ static int pin_down_extent(struct btrfs_fs_info *fs_info, trace_btrfs_space_reservation(fs_info, "pinned", cache->space_info->flags, num_bytes, 1); - percpu_counter_add(&cache->space_info->total_bytes_pinned, num_bytes); + percpu_counter_add_batch(&cache->space_info->total_bytes_pinned, + num_bytes, cache->space_info->total_bytes_pinned_batch); set_extent_dirty(fs_info->pinned_extents, bytenr, bytenr + num_bytes - 1, GFP_NOFS | __GFP_NOFAIL); return 0; @@ -6711,7 +6722,8 @@ static int unpin_extent_range(struct btrfs_fs_info *fs_info, trace_btrfs_space_reservation(fs_info, "pinned", space_info->flags, len, 0); space_info->max_extent_size = 0; - percpu_counter_add(&space_info->total_bytes_pinned, -len); + percpu_counter_add_batch(&space_info->total_bytes_pinned, + -len, space_info->total_bytes_pinned_batch); if (cache->ro) { space_info->bytes_readonly += len; readonly = true; @@ -10764,8 +10776,9 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) space_info->bytes_pinned -= block_group->pinned; space_info->bytes_readonly += block_group->pinned; - percpu_counter_add(&space_info->total_bytes_pinned, - -block_group->pinned); + percpu_counter_add_batch(&space_info->total_bytes_pinned, + -block_group->pinned, + space_info->total_bytes_pinned_batch); block_group->pinned = 0; spin_unlock(&block_group->lock);
We use percpu_counter to reduce lock contention of frequently updated counter. But the default 32 batch size is suitable only for inc/dec update, not for sector size update. The end result is we still acquire spin lock for every percpu_counter_add(). So use customized batch size for total_bytes_pinned as we already done for dirty_metadata_bytes and delalloc_bytes. Also we fix dirty_metadata_bytes to use __percpu_counter_compare of customized batch so we get precise value of dirty_metadata_bytes. Signed-off-by: Ethan Lien <ethanlien@synology.com> --- fs/btrfs/ctree.h | 1 + fs/btrfs/disk-io.c | 10 ++++++---- fs/btrfs/extent-tree.c | 43 +++++++++++++++++++++++++++--------------- 3 files changed, 35 insertions(+), 19 deletions(-)