On Tue, Apr 02, 2019 at 04:35:12PM +0300, Nikolay Borisov wrote:
> On 2.04.19 г. 13:07 ч., Anand Jain wrote:
> > When an inode inherits property from its parent, we call btrfs_set_prop().
> > btrfs_set_prop() does an elaborate checks, which is not required in the
> > context of inheriting a property. Instead just open-code only the required
> > items from btrfs_set_prop() and then call btrfs_setxattr() directly. So
> > now the only user of btrfs_set_prop() is gone, (except for the wraper
> > function btrfs_set_prop_trans()).
> >
> > Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> > ---
> > fs/btrfs/props.c | 27 +++++++++++++++++++++------
> > 1 file changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> > index 72a06c4d3c70..fb84e76f3b1d 100644
> > --- a/fs/btrfs/props.c
> > +++ b/fs/btrfs/props.c
> > @@ -367,20 +367,35 @@ static int inherit_props(struct btrfs_trans_handle *trans,
> > if (!value)
> > continue;
> >
> > + /* may be removed */
>
> This comment brings no value. Either explain *why* it can be removed or
> remove it altogether.
>
> > + ret = h->validate(value, strlen(value));
> > + if (ret)
> > + 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;
> > - ret = btrfs_set_prop(trans, inode, h->xattr_name, value,
> > + return ret;
> > +
> > + ret = btrfs_setxattr(trans, inode, h->xattr_name, value,
> > strlen(value), 0);
> > + if (!ret) {
> > + ret = h->apply(inode, value, strlen(value));
> > + if (ret)
> > + btrfs_setxattr(trans, inode, h->xattr_name,
> > + NULL, 0, 0);
> > + else
> > + set_bit(BTRFS_INODE_HAS_PROPS,
> > + &BTRFS_I(inode)->runtime_flags);
> > + }
> > +
> > btrfs_block_rsv_release(fs_info, trans->block_rsv, num_bytes);
>
> (not related to this patch) but should the reservation be released after
> setxattr is called, even if it succeeds, since we need to hold it until
> the reservation is committed. E.g. if we have to inherit 2 props then in
> the first iteration of the loop we reserve space, we call btrfs_Setxattr
> with enough reserved space, then in the 2nd reservation we need another
> reservation on top of the previous one, no ?
I think that's the problem Josef's patches were supposed to fix.
