diff mbox series

ext4: Reduce time overhead for replacing xattr values.

Message ID 20230509011042.11781-1-sunjunchao2870@gmail.com
State New
Headers show
Series ext4: Reduce time overhead for replacing xattr values. | expand

Commit Message

Julian Sun May 9, 2023, 1:10 a.m. UTC
When replace the value of xattr which found in an ea_inode, currently
ext4 will evict ea_inode which stores the old value, recreate an
ea_inode and then write the new value into new ea_inode. This can
be optimized by writing the new value into old ea_inode directly.

The logic for replacing the value of xattr without this patch
is as follows:
ext4_xattr_set_entry()
  ->ext4_xattr_inode_iget(&old_ea_inode)
  ->ext4_xattr_inode_lookup_create(&new_ea_inode)
  ->ext4_xattr_inode_dec_ref(old_ea_inode)
  ->iput(old_ea_inode)
      ->ext4_destroy_inode()
      ->ext4_evict_inode()
      ->ext4_free_inode()
  ->iput(new_ea_inode)

The logic with this patch is:
ext4_xattr_set_entry()
  ->ext4_xattr_inode_iget(&old_ea_inode)
  ->ext4_xattr_inode_write(old_ea_inode, new_value)
  ->iput(old_ea_inode)

This patch reduces the time it takes to replace xattr in ext4.
Without this patch, replacing the value of xattr two million times takes
about 45 seconds on Intel(R) Xeon(R) CPU E5-2620 v3 platform.
With this patch, the same operation takes only 6 seconds.

[root@client01 sjc]# ./mount.sh
/dev/sdb1 contains a ext4 file system
    last mounted on /mnt/ext4 on Mon May  8 17:05:38 2023
[root@client01 sjc]# touch /mnt/ext4/file1
[root@client01 sjc]# gcc test.c
[root@client01 sjc]# time ./a.out

real    0m45.248s
user    0m0.513s
sys 0m39.231s

[root@client01 sjc]# ./mount.sh
/dev/sdb1 contains a ext4 file system
    last mounted on /mnt/ext4 on Mon May  8 17:08:20 2023
[root@client01 sjc]# touch /mnt/ext4/file1
[root@client01 sjc]# time ./a.out

real    0m5.977s
user    0m0.316s
sys 0m5.659s

The test.c and mount.sh are in [1].
This patch passed the tests with xfstests using 'check -g quick'.

[1]: https://gist.github.com/sjc2870/c923d7fa627d10ab65d6c305afb02cdb

