2022-05-11 20:17:42

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v4 10/10] f2fs: Move CONFIG_UNICODE defguards into the code flow

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 <[email protected]>
---
fs/f2fs/namei.c | 12 ++++++------
fs/f2fs/super.c | 22 ++++++++++++----------
2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 5f213f05556d..843e4102347d 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -561,8 +561,8 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
goto out_iput;
}
out_splice:
-#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
@@ -571,7 +571,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
trace_f2fs_lookup_end(dir, dentry, ino, err);
return NULL;
}
-#endif
+
new = d_splice_alias(inode, dentry);
err = PTR_ERR_OR_ZERO(new);
trace_f2fs_lookup_end(dir, dentry, ino, !new ? -ENOENT : err);
@@ -622,16 +622,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
goto fail;
}
f2fs_delete_entry(de, page, dir, inode);
-#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 f2fs_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
+
f2fs_unlock_op(sbi);

if (IS_DIRSYNC(dir))
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index baefd398ec1a..c336760ff743 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -278,12 +278,13 @@ f2fs_sb_read_encoding(const struct f2fs_super_block *sb)

return NULL;
}
+#endif

struct kmem_cache *f2fs_cf_name_slab;
static int __init f2fs_create_casefold_cache(void)
{
f2fs_cf_name_slab = f2fs_kmem_cache_create("f2fs_casefolded_name",
- F2FS_NAME_LEN);
+ F2FS_NAME_LEN);
if (!f2fs_cf_name_slab)
return -ENOMEM;
return 0;
@@ -293,10 +294,6 @@ static void f2fs_destroy_casefold_cache(void)
{
kmem_cache_destroy(f2fs_cf_name_slab);
}
-#else
-static int __init f2fs_create_casefold_cache(void) { return 0; }
-static void f2fs_destroy_casefold_cache(void) { }
-#endif

static inline void limit_reserve_root(struct f2fs_sb_info *sbi)
{
@@ -1259,13 +1256,13 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
#endif
-#if !IS_ENABLED(CONFIG_UNICODE)
- if (f2fs_sb_has_casefold(sbi)) {
+
+ if (!IS_ENABLED(CONFIG_UNICODE) && f2fs_sb_has_casefold(sbi)) {
f2fs_err(sbi,
"Filesystem with casefold feature cannot be mounted without CONFIG_UNICODE");
return -EINVAL;
}
-#endif
+
/*
* The BLKZONED feature indicates that the drive was formatted with
* zone alignment optimization. This is optional for host-aware
@@ -4611,7 +4608,10 @@ static int __init init_f2fs_fs(void)
err = f2fs_init_compress_cache();
if (err)
goto free_compress_mempool;
- err = f2fs_create_casefold_cache();
+
+ if (IS_ENABLED(CONFIG_UNICODE))
+ err = f2fs_create_casefold_cache();
+
if (err)
goto free_compress_cache;
return 0;
@@ -4654,7 +4654,9 @@ static int __init init_f2fs_fs(void)

static void __exit exit_f2fs_fs(void)
{
- f2fs_destroy_casefold_cache();
+ if (IS_ENABLED(CONFIG_UNICODE))
+ f2fs_destroy_casefold_cache();
+
f2fs_destroy_compress_cache();
f2fs_destroy_compress_mempool();
f2fs_destroy_bioset();
--
2.36.1



2022-05-12 05:18:42

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v4 10/10] f2fs: Move CONFIG_UNICODE defguards into the code flow

On Wed, May 11, 2022 at 03:31:46PM -0400, Gabriel Krisman Bertazi wrote:
> @@ -293,10 +294,6 @@ static void f2fs_destroy_casefold_cache(void)
> {
> kmem_cache_destroy(f2fs_cf_name_slab);
> }
> -#else
> -static int __init f2fs_create_casefold_cache(void) { return 0; }
> -static void f2fs_destroy_casefold_cache(void) { }
> -#endif
[...]
> @@ -4611,7 +4608,10 @@ static int __init init_f2fs_fs(void)
> err = f2fs_init_compress_cache();
> if (err)
> goto free_compress_mempool;
> - err = f2fs_create_casefold_cache();
> +
> + if (IS_ENABLED(CONFIG_UNICODE))
> + err = f2fs_create_casefold_cache();
> +
> if (err)
> goto free_compress_cache;
> return 0;
> @@ -4654,7 +4654,9 @@ static int __init init_f2fs_fs(void)
>
> static void __exit exit_f2fs_fs(void)
> {
> - f2fs_destroy_casefold_cache();
> + if (IS_ENABLED(CONFIG_UNICODE))
> + f2fs_destroy_casefold_cache();
> +

I don't think the above two changes are actually an improvement. It's cleaner
to use stub functions to keep the callers simpler, as the original code did.

- Eric