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@xxxxxxxxxxxxxx>
>
> This alleviates some of the reservation pressure so :
>
> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>, 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