Signed-off-by: JunChao Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/xattr.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Julian Sun May 9, 2023, 4:38 a.m. UTC | #1
JunChao Sun <sunjunchao2870@gmail.com> 于2023年5月9日周二 09:10写道:
>
> When replace the value of xattr which found in an ea_inode, currently
> ext4 will evict ea_inode which stores the old value, recreate an
> ea_inode and then write the new value into new ea_inode. This can
> be optimized by writing the new value into old ea_inode directly.
>
> The logic for replacing the value of xattr without this patch
> is as follows:
> ext4_xattr_set_entry()
>   ->ext4_xattr_inode_iget(&old_ea_inode)
>   ->ext4_xattr_inode_lookup_create(&new_ea_inode)
>   ->ext4_xattr_inode_dec_ref(old_ea_inode)
>   ->iput(old_ea_inode)
>       ->ext4_destroy_inode()
>       ->ext4_evict_inode()
>       ->ext4_free_inode()
>   ->iput(new_ea_inode)
>
> The logic with this patch is:
> ext4_xattr_set_entry()
>   ->ext4_xattr_inode_iget(&old_ea_inode)
>   ->ext4_xattr_inode_write(old_ea_inode, new_value)
>   ->iput(old_ea_inode)
>
> This patch reduces the time it takes to replace xattr in ext4.
> Without this patch, replacing the value of xattr two million times takes
> about 45 seconds on Intel(R) Xeon(R) CPU E5-2620 v3 platform.
> With this patch, the same operation takes only 6 seconds.
>
> [root@client01 sjc]# ./mount.sh
> /dev/sdb1 contains a ext4 file system
>     last mounted on /mnt/ext4 on Mon May  8 17:05:38 2023
> [root@client01 sjc]# touch /mnt/ext4/file1
> [root@client01 sjc]# gcc test.c
> [root@client01 sjc]# time ./a.out
>
> real    0m45.248s
> user    0m0.513s
> sys 0m39.231s
>
> [root@client01 sjc]# ./mount.sh
> /dev/sdb1 contains a ext4 file system
>     last mounted on /mnt/ext4 on Mon May  8 17:08:20 2023
> [root@client01 sjc]# touch /mnt/ext4/file1
> [root@client01 sjc]# time ./a.out
>
> real    0m5.977s
> user    0m0.316s
> sys 0m5.659s
>
> The test.c and mount.sh are in [1].
> This patch passed the tests with xfstests using 'check -g quick'.
>
> [1]: https://gist.github.com/sjc2870/c923d7fa627d10ab65d6c305afb02cdb
>
> Signed-off-by: JunChao Sun <sunjunchao2870@gmail.com>
> ---
>  fs/ext4/xattr.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index d57408cbe903..37f79594ac70 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1713,6 +1713,39 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
>                 }
>         }
>
> +       if (!s->not_found && i->value && here->e_value_inum && i->in_inode) {
> +               /* Replace xattr value in ea_inode in place */
> +               int size_diff = i->value_len - le32_to_cpu(here->e_value_size);
> +
> +               ret = ext4_xattr_inode_iget(inode,
> +                                               le32_to_cpu(here->e_value_inum),
> +                                               le32_to_cpu(here->e_hash),
> +                                               &old_ea_inode);
> +               if (ret) {
> +                       old_ea_inode = NULL;
> +                       goto out;
> +               }
>
> > +               if (size_diff > 0)
> > +                       ret = ext4_xattr_inode_alloc_quota(inode, size_diff);
> > +               else if (size_diff < 0)
> > +                       ext4_xattr_inode_free_quota(inode, NULL, -size_diff);
> > +               if (ret)
> > +                       goto out;
> > +
> > +               ret = ext4_xattr_inode_write(handle, old_ea_inode, i->value, i->value_len);
> > +               if (ret) {
> > +                       if (size_diff > 0)
> > +                               ext4_xattr_inode_free_quota(inode, NULL, size_diff);
> > +                       else if (size_diff < 0)
> > +                               ret = ext4_xattr_inode_alloc_quota(inode, -size_diff);
> > +                       goto out;
> > +               }
Here might missed a judgment condition: if
(ext4_xattr_inodee_get_ref(old_ea_inode) == 1). I will send patch v2
to fix this.

> +               here->e_value_size = cpu_to_le32(i->value_len);
> +               new_ea_inode = old_ea_inode;
> +               old_ea_inode = NULL;
> +               goto update_hash;
> +       }
> +
>         /*
>          * Getting access to old and new ea inodes is subject to failures.
>          * Finish that work before doing any modifications to the xattr data.
> --
> 1.8.3.1
>
diff mbox series

Patch

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index d57408cbe903..37f79594ac70 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1713,6 +1713,39 @@  static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
 		}
 	}
 
+	if (!s->not_found && i->value && here->e_value_inum && i->in_inode) {
+		/* Replace xattr value in ea_inode in place */
+		int size_diff = i->value_len - le32_to_cpu(here->e_value_size);
+
+		ret = ext4_xattr_inode_iget(inode,
+						le32_to_cpu(here->e_value_inum),
+						le32_to_cpu(here->e_hash),
+						&old_ea_inode);
+		if (ret) {
+			old_ea_inode = NULL;
+			goto out;
+		}
+		if (size_diff > 0)
+			ret = ext4_xattr_inode_alloc_quota(inode, size_diff);
+		else if (size_diff < 0)
+			ext4_xattr_inode_free_quota(inode, NULL, -size_diff);
+		if (ret)
+			goto out;
+
+		ret = ext4_xattr_inode_write(handle, old_ea_inode, i->value, i->value_len);
+		if (ret) {
+			if (size_diff > 0)
+				ext4_xattr_inode_free_quota(inode, NULL, size_diff);
+			else if (size_diff < 0)
+				ret = ext4_xattr_inode_alloc_quota(inode, -size_diff);
+			goto out;
+		}
+		here->e_value_size = cpu_to_le32(i->value_len);
+		new_ea_inode = old_ea_inode;
+		old_ea_inode = NULL;
+		goto update_hash;
+	}
+
 	/*
 	 * Getting access to old and new ea inodes is subject to failures.
 	 * Finish that work before doing any modifications to the xattr data.