On 2020/7/18 上午4:42, Josef Bacik wrote:
> Autodefrag is very useful for somethings, like the 9000 sqllite files
> that Firefox uses, but is way less useful for virt images.
> Unfortunately this is only available currently as a whole mount option.
> Fix this by adding an "autodefrag" property, that users can set on a per
> file or per directory basis. Thus allowing them to control where
> exactly the extra write activity is going to occur.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> - This is an RFC because I want to make sure we're ok with this before I go and
> add btrfs-progs support for this. I'm not married to the name or the value,
> but I think the core goal is valuable.
The idea looks pretty good to me.
Although it would be much more convincing to bring some real-world micro
bench to show the benefit.
However I still have a concern related to defrag.
Since it's on-disk flag, thus can be inherited by snapshot, then what
would happen if an auto-defrag inode get snapshotted.
Would any write to the auto-defrag inode in new snapshot break the space
saving?
>
> fs/btrfs/btrfs_inode.h | 1 +
> fs/btrfs/file.c | 16 ++++++++++------
> fs/btrfs/props.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index e7d709505cb1..4f04f0535f90 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -31,6 +31,7 @@ enum {
> BTRFS_INODE_READDIO_NEED_LOCK,
> BTRFS_INODE_HAS_PROPS,
> BTRFS_INODE_SNAPSHOT_FLUSH,
> + BTRFS_INODE_AUTODEFRAG,
> };
>
> /* in memory btrfs inode */
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 841c516079a9..cac2092bdcdf 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -116,12 +116,16 @@ static int __btrfs_add_inode_defrag(struct btrfs_inode *inode,
> return 0;
> }
>
> -static inline int __need_auto_defrag(struct btrfs_fs_info *fs_info)
> +static inline int __need_auto_defrag(struct btrfs_fs_info *fs_info,
> + struct btrfs_inode *inode)
We can grab fs_info from btrfs_inode, thus no need for the fs_info
parameter.
Thanks,
Qu
> {
> - if (!btrfs_test_opt(fs_info, AUTO_DEFRAG))
> + if (btrfs_fs_closing(fs_info))
> return 0;
>
> - if (btrfs_fs_closing(fs_info))
> + if (inode && test_bit(BTRFS_INODE_AUTODEFRAG, &inode->runtime_flags))
> + return 1;
> +
> + if (!btrfs_test_opt(fs_info, AUTO_DEFRAG))
> return 0;
>
> return 1;
> @@ -140,7 +144,7 @@ int btrfs_add_inode_defrag(struct btrfs_trans_handle *trans,
> u64 transid;
> int ret;
>
> - if (!__need_auto_defrag(fs_info))
> + if (!__need_auto_defrag(fs_info, inode))
> return 0;
>
> if (test_bit(BTRFS_INODE_IN_DEFRAG, &inode->runtime_flags))
> @@ -187,7 +191,7 @@ static void btrfs_requeue_inode_defrag(struct btrfs_inode *inode,
> struct btrfs_fs_info *fs_info = inode->root->fs_info;
> int ret;
>
> - if (!__need_auto_defrag(fs_info))
> + if (!__need_auto_defrag(fs_info, inode))
> goto out;
>
> /*
> @@ -348,7 +352,7 @@ int btrfs_run_defrag_inodes(struct btrfs_fs_info *fs_info)
> &fs_info->fs_state))
> break;
>
> - if (!__need_auto_defrag(fs_info))
> + if (btrfs_fs_closing(fs_info))
> break;
>
> /* find an inode to defrag */
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index 2dcb1cb21634..5c6411c8b19f 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -310,6 +310,40 @@ static const char *prop_compression_extract(struct inode *inode)
> return NULL;
> }
>
> +static int prop_autodefrag_validate(const char *value, size_t len)
> +{
> + if (!value)
> + return 0;
> + if (!strncmp("true", value, 4))
> + return 0;
> + return -EINVAL;
> +}
> +
> +static int prop_autodefrag_apply(struct inode *inode, const char *value,
> + size_t len)
> +{
> + if (len == 0) {
> + clear_bit(BTRFS_INODE_AUTODEFRAG,
> + &BTRFS_I(inode)->runtime_flags);
> + return 0;
> + }
> +
> + if (!strncmp("true", value, 4)) {
> + set_bit(BTRFS_INODE_AUTODEFRAG,
> + &BTRFS_I(inode)->runtime_flags);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const char *prop_autodefrag_extract(struct inode *inode)
> +{
> + if (test_bit(BTRFS_INODE_AUTODEFRAG, &BTRFS_I(inode)->runtime_flags))
> + return "true";
> + return NULL;
> +}
> +
> static struct prop_handler prop_handlers[] = {
> {
> .xattr_name = XATTR_BTRFS_PREFIX "compression",
> @@ -318,6 +352,13 @@ static struct prop_handler prop_handlers[] = {
> .extract = prop_compression_extract,
> .inheritable = 1
> },
> + {
> + .xattr_name = XATTR_BTRFS_PREFIX "autodefrag",
> + .validate = prop_autodefrag_validate,
> + .apply = prop_autodefrag_apply,
> + .extract = prop_autodefrag_extract,
> + .inheritable = 1
> + },
> };
>
> static int inherit_props(struct btrfs_trans_handle *trans,
>
Attachment:
signature.asc
Description: OpenPGP digital signature
