On Wed, May 11, 2022 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote: > Keeping it as qstr avoids the unnecessary conversion in f2fs_match > > Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxx> > --- > fs/f2fs/dir.c | 52 ++++++++++++++++++++++++++++------------------ > fs/f2fs/f2fs.h | 3 ++- > fs/f2fs/recovery.c | 5 +---- > 3 files changed, 35 insertions(+), 25 deletions(-) > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > index 166f08623362..c2a02003c5b9 100644 > --- a/fs/f2fs/dir.c > +++ b/fs/f2fs/dir.c > @@ -81,28 +81,47 @@ int f2fs_init_casefolded_name(const struct inode *dir, > { > #if IS_ENABLED(CONFIG_UNICODE) > struct super_block *sb = dir->i_sb; > + unsigned char *buf; > + int len; > > if (IS_CASEFOLDED(dir)) { > - fname->cf_name.name = f2fs_kmem_cache_alloc(f2fs_cf_name_slab, > + buf = f2fs_kmem_cache_alloc(f2fs_cf_name_slab, > GFP_NOFS, false, F2FS_SB(sb)); > - if (!fname->cf_name.name) > - return -ENOMEM; > - fname->cf_name.len = utf8_casefold(sb->s_encoding, > - fname->usr_fname, > - fname->cf_name.name, > - F2FS_NAME_LEN); > - if ((int)fname->cf_name.len <= 0) { > - kmem_cache_free(f2fs_cf_name_slab, fname->cf_name.name); > + if (!buf) { > fname->cf_name.name = NULL; > + return -ENOMEM; > + } > + > + len = utf8_casefold(sb->s_encoding, fname->usr_fname, > + buf, F2FS_NAME_LEN); > + > + if (len <= 0) { > + kmem_cache_free(f2fs_cf_name_slab, buf); > + buf = NULL; > if (sb_has_strict_encoding(sb)) > return -EINVAL; > /* fall back to treating name as opaque byte sequence */ > } > + fname->cf_name.name = buf; > + fname->cf_name.len = (unsigned int) len; > } > #endif There's some inconsistent behavior above; now sometimes fname->cf_name.name is set to NULL on failure and sometime it's not. Also now fname->cf_name.len can be set to a negative value. Since struct f2fs_filename is always zero-initialized, how about only setting the fname->cf_name fields if we actually have a valid value to assign? I.e. buf = f2fs_kmem_cache_alloc(f2fs_cf_name_slab, GFP_NOFS, false, F2FS_SB(sb)); if (!buf) return -ENOMEM; len = utf8_casefold(sb->s_encoding, fname->usr_fname, buf, F2FS_NAME_LEN); if (len <= 0) { kmem_cache_free(f2fs_cf_name_slab, buf); if (sb_has_strict_encoding(sb)) return -EINVAL; /* fall back to treating name as opaque byte sequence */ return 0; } fname->cf_name.name = buf; fname->cf_name.len = len; > +void f2fs_free_casefolded_name(struct f2fs_filename *fname) > +{ > +#if IS_ENABLED(CONFIG_UNICODE) > + unsigned char *buf = (unsigned char *)fname->cf_name.name; > + > + if (buf) { > + kmem_cache_free(f2fs_cf_name_slab, buf); > + fname->cf_name.name = NULL; > + } > + > +#endif > +} Kernel code usually uses static inline stubs for the !CONFIG_$FOO case in cases like this, as that causes the function calls to be compiled away to nothing when they're unneeded. - Eric