| Message ID | 20240320084622.46643-9-eugen.hristev@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Cache insensitive cleanup for ext4/f2fs | expand |
Eugen Hristev <eugen.hristev@collabora.com> writes: > From: Gabriel Krisman Bertazi <krisman@collabora.com> > > Instead of a bunch of ifdefs, make the unicode built checks part of the > code flow where possible, as requested by Torvalds. > > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> > [eugen.hristev@collabora.com: port to 6.8-rc3] > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com> > --- > fs/ext4/crypto.c | 19 +++---------------- > fs/ext4/ext4.h | 33 +++++++++++++++++++++------------ > fs/ext4/namei.c | 14 +++++--------- > fs/ext4/super.c | 4 +--- > 4 files changed, 30 insertions(+), 40 deletions(-) > > diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c > index 7ae0b61258a7..1d2f8b79529c 100644 > --- a/fs/ext4/crypto.c > +++ b/fs/ext4/crypto.c > @@ -31,12 +31,7 @@ int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname, > > ext4_fname_from_fscrypt_name(fname, &name); > > -#if IS_ENABLED(CONFIG_UNICODE) > - err = ext4_fname_setup_ci_filename(dir, iname, fname); > - if (err) > - ext4_fname_free_filename(fname); > -#endif > - return err; > + return ext4_fname_setup_ci_filename(dir, iname, fname); > } Oops. I ended up replying to v10 but it still applies in the latest version. copying it here for reference: This shouldn't remove the error path. It effectively reintroduces the memory leak fixed by commit 7ca4b085f430 ("ext4: fix memory leaks in ext4_fname_{setup_filename,prepare_lookup}"). This patch was only about inlining the codeguards, so it shouldn't be changing the logic. > int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry, > @@ -51,12 +46,7 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry, > > ext4_fname_from_fscrypt_name(fname, &name); > > -#if IS_ENABLED(CONFIG_UNICODE) > - err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname); > - if (err) > - ext4_fname_free_filename(fname); > -#endif > - return err; > + return ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname); > } > > void ext4_fname_free_filename(struct ext4_filename *fname) > @@ -70,10 +60,7 @@ void ext4_fname_free_filename(struct ext4_filename *fname) > fname->usr_fname = NULL; > fname->disk_name.name = NULL; > > -#if IS_ENABLED(CONFIG_UNICODE) > - kfree(fname->cf_name.name); > - fname->cf_name.name = NULL; > -#endif > + ext4_fname_free_ci_filename(fname); > } > > static bool uuid_is_zero(__u8 u[16]) > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 4061d11b9763..c68f48f706cd 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2740,8 +2740,25 @@ ext4_fsblk_t ext4_inode_to_goal_block(struct inode *); > > #if IS_ENABLED(CONFIG_UNICODE) > extern int ext4_fname_setup_ci_filename(struct inode *dir, > - const struct qstr *iname, > - struct ext4_filename *fname); > + const struct qstr *iname, > + struct ext4_filename *fname); > + > +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname) > +{ > + kfree(fname->cf_name.name); > + fname->cf_name.name = NULL; > +} > +#else > +static inline int ext4_fname_setup_ci_filename(struct inode *dir, > + const struct qstr *iname, > + struct ext4_filename *fname) > +{ > + return 0; > +} > + > +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname) > +{ > +} > #endif > > /* ext4 encryption related stuff goes here crypto.c */ > @@ -2764,16 +2781,11 @@ static inline int ext4_fname_setup_filename(struct inode *dir, > int lookup, > struct ext4_filename *fname) > { > - int err = 0; > fname->usr_fname = iname; > fname->disk_name.name = (unsigned char *) iname->name; > fname->disk_name.len = iname->len; > > -#if IS_ENABLED(CONFIG_UNICODE) > - err = ext4_fname_setup_ci_filename(dir, iname, fname); > -#endif > - > - return err; > + return ext4_fname_setup_ci_filename(dir, iname, fname); > } > > static inline int ext4_fname_prepare_lookup(struct inode *dir, > @@ -2785,10 +2797,7 @@ static inline int ext4_fname_prepare_lookup(struct inode *dir, > > static inline void ext4_fname_free_filename(struct ext4_filename *fname) > { > -#if IS_ENABLED(CONFIG_UNICODE) > - kfree(fname->cf_name.name); > - fname->cf_name.name = NULL; > -#endif > + ext4_fname_free_ci_filename(fname); > } > > static inline int ext4_ioctl_get_encryption_pwsalt(struct file *filp, > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 3268cf45d9db..a5d9e5b01015 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -1834,8 +1834,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi > } > } > > -#if IS_ENABLED(CONFIG_UNICODE) > - if (!inode && IS_CASEFOLDED(dir)) { > + if (IS_ENABLED(CONFIG_UNICODE) && !inode && IS_CASEFOLDED(dir)) { > /* Eventually we want to call d_add_ci(dentry, NULL) > * for negative dentries in the encoding case as > * well. For now, prevent the negative dentry > @@ -1843,7 +1842,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi > */ > return NULL; > } > -#endif > + > return d_splice_alias(inode, dentry); > } > > @@ -3173,16 +3172,14 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry) > ext4_fc_track_unlink(handle, dentry); > retval = ext4_mark_inode_dirty(handle, dir); > > -#if IS_ENABLED(CONFIG_UNICODE) > /* VFS negative dentries are incompatible with Encoding and > * Case-insensitiveness. Eventually we'll want avoid > * invalidating the dentries here, alongside with returning the > * negative dentries at ext4_lookup(), when it is better > * supported by the VFS for the CI case. > */ > - if (IS_CASEFOLDED(dir)) > + if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir)) > d_invalidate(dentry); > -#endif > > end_rmdir: > brelse(bh); > @@ -3284,16 +3281,15 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry) > goto out_trace; > > retval = __ext4_unlink(dir, &dentry->d_name, d_inode(dentry), dentry); > -#if IS_ENABLED(CONFIG_UNICODE) > + > /* VFS negative dentries are incompatible with Encoding and > * Case-insensitiveness. Eventually we'll want avoid > * invalidating the dentries here, alongside with returning the > * negative dentries at ext4_lookup(), when it is better > * supported by the VFS for the CI case. > */ > - if (IS_CASEFOLDED(dir)) > + if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir)) > d_invalidate(dentry); > -#endif > > out_trace: > trace_ext4_unlink_exit(dentry, retval); > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 215b4614eb15..179083728b4b 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -3609,14 +3609,12 @@ int ext4_feature_set_ok(struct super_block *sb, int readonly) > return 0; > } > > -#if !IS_ENABLED(CONFIG_UNICODE) > - if (ext4_has_feature_casefold(sb)) { > + if (!IS_ENABLED(CONFIG_UNICODE) && ext4_has_feature_casefold(sb)) { > ext4_msg(sb, KERN_ERR, > "Filesystem with casefold feature cannot be " > "mounted without CONFIG_UNICODE"); > return 0; > } > -#endif > > if (readonly) > return 1;
diff --git a/fs/ext4/crypto.c b/fs/ext4/crypto.c index 7ae0b61258a7..1d2f8b79529c 100644 --- a/fs/ext4/crypto.c +++ b/fs/ext4/crypto.c @@ -31,12 +31,7 @@ int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname, ext4_fname_from_fscrypt_name(fname, &name); -#if IS_ENABLED(CONFIG_UNICODE) - err = ext4_fname_setup_ci_filename(dir, iname, fname); - if (err) - ext4_fname_free_filename(fname); -#endif - return err; + return ext4_fname_setup_ci_filename(dir, iname, fname); } int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry, @@ -51,12 +46,7 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry, ext4_fname_from_fscrypt_name(fname, &name); -#if IS_ENABLED(CONFIG_UNICODE) - err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname); - if (err) - ext4_fname_free_filename(fname); -#endif - return err; + return ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname); } void ext4_fname_free_filename(struct ext4_filename *fname) @@ -70,10 +60,7 @@ void ext4_fname_free_filename(struct ext4_filename *fname) fname->usr_fname = NULL; fname->disk_name.name = NULL; -#if IS_ENABLED(CONFIG_UNICODE) - kfree(fname->cf_name.name); - fname->cf_name.name = NULL; -#endif + ext4_fname_free_ci_filename(fname); } static bool uuid_is_zero(__u8 u[16]) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 4061d11b9763..c68f48f706cd 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2740,8 +2740,25 @@ ext4_fsblk_t ext4_inode_to_goal_block(struct inode *); #if IS_ENABLED(CONFIG_UNICODE) extern int ext4_fname_setup_ci_filename(struct inode *dir, - const struct qstr *iname, - struct ext4_filename *fname); + const struct qstr *iname, + struct ext4_filename *fname); + +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname) +{ + kfree(fname->cf_name.name); + fname->cf_name.name = NULL; +} +#else +static inline int ext4_fname_setup_ci_filename(struct inode *dir, + const struct qstr *iname, + struct ext4_filename *fname) +{ + return 0; +} + +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname) +{ +} #endif /* ext4 encryption related stuff goes here crypto.c */ @@ -2764,16 +2781,11 @@ static inline int ext4_fname_setup_filename(struct inode *dir, int lookup, struct ext4_filename *fname) { - int err = 0; fname->usr_fname = iname; fname->disk_name.name = (unsigned char *) iname->name; fname->disk_name.len = iname->len; -#if IS_ENABLED(CONFIG_UNICODE) - err = ext4_fname_setup_ci_filename(dir, iname, fname); -#endif - - return err; + return ext4_fname_setup_ci_filename(dir, iname, fname); } static inline int ext4_fname_prepare_lookup(struct inode *dir, @@ -2785,10 +2797,7 @@ static inline int ext4_fname_prepare_lookup(struct inode *dir, static inline void ext4_fname_free_filename(struct ext4_filename *fname) { -#if IS_ENABLED(CONFIG_UNICODE) - kfree(fname->cf_name.name); - fname->cf_name.name = NULL; -#endif + ext4_fname_free_ci_filename(fname); } static inline int ext4_ioctl_get_encryption_pwsalt(struct file *filp, diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 3268cf45d9db..a5d9e5b01015 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1834,8 +1834,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi } } -#if IS_ENABLED(CONFIG_UNICODE) - if (!inode && IS_CASEFOLDED(dir)) { + if (IS_ENABLED(CONFIG_UNICODE) && !inode && IS_CASEFOLDED(dir)) { /* Eventually we want to call d_add_ci(dentry, NULL) * for negative dentries in the encoding case as * well. For now, prevent the negative dentry @@ -1843,7 +1842,7 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi */ return NULL; } -#endif + return d_splice_alias(inode, dentry); } @@ -3173,16 +3172,14 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry) ext4_fc_track_unlink(handle, dentry); retval = ext4_mark_inode_dirty(handle, dir); -#if IS_ENABLED(CONFIG_UNICODE) /* VFS negative dentries are incompatible with Encoding and * Case-insensitiveness. Eventually we'll want avoid * invalidating the dentries here, alongside with returning the * negative dentries at ext4_lookup(), when it is better * supported by the VFS for the CI case. */ - if (IS_CASEFOLDED(dir)) + if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir)) d_invalidate(dentry); -#endif end_rmdir: brelse(bh); @@ -3284,16 +3281,15 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry) goto out_trace; retval = __ext4_unlink(dir, &dentry->d_name, d_inode(dentry), dentry); -#if IS_ENABLED(CONFIG_UNICODE) + /* VFS negative dentries are incompatible with Encoding and * Case-insensitiveness. Eventually we'll want avoid * invalidating the dentries here, alongside with returning the * negative dentries at ext4_lookup(), when it is better * supported by the VFS for the CI case. */ - if (IS_CASEFOLDED(dir)) + if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir)) d_invalidate(dentry); -#endif out_trace: trace_ext4_unlink_exit(dentry, retval); diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 215b4614eb15..179083728b4b 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3609,14 +3609,12 @@ int ext4_feature_set_ok(struct super_block *sb, int readonly) return 0; } -#if !IS_ENABLED(CONFIG_UNICODE) - if (ext4_has_feature_casefold(sb)) { + if (!IS_ENABLED(CONFIG_UNICODE) && ext4_has_feature_casefold(sb)) { ext4_msg(sb, KERN_ERR, "Filesystem with casefold feature cannot be " "mounted without CONFIG_UNICODE"); return 0; } -#endif if (readonly) return 1;