Re: [PATCH 05/20] ext4 fast_commit: make use of name_snapshot primitives

Jan Kara
2025-01-10T09:15:46Z

On Fri 10-01-25 02:42:48, Al Viro wrote:
> ... rather than open-coding them. As a bonus, that avoids the pointless
> work with extra allocations, etc. for long names.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Nice! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> fs/ext4/fast_commit.c | 29 +++++------------------------
> fs/ext4/fast_commit.h | 3 +--
> 2 files changed, 6 insertions(+), 26 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 26c4fc37edcf..da4263a14a20 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -322,9 +322,7 @@ void ext4_fc_del(struct inode *inode)
> WARN_ON(!list_empty(&ei->i_fc_dilist));
> spin_unlock(&sbi->s_fc_lock);
>
> - if (fc_dentry->fcd_name.name &&
> - fc_dentry->fcd_name.len > DNAME_INLINE_LEN)
> - kfree(fc_dentry->fcd_name.name);
> + release_dentry_name_snapshot(&fc_dentry->fcd_name);
> kmem_cache_free(ext4_fc_dentry_cachep, fc_dentry);
>
> return;
> @@ -449,22 +447,7 @@ static int __track_dentry_update(handle_t *handle, struct inode *inode,
> node->fcd_op = dentry_update->op;
> node->fcd_parent = dir->i_ino;
> node->fcd_ino = inode->i_ino;
> - if (dentry->d_name.len > DNAME_INLINE_LEN) {
> - node->fcd_name.name = kmalloc(dentry->d_name.len, GFP_NOFS);
> - if (!node->fcd_name.name) {
> - kmem_cache_free(ext4_fc_dentry_cachep, node);
> - ext4_fc_mark_ineligible(sb, EXT4_FC_REASON_NOMEM, handle);
> - mutex_lock(&ei->i_fc_lock);
> - return -ENOMEM;
> - }
> - memcpy((u8 *)node->fcd_name.name, dentry->d_name.name,
> - dentry->d_name.len);
> - } else {
> - memcpy(node->fcd_iname, dentry->d_name.name,
> - dentry->d_name.len);
> - node->fcd_name.name = node->fcd_iname;
> - }
> - node->fcd_name.len = dentry->d_name.len;
> + take_dentry_name_snapshot(&node->fcd_name, dentry);
> INIT_LIST_HEAD(&node->fcd_dilist);
> spin_lock(&sbi->s_fc_lock);
> if (sbi->s_journal->j_flags & JBD2_FULL_COMMIT_ONGOING ||
> @@ -832,7 +815,7 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
> {
> struct ext4_fc_dentry_info fcd;
> struct ext4_fc_tl tl;
> - int dlen = fc_dentry->fcd_name.len;
> + int dlen = fc_dentry->fcd_name.name.len;
> u8 *dst = ext4_fc_reserve_space(sb,
> EXT4_FC_TAG_BASE_LEN + sizeof(fcd) + dlen, crc);
>
> @@ -847,7 +830,7 @@ static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
> dst += EXT4_FC_TAG_BASE_LEN;
> memcpy(dst, &fcd, sizeof(fcd));
> dst += sizeof(fcd);
> - memcpy(dst, fc_dentry->fcd_name.name, dlen);
> + memcpy(dst, fc_dentry->fcd_name.name.name, dlen);
>
> return true;
> }
> @@ -1328,9 +1311,7 @@ static void ext4_fc_cleanup(journal_t *journal, int full, tid_t tid)
> list_del_init(&fc_dentry->fcd_dilist);
> spin_unlock(&sbi->s_fc_lock);
>
> - if (fc_dentry->fcd_name.name &&
> - fc_dentry->fcd_name.len > DNAME_INLINE_LEN)
> - kfree(fc_dentry->fcd_name.name);
> + release_dentry_name_snapshot(&fc_dentry->fcd_name);
> kmem_cache_free(ext4_fc_dentry_cachep, fc_dentry);
> spin_lock(&sbi->s_fc_lock);
> }
> diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
> index 2fadb2c4780c..3bd534e4dbbf 100644
> --- a/fs/ext4/fast_commit.h
> +++ b/fs/ext4/fast_commit.h
> @@ -109,8 +109,7 @@ struct ext4_fc_dentry_update {
> int fcd_op; /* Type of update create / unlink / link */
> int fcd_parent; /* Parent inode number */
> int fcd_ino; /* Inode number */
> - struct qstr fcd_name; /* Dirent name */
> - unsigned char fcd_iname[DNAME_INLINE_LEN]; /* Dirent name string */
> + struct name_snapshot fcd_name; /* Dirent name */
> struct list_head fcd_list;
> struct list_head fcd_dilist;
> };
> --
> 2.39.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR