| Message ID | 20190207165426.15866-3-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reserve more space for property inheritance | expand |
On Thu, Feb 7, 2019 at 4:57 PM Josef Bacik <josef@toxicpanda.com> wrote: > > We're now reserving an extra items worth of space for property > inheritance. We only have one property at the moment so this covers us, > but if we add more in the future this will allow us to not get bitten by > the extra space reservation. If we do add more properties in the future > we should re-visit how we calculate the space reservation needs by the > callers. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good, thanks. > --- > fs/btrfs/props.c | 32 ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c > index dc6140013ae8..b3d22fef8c92 100644 > --- a/fs/btrfs/props.c > +++ b/fs/btrfs/props.c > @@ -291,6 +291,7 @@ static int inherit_props(struct btrfs_trans_handle *trans, > struct btrfs_fs_info *fs_info = root->fs_info; > int ret; > int i; > + bool need_reserve = false; > > if (!test_bit(BTRFS_INODE_HAS_PROPS, > &BTRFS_I(parent)->runtime_flags)) > @@ -308,16 +309,31 @@ static int inherit_props(struct btrfs_trans_handle *trans, > if (!value) > continue; > > - num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1); > - ret = btrfs_block_rsv_add(root, trans->block_rsv, > - num_bytes, BTRFS_RESERVE_NO_FLUSH); > - if (ret) > - goto out; > + /* > + * Currently callers should be reserving 1 credit for > + * properties, since we only have 1 property that we currently > + * support. If we add more in the future we need to try and > + * reserve more space for them. But we should also revisit how > + * we do space reservations if we do add more properties in the > + * future. > + */ > + if (need_reserve) { > + num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1); > + ret = btrfs_block_rsv_add(root, trans->block_rsv, > + num_bytes, > + BTRFS_RESERVE_NO_FLUSH); > + if (ret) > + goto out; > + } > ret = __btrfs_set_prop(trans, inode, h->xattr_name, > value, strlen(value), 0); > - btrfs_block_rsv_release(fs_info, trans->block_rsv, num_bytes); > - if (ret) > - goto out; > + if (need_reserve) { > + btrfs_block_rsv_release(fs_info, trans->block_rsv, > + num_bytes); > + if (ret) > + goto out; > + } > + need_reserve = true; > } > ret = 0; > out: > -- > 2.14.3 >
On Thu, Feb 07, 2019 at 11:54:26AM -0500, Josef Bacik wrote: > We're now reserving an extra items worth of space for property > inheritance. We only have one property at the moment so this covers us, > but if we add more in the future this will allow us to not get bitten by > the extra space reservation. If we do add more properties in the future > we should re-visit how we calculate the space reservation needs by the > callers. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> There were xattr and property code cleanups, I've applied this patch on top of that as the logic hasn't changed, only the context.
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c index dc6140013ae8..b3d22fef8c92 100644 --- a/fs/btrfs/props.c +++ b/fs/btrfs/props.c @@ -291,6 +291,7 @@ static int inherit_props(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info = root->fs_info; int ret; int i; + bool need_reserve = false; if (!test_bit(BTRFS_INODE_HAS_PROPS, &BTRFS_I(parent)->runtime_flags)) @@ -308,16 +309,31 @@ static int inherit_props(struct btrfs_trans_handle *trans, if (!value) continue; - num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1); - ret = btrfs_block_rsv_add(root, trans->block_rsv, - num_bytes, BTRFS_RESERVE_NO_FLUSH); - if (ret) - goto out; + /* + * Currently callers should be reserving 1 credit for + * properties, since we only have 1 property that we currently + * support. If we add more in the future we need to try and + * reserve more space for them. But we should also revisit how + * we do space reservations if we do add more properties in the + * future. + */ + if (need_reserve) { + num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1); + ret = btrfs_block_rsv_add(root, trans->block_rsv, + num_bytes, + BTRFS_RESERVE_NO_FLUSH); + if (ret) + goto out; + } ret = __btrfs_set_prop(trans, inode, h->xattr_name, value, strlen(value), 0); - btrfs_block_rsv_release(fs_info, trans->block_rsv, num_bytes); - if (ret) - goto out; + if (need_reserve) { + btrfs_block_rsv_release(fs_info, trans->block_rsv, + num_bytes); + if (ret) + goto out; + } + need_reserve = true; } ret = 0; out:
We're now reserving an extra items worth of space for property inheritance. We only have one property at the moment so this covers us, but if we add more in the future this will allow us to not get bitten by the extra space reservation. If we do add more properties in the future we should re-visit how we calculate the space reservation needs by the callers. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/props.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)