Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
From: Filipe Manana <fdmanana@gmail.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-btrfs <linux-btrfs@vger.kernel.org>, kernel-team@fb.com
Subject: Re: [PATCH 2/2] btrfs: use the existing credit for our first prop
Date: Wed, 13 Feb 2019 12:38:59 +0000
Message-ID: <CAL3q7H458ZzjK0RPrHtsURL3RkjzmPQqg1jwHvc6=oPZjMJzWA@mail.gmail.com> (raw)
In-Reply-To: <20190207165426.15866-3-josef@toxicpanda.com>

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
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-07 16:54 [PATCH 0/2] Reserve more space for property inheritance Josef Bacik
2019-02-07 16:54 ` [PATCH 1/2] btrfs: reserve space for inheriting properties Josef Bacik
2019-02-08  6:30   ` Nikolay Borisov
2019-03-22 19:11     ` David Sterba
2019-02-13 12:37   ` Filipe Manana
2019-02-07 16:54 ` [PATCH 2/2] btrfs: use the existing credit for our first prop Josef Bacik
2019-02-13 12:38   ` Filipe Manana [this message]
2019-04-26 14:29   ` David Sterba

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAL3q7H458ZzjK0RPrHtsURL3RkjzmPQqg1jwHvc6=oPZjMJzWA@mail.gmail.com' \
    --to=fdmanana@gmail.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox