On Mar 12, 2022, at 4:18 PM, Artem Blagodarenko <[email protected]> wrote:
>
> From: Andrew Perepechko <[email protected]>
>
> When changing a large xattr value to a different large xattr value,
> the old xattr inode is freed. Truncate during the final iput causes
> current transaction restart. Eventually, parent inode bh is marked
> dirty and kernel panic happens when jbd2 figures out that this bh
> belongs to the committed transaction.
>
> A possible fix is to call this final iput in a separate thread.
> This way, setxattr transactions will never be split into two.
> Since the setxattr code adds xattr inodes with nlink=0 into the
> orphan list, old xattr inodes will be properly cleaned up in
> any case.
>
> Signed-off-by: Andrew Perepechko <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
> HPE-bug-id: LUS-10534
>
> Changes since v1:
> - fixed bug added during the porting
>
> ---
> fs/ext4/super.c | 1 +
> fs/ext4/xattr.c | 34 ++++++++++++++++++++++++++++++++--
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c5021ca0a28a..8c04c19fa4b8 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1199,6 +1199,7 @@ static void ext4_put_super(struct super_block *sb)
> int aborted = 0;
> int i, err;
>
> + flush_scheduled_work();
> ext4_unregister_li_request(sb);
> ext4_quota_off_umount(sb);
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 042325349098..13c396e354c8 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1544,6 +1544,31 @@ static int ext4_xattr_inode_lookup_create(handle_t *handle, struct inode *inode,
> return 0;
> }
>
> +struct delayed_iput_work {
> + struct work_struct work;
> + struct inode *inode;
> +};
> +
> +static void delayed_iput_fn(struct work_struct *work)
> +{
> + struct delayed_iput_work *diwork;
> +
> + diwork = container_of(work, struct delayed_iput_work, work);
> + iput(diwork->inode);
> + kfree(diwork);
> +}
> +
> +static void delayed_iput(struct inode *inode, struct delayed_iput_work *work)
> +{
> + if (!work) {
> + iput(inode);
> + } else {
> + INIT_WORK(&work->work, delayed_iput_fn);
> + work->inode = inode;
> + schedule_work(&work->work);
> + }
> +}
> +
> /*
> * Reserve min(block_size/8, 1024) bytes for xattr entries/names if ea_inode
> * feature is enabled.
> @@ -1561,6 +1586,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
> int in_inode = i->in_inode;
> struct inode *old_ea_inode = NULL;
> struct inode *new_ea_inode = NULL;
> + struct delayed_iput_work *diwork = NULL;
> size_t old_size, new_size;
> int ret;
>
> @@ -1637,7 +1663,11 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
> * Finish that work before doing any modifications to the xattr data.
> */
> if (!s->not_found && here->e_value_inum) {
> - ret = ext4_xattr_inode_iget(inode,
> + diwork = kmalloc(sizeof(*diwork), GFP_NOFS);
> + if (!diwork)
> + ret = -ENOMEM;
> + else
> + ret = ext4_xattr_inode_iget(inode,
> le32_to_cpu(here->e_value_inum),
> le32_to_cpu(here->e_hash),
> &old_ea_inode);
> @@ -1790,7 +1820,7 @@ static int ext4_xattr_set_entry(struct ext4_xattr_info *i,
>
> ret = 0;
> out:
> - iput(old_ea_inode);
> + delayed_iput(old_ea_inode, diwork);
> iput(new_ea_inode);
> return ret;
> }
> --
> 2.31.1
>
Cheers, Andreas