| Message ID | 20190816150600.9188-3-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rework the worst case calculations for space reservation | expand |
On 16.08.19 г. 18:05 ч., Josef Bacik wrote: > Historically we reserved worst case for every btree operation, and > generally speaking we want to do that in cases where it could be the > worst case. However for updating inodes we know the inode items are > already in the tree, so it will only be an update operation and never an > insert operation. This allows us to always reserve only the > metadata_size amount for inode updates rather than the > insert_metadata_size amount. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> This alleviates some of the reservation pressure so : Reviewed-by: Nikolay Borisov <nborisov@suse.com>, however one small nit below. > --- > fs/btrfs/delalloc-space.c | 15 ++++++++++++--- > fs/btrfs/delayed-inode.c | 2 +- > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c > index 2412be4a3de2..b8111ebdc92a 100644 > --- a/fs/btrfs/delalloc-space.c > +++ b/fs/btrfs/delalloc-space.c > @@ -251,9 +251,16 @@ static void btrfs_calculate_inode_block_rsv_size(struct btrfs_fs_info *fs_info, > > lockdep_assert_held(&inode->lock); > outstanding_extents = inode->outstanding_extents; > - if (outstanding_extents) > + > + /* > + * Insert size for the number of outstanding extents, 1 normal size for > + * updating the inode. > + */ > + if (outstanding_extents) { > reserve_size = btrfs_calc_insert_metadata_size(fs_info, > - outstanding_extents + 1); > + outstanding_extents); > + reserve_size += btrfs_calc_metadata_size(fs_info, 1); > + } > csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, > inode->csum_bytes); > reserve_size += btrfs_calc_insert_metadata_size(fs_info, > @@ -278,10 +285,12 @@ static void calc_inode_reservations(struct btrfs_fs_info *fs_info, > { > u64 nr_extents = count_max_extents(num_bytes); > u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, num_bytes); > + u64 inode_update = btrfs_calc_metadata_size(fs_info, 1); > > /* We add one for the inode update at finish ordered time */ This comment becomes somewhat outdated and should be removed/reworded. Perhaps put above *meta_reserve += inode_update. > *meta_reserve = btrfs_calc_insert_metadata_size(fs_info, > - nr_extents + csum_leaves + 1); > + nr_extents + csum_leaves); > + *meta_reserve += inode_update; > *qgroup_reserve = nr_extents * fs_info->nodesize; > } > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c > index de87ea7ce84d..9318cf761a07 100644 > --- a/fs/btrfs/delayed-inode.c > +++ b/fs/btrfs/delayed-inode.c > @@ -612,7 +612,7 @@ static int btrfs_delayed_inode_reserve_metadata( > src_rsv = trans->block_rsv; > dst_rsv = &fs_info->delayed_block_rsv; > > - num_bytes = btrfs_calc_insert_metadata_size(fs_info, 1); > + num_bytes = btrfs_calc_metadata_size(fs_info, 1); > > /* > * btrfs_dirty_inode will update the inode under btrfs_join_transaction >
On Mon, Aug 19, 2019 at 12:17:07PM +0300, Nikolay Borisov wrote: > > > On 16.08.19 г. 18:05 ч., Josef Bacik wrote: > > Historically we reserved worst case for every btree operation, and > > generally speaking we want to do that in cases where it could be the > > worst case. However for updating inodes we know the inode items are > > already in the tree, so it will only be an update operation and never an > > insert operation. This allows us to always reserve only the > > metadata_size amount for inode updates rather than the > > insert_metadata_size amount. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > This alleviates some of the reservation pressure so : > > Reviewed-by: Nikolay Borisov <nborisov@suse.com>, however one small nit > below. > > > --- > > fs/btrfs/delalloc-space.c | 15 ++++++++++++--- > > fs/btrfs/delayed-inode.c | 2 +- > > 2 files changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c > > index 2412be4a3de2..b8111ebdc92a 100644 > > --- a/fs/btrfs/delalloc-space.c > > +++ b/fs/btrfs/delalloc-space.c > > @@ -251,9 +251,16 @@ static void btrfs_calculate_inode_block_rsv_size(struct btrfs_fs_info *fs_info, > > > > lockdep_assert_held(&inode->lock); > > outstanding_extents = inode->outstanding_extents; > > - if (outstanding_extents) > > + > > + /* > > + * Insert size for the number of outstanding extents, 1 normal size for > > + * updating the inode. > > + */ > > + if (outstanding_extents) { > > reserve_size = btrfs_calc_insert_metadata_size(fs_info, > > - outstanding_extents + 1); > > + outstanding_extents); > > + reserve_size += btrfs_calc_metadata_size(fs_info, 1); > > + } > > csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, > > inode->csum_bytes); > > reserve_size += btrfs_calc_insert_metadata_size(fs_info, > > @@ -278,10 +285,12 @@ static void calc_inode_reservations(struct btrfs_fs_info *fs_info, > > { > > u64 nr_extents = count_max_extents(num_bytes); > > u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, num_bytes); > > + u64 inode_update = btrfs_calc_metadata_size(fs_info, 1); > > > > /* We add one for the inode update at finish ordered time */ > > This comment becomes somewhat outdated and should be removed/reworded. > Perhaps put above *meta_reserve += inode_update. > Yup I'll update this so it's fixed in the next version. Thanks, Josef
diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c index 2412be4a3de2..b8111ebdc92a 100644 --- a/fs/btrfs/delalloc-space.c +++ b/fs/btrfs/delalloc-space.c @@ -251,9 +251,16 @@ static void btrfs_calculate_inode_block_rsv_size(struct btrfs_fs_info *fs_info, lockdep_assert_held(&inode->lock); outstanding_extents = inode->outstanding_extents; - if (outstanding_extents) + + /* + * Insert size for the number of outstanding extents, 1 normal size for + * updating the inode. + */ + if (outstanding_extents) { reserve_size = btrfs_calc_insert_metadata_size(fs_info, - outstanding_extents + 1); + outstanding_extents); + reserve_size += btrfs_calc_metadata_size(fs_info, 1); + } csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, inode->csum_bytes); reserve_size += btrfs_calc_insert_metadata_size(fs_info, @@ -278,10 +285,12 @@ static void calc_inode_reservations(struct btrfs_fs_info *fs_info, { u64 nr_extents = count_max_extents(num_bytes); u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, num_bytes); + u64 inode_update = btrfs_calc_metadata_size(fs_info, 1); /* We add one for the inode update at finish ordered time */ *meta_reserve = btrfs_calc_insert_metadata_size(fs_info, - nr_extents + csum_leaves + 1); + nr_extents + csum_leaves); + *meta_reserve += inode_update; *qgroup_reserve = nr_extents * fs_info->nodesize; } diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c index de87ea7ce84d..9318cf761a07 100644 --- a/fs/btrfs/delayed-inode.c +++ b/fs/btrfs/delayed-inode.c @@ -612,7 +612,7 @@ static int btrfs_delayed_inode_reserve_metadata( src_rsv = trans->block_rsv; dst_rsv = &fs_info->delayed_block_rsv; - num_bytes = btrfs_calc_insert_metadata_size(fs_info, 1); + num_bytes = btrfs_calc_metadata_size(fs_info, 1); /* * btrfs_dirty_inode will update the inode under btrfs_join_transaction
Historically we reserved worst case for every btree operation, and generally speaking we want to do that in cases where it could be the worst case. However for updating inodes we know the inode items are already in the tree, so it will only be an update operation and never an insert operation. This allows us to always reserve only the metadata_size amount for inode updates rather than the insert_metadata_size amount. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/delalloc-space.c | 15 ++++++++++++--- fs/btrfs/delayed-inode.c | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-)