From e7cda1ee94f464e02e356319aec7d83ccba8cab4 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Tue, 26 May 2020 17:03:43 +0800 Subject: [PATCH 1/3] erofs: code cleanup by removing ifdef macro surrounding Define erofs_listxattr and erofs_xattr_handlers to NULL when CONFIG_EROFS_FS_XATTR is not enabled, then we can remove many ugly ifdef macros in the code. Signed-off-by: Chengguang Xu Reviewed-by: Gao Xiang Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20200526090343.22794-1-cgxu519@mykernel.net Signed-off-by: Gao Xiang --- fs/erofs/inode.c | 6 ------ fs/erofs/namei.c | 2 -- fs/erofs/super.c | 4 +--- fs/erofs/xattr.h | 7 ++----- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c index 3350ab65d892..7dd4bbe9674f 100644 --- a/fs/erofs/inode.c +++ b/fs/erofs/inode.c @@ -311,27 +311,21 @@ int erofs_getattr(const struct path *path, struct kstat *stat, const struct inode_operations erofs_generic_iops = { .getattr = erofs_getattr, -#ifdef CONFIG_EROFS_FS_XATTR .listxattr = erofs_listxattr, -#endif .get_acl = erofs_get_acl, }; const struct inode_operations erofs_symlink_iops = { .get_link = page_get_link, .getattr = erofs_getattr, -#ifdef CONFIG_EROFS_FS_XATTR .listxattr = erofs_listxattr, -#endif .get_acl = erofs_get_acl, }; const struct inode_operations erofs_fast_symlink_iops = { .get_link = simple_get_link, .getattr = erofs_getattr, -#ifdef CONFIG_EROFS_FS_XATTR .listxattr = erofs_listxattr, -#endif .get_acl = erofs_get_acl, }; diff --git a/fs/erofs/namei.c b/fs/erofs/namei.c index 3abbecbf73de..52f201e03c62 100644 --- a/fs/erofs/namei.c +++ b/fs/erofs/namei.c @@ -244,9 +244,7 @@ static struct dentry *erofs_lookup(struct inode *dir, const struct inode_operations erofs_dir_iops = { .lookup = erofs_lookup, .getattr = erofs_getattr, -#ifdef CONFIG_EROFS_FS_XATTR .listxattr = erofs_listxattr, -#endif .get_acl = erofs_get_acl, }; diff --git a/fs/erofs/super.c b/fs/erofs/super.c index b514c67e5fc2..8e46d204a0c2 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -408,10 +408,8 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent) sb->s_time_gran = 1; sb->s_op = &erofs_sops; - -#ifdef CONFIG_EROFS_FS_XATTR sb->s_xattr = erofs_xattr_handlers; -#endif + /* set erofs default mount options */ erofs_default_options(sbi); diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h index 50966f1c676e..e4e5093f012c 100644 --- a/fs/erofs/xattr.h +++ b/fs/erofs/xattr.h @@ -76,11 +76,8 @@ static inline int erofs_getxattr(struct inode *inode, int index, return -EOPNOTSUPP; } -static inline ssize_t erofs_listxattr(struct dentry *dentry, - char *buffer, size_t buffer_size) -{ - return -EOPNOTSUPP; -} +#define erofs_listxattr (NULL) +#define erofs_xattr_handlers (NULL) #endif /* !CONFIG_EROFS_FS_XATTR */ #ifdef CONFIG_EROFS_FS_POSIX_ACL From f57a3fe44995a3820192e0cf7c3ebdecedd9586e Mon Sep 17 00:00:00 2001 From: Chao Yu Date: Fri, 29 May 2020 18:48:36 +0800 Subject: [PATCH 2/3] erofs: convert to use the new mount fs_context api Convert the erofs to use new internal mount API as the old one will be obsoleted and removed. This allows greater flexibility in communication of mount parameters between userspace, the VFS and the filesystem. See Documentation/filesystems/mount_api.txt for more information. Cc: Al Viro Cc: David Howells Signed-off-by: Chao Yu Link: https://lore.kernel.org/r/20200529104836.17843-1-hsiangkao@redhat.com Signed-off-by: Gao Xiang --- fs/erofs/internal.h | 27 +++-- fs/erofs/super.c | 253 ++++++++++++++++++++------------------------ fs/erofs/xattr.c | 4 +- fs/erofs/zdata.c | 4 +- 4 files changed, 132 insertions(+), 156 deletions(-) diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 5eead7fdc7a6..1c077b7bb43d 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -46,6 +46,17 @@ typedef u64 erofs_off_t; /* data type for filesystem-wide blocks number */ typedef u32 erofs_blk_t; +struct erofs_fs_context { +#ifdef CONFIG_EROFS_FS_ZIP + /* current strategy of how to use managed cache */ + unsigned char cache_strategy; + + /* threshold for decompression synchronously */ + unsigned int max_sync_decompress_pages; +#endif + unsigned int mount_opt; +}; + struct erofs_sb_info { #ifdef CONFIG_EROFS_FS_ZIP /* list for all registered superblocks, mainly for shrinker */ @@ -55,14 +66,8 @@ struct erofs_sb_info { /* managed XArray arranged in physical block number */ struct xarray managed_pslots; - /* threshold for decompression synchronously */ - unsigned int max_sync_decompress_pages; - unsigned int shrinker_run_no; - /* current strategy of how to use managed cache */ - unsigned char cache_strategy; - /* pseudo inode to manage cached pages */ struct inode *managed_cache; #endif /* CONFIG_EROFS_FS_ZIP */ @@ -88,7 +93,7 @@ struct erofs_sb_info { u32 feature_compat; u32 feature_incompat; - unsigned int mount_opt; + struct erofs_fs_context ctx; /* options */ }; #define EROFS_SB(sb) ((struct erofs_sb_info *)(sb)->s_fs_info) @@ -98,17 +103,17 @@ struct erofs_sb_info { #define EROFS_MOUNT_XATTR_USER 0x00000010 #define EROFS_MOUNT_POSIX_ACL 0x00000020 -#define clear_opt(sbi, option) ((sbi)->mount_opt &= ~EROFS_MOUNT_##option) -#define set_opt(sbi, option) ((sbi)->mount_opt |= EROFS_MOUNT_##option) -#define test_opt(sbi, option) ((sbi)->mount_opt & EROFS_MOUNT_##option) +#define clear_opt(ctx, option) ((ctx)->mount_opt &= ~EROFS_MOUNT_##option) +#define set_opt(ctx, option) ((ctx)->mount_opt |= EROFS_MOUNT_##option) +#define test_opt(ctx, option) ((ctx)->mount_opt & EROFS_MOUNT_##option) -#ifdef CONFIG_EROFS_FS_ZIP enum { EROFS_ZIP_CACHE_DISABLED, EROFS_ZIP_CACHE_READAHEAD, EROFS_ZIP_CACHE_READAROUND }; +#ifdef CONFIG_EROFS_FS_ZIP #define EROFS_LOCKED_MAGIC (INT_MIN | 0xE0F510CCL) /* basic unit of the workstation of a super_block */ diff --git a/fs/erofs/super.c b/fs/erofs/super.c index 8e46d204a0c2..7a13ffb07c23 100644 --- a/fs/erofs/super.c +++ b/fs/erofs/super.c @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include "xattr.h" #define CREATE_TRACE_POINTS @@ -192,53 +194,18 @@ out: return ret; } -#ifdef CONFIG_EROFS_FS_ZIP -static int erofs_build_cache_strategy(struct super_block *sb, - substring_t *args) -{ - struct erofs_sb_info *sbi = EROFS_SB(sb); - const char *cs = match_strdup(args); - int err = 0; - - if (!cs) { - erofs_err(sb, "Not enough memory to store cache strategy"); - return -ENOMEM; - } - - if (!strcmp(cs, "disabled")) { - sbi->cache_strategy = EROFS_ZIP_CACHE_DISABLED; - } else if (!strcmp(cs, "readahead")) { - sbi->cache_strategy = EROFS_ZIP_CACHE_READAHEAD; - } else if (!strcmp(cs, "readaround")) { - sbi->cache_strategy = EROFS_ZIP_CACHE_READAROUND; - } else { - erofs_err(sb, "Unrecognized cache strategy \"%s\"", cs); - err = -EINVAL; - } - kfree(cs); - return err; -} -#else -static int erofs_build_cache_strategy(struct super_block *sb, - substring_t *args) -{ - erofs_info(sb, "EROFS compression is disabled, so cache strategy is ignored"); - return 0; -} -#endif - /* set up default EROFS parameters */ -static void erofs_default_options(struct erofs_sb_info *sbi) +static void erofs_default_options(struct erofs_fs_context *ctx) { #ifdef CONFIG_EROFS_FS_ZIP - sbi->cache_strategy = EROFS_ZIP_CACHE_READAROUND; - sbi->max_sync_decompress_pages = 3; + ctx->cache_strategy = EROFS_ZIP_CACHE_READAROUND; + ctx->max_sync_decompress_pages = 3; #endif #ifdef CONFIG_EROFS_FS_XATTR - set_opt(sbi, XATTR_USER); + set_opt(ctx, XATTR_USER); #endif #ifdef CONFIG_EROFS_FS_POSIX_ACL - set_opt(sbi, POSIX_ACL); + set_opt(ctx, POSIX_ACL); #endif } @@ -251,73 +218,62 @@ enum { Opt_err }; -static match_table_t erofs_tokens = { - {Opt_user_xattr, "user_xattr"}, - {Opt_nouser_xattr, "nouser_xattr"}, - {Opt_acl, "acl"}, - {Opt_noacl, "noacl"}, - {Opt_cache_strategy, "cache_strategy=%s"}, - {Opt_err, NULL} +static const struct constant_table erofs_param_cache_strategy[] = { + {"disabled", EROFS_ZIP_CACHE_DISABLED}, + {"readahead", EROFS_ZIP_CACHE_READAHEAD}, + {"readaround", EROFS_ZIP_CACHE_READAROUND}, + {} }; -static int erofs_parse_options(struct super_block *sb, char *options) +static const struct fs_parameter_spec erofs_fs_parameters[] = { + fsparam_flag_no("user_xattr", Opt_user_xattr), + fsparam_flag_no("acl", Opt_acl), + fsparam_enum("cache_strategy", Opt_cache_strategy, + erofs_param_cache_strategy), + {} +}; + +static int erofs_fc_parse_param(struct fs_context *fc, + struct fs_parameter *param) { - substring_t args[MAX_OPT_ARGS]; - char *p; - int err; + struct erofs_fs_context *ctx __maybe_unused = fc->fs_private; + struct fs_parse_result result; + int opt; - if (!options) - return 0; + opt = fs_parse(fc, erofs_fs_parameters, param, &result); + if (opt < 0) + return opt; - while ((p = strsep(&options, ","))) { - int token; - - if (!*p) - continue; - - args[0].to = args[0].from = NULL; - token = match_token(p, erofs_tokens, args); - - switch (token) { + switch (opt) { + case Opt_user_xattr: #ifdef CONFIG_EROFS_FS_XATTR - case Opt_user_xattr: - set_opt(EROFS_SB(sb), XATTR_USER); - break; - case Opt_nouser_xattr: - clear_opt(EROFS_SB(sb), XATTR_USER); - break; + if (result.boolean) + set_opt(ctx, XATTR_USER); + else + clear_opt(ctx, XATTR_USER); #else - case Opt_user_xattr: - erofs_info(sb, "user_xattr options not supported"); - break; - case Opt_nouser_xattr: - erofs_info(sb, "nouser_xattr options not supported"); - break; + errorfc(fc, "{,no}user_xattr options not supported"); #endif + break; + case Opt_acl: #ifdef CONFIG_EROFS_FS_POSIX_ACL - case Opt_acl: - set_opt(EROFS_SB(sb), POSIX_ACL); - break; - case Opt_noacl: - clear_opt(EROFS_SB(sb), POSIX_ACL); - break; + if (result.boolean) + set_opt(ctx, POSIX_ACL); + else + clear_opt(ctx, POSIX_ACL); #else - case Opt_acl: - erofs_info(sb, "acl options not supported"); - break; - case Opt_noacl: - erofs_info(sb, "noacl options not supported"); - break; + errorfc(fc, "{,no}acl options not supported"); #endif - case Opt_cache_strategy: - err = erofs_build_cache_strategy(sb, args); - if (err) - return err; - break; - default: - erofs_err(sb, "Unrecognized mount option \"%s\" or missing value", p); - return -EINVAL; - } + break; + case Opt_cache_strategy: +#ifdef CONFIG_EROFS_FS_ZIP + ctx->cache_strategy = result.uint_32; +#else + errorfc(fc, "compression not supported, cache_strategy ignored"); +#endif + break; + default: + return -ENOPARAM; } return 0; } @@ -381,10 +337,11 @@ static int erofs_init_managed_cache(struct super_block *sb) static int erofs_init_managed_cache(struct super_block *sb) { return 0; } #endif -static int erofs_fill_super(struct super_block *sb, void *data, int silent) +static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc) { struct inode *inode; struct erofs_sb_info *sbi; + struct erofs_fs_context *ctx = fc->fs_private; int err; sb->s_magic = EROFS_SUPER_MAGIC; @@ -410,18 +367,13 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent) sb->s_op = &erofs_sops; sb->s_xattr = erofs_xattr_handlers; - /* set erofs default mount options */ - erofs_default_options(sbi); - - err = erofs_parse_options(sb, data); - if (err) - return err; - - if (test_opt(sbi, POSIX_ACL)) + if (test_opt(ctx, POSIX_ACL)) sb->s_flags |= SB_POSIXACL; else sb->s_flags &= ~SB_POSIXACL; + sbi->ctx = *ctx; + #ifdef CONFIG_EROFS_FS_ZIP xa_init(&sbi->managed_pslots); #endif @@ -448,15 +400,58 @@ static int erofs_fill_super(struct super_block *sb, void *data, int silent) if (err) return err; - erofs_info(sb, "mounted with opts: %s, root inode @ nid %llu.", - (char *)data, ROOT_NID(sbi)); + erofs_info(sb, "mounted with root inode @ nid %llu.", ROOT_NID(sbi)); return 0; } -static struct dentry *erofs_mount(struct file_system_type *fs_type, int flags, - const char *dev_name, void *data) +static int erofs_fc_get_tree(struct fs_context *fc) { - return mount_bdev(fs_type, flags, dev_name, data, erofs_fill_super); + return get_tree_bdev(fc, erofs_fc_fill_super); +} + +static int erofs_fc_reconfigure(struct fs_context *fc) +{ + struct super_block *sb = fc->root->d_sb; + struct erofs_sb_info *sbi = EROFS_SB(sb); + struct erofs_fs_context *ctx = fc->fs_private; + + DBG_BUGON(!sb_rdonly(sb)); + + if (test_opt(ctx, POSIX_ACL)) + fc->sb_flags |= SB_POSIXACL; + else + fc->sb_flags &= ~SB_POSIXACL; + + sbi->ctx = *ctx; + + fc->sb_flags |= SB_RDONLY; + return 0; +} + +static void erofs_fc_free(struct fs_context *fc) +{ + kfree(fc->fs_private); +} + +static const struct fs_context_operations erofs_context_ops = { + .parse_param = erofs_fc_parse_param, + .get_tree = erofs_fc_get_tree, + .reconfigure = erofs_fc_reconfigure, + .free = erofs_fc_free, +}; + +static int erofs_init_fs_context(struct fs_context *fc) +{ + fc->fs_private = kzalloc(sizeof(struct erofs_fs_context), GFP_KERNEL); + if (!fc->fs_private) + return -ENOMEM; + + /* set default mount options */ + erofs_default_options(fc->fs_private); + + fc->ops = &erofs_context_ops; + + return 0; } /* @@ -495,7 +490,7 @@ static void erofs_put_super(struct super_block *sb) static struct file_system_type erofs_fs_type = { .owner = THIS_MODULE, .name = "erofs", - .mount = erofs_mount, + .init_fs_context = erofs_init_fs_context, .kill_sb = erofs_kill_sb, .fs_flags = FS_REQUIRES_DEV, }; @@ -576,61 +571,37 @@ static int erofs_statfs(struct dentry *dentry, struct kstatfs *buf) static int erofs_show_options(struct seq_file *seq, struct dentry *root) { struct erofs_sb_info *sbi __maybe_unused = EROFS_SB(root->d_sb); + struct erofs_fs_context *ctx __maybe_unused = &sbi->ctx; #ifdef CONFIG_EROFS_FS_XATTR - if (test_opt(sbi, XATTR_USER)) + if (test_opt(ctx, XATTR_USER)) seq_puts(seq, ",user_xattr"); else seq_puts(seq, ",nouser_xattr"); #endif #ifdef CONFIG_EROFS_FS_POSIX_ACL - if (test_opt(sbi, POSIX_ACL)) + if (test_opt(ctx, POSIX_ACL)) seq_puts(seq, ",acl"); else seq_puts(seq, ",noacl"); #endif #ifdef CONFIG_EROFS_FS_ZIP - if (sbi->cache_strategy == EROFS_ZIP_CACHE_DISABLED) { + if (ctx->cache_strategy == EROFS_ZIP_CACHE_DISABLED) seq_puts(seq, ",cache_strategy=disabled"); - } else if (sbi->cache_strategy == EROFS_ZIP_CACHE_READAHEAD) { + else if (ctx->cache_strategy == EROFS_ZIP_CACHE_READAHEAD) seq_puts(seq, ",cache_strategy=readahead"); - } else if (sbi->cache_strategy == EROFS_ZIP_CACHE_READAROUND) { + else if (ctx->cache_strategy == EROFS_ZIP_CACHE_READAROUND) seq_puts(seq, ",cache_strategy=readaround"); - } #endif return 0; } -static int erofs_remount(struct super_block *sb, int *flags, char *data) -{ - struct erofs_sb_info *sbi = EROFS_SB(sb); - unsigned int org_mnt_opt = sbi->mount_opt; - int err; - - DBG_BUGON(!sb_rdonly(sb)); - err = erofs_parse_options(sb, data); - if (err) - goto out; - - if (test_opt(sbi, POSIX_ACL)) - sb->s_flags |= SB_POSIXACL; - else - sb->s_flags &= ~SB_POSIXACL; - - *flags |= SB_RDONLY; - return 0; -out: - sbi->mount_opt = org_mnt_opt; - return err; -} - const struct super_operations erofs_sops = { .put_super = erofs_put_super, .alloc_inode = erofs_alloc_inode, .free_inode = erofs_free_inode, .statfs = erofs_statfs, .show_options = erofs_show_options, - .remount_fs = erofs_remount, }; module_init(erofs_module_init); diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index b766c3ee5fa8..87e437e7b34f 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -422,7 +422,7 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it) static bool erofs_xattr_user_list(struct dentry *dentry) { - return test_opt(EROFS_SB(dentry->d_sb), XATTR_USER); + return test_opt(&EROFS_SB(dentry->d_sb)->ctx, XATTR_USER); } static bool erofs_xattr_trusted_list(struct dentry *dentry) @@ -469,7 +469,7 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler, switch (handler->flags) { case EROFS_XATTR_INDEX_USER: - if (!test_opt(sbi, XATTR_USER)) + if (!test_opt(&sbi->ctx, XATTR_USER)) return -EOPNOTSUPP; break; case EROFS_XATTR_INDEX_TRUSTED: diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index c4b6c9aa87ec..5086b1218aac 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -615,7 +615,7 @@ restart_now: goto err_out; /* preload all compressed pages (maybe downgrade role if necessary) */ - if (should_alloc_managed_pages(fe, sbi->cache_strategy, map->m_la)) + if (should_alloc_managed_pages(fe, sbi->ctx.cache_strategy, map->m_la)) cache_strategy = DELAYEDALLOC; else cache_strategy = DONTALLOC; @@ -1302,7 +1302,7 @@ static int z_erofs_readpage(struct file *file, struct page *page) static bool should_decompress_synchronously(struct erofs_sb_info *sbi, unsigned int nr) { - return nr <= sbi->max_sync_decompress_pages; + return nr <= sbi->ctx.max_sync_decompress_pages; } static int z_erofs_readpages(struct file *filp, struct address_space *mapping, From 34f853b849eb6a509eb8f40f2f5946ebb1f62739 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Thu, 28 May 2020 04:48:44 -0400 Subject: [PATCH 3/3] erofs: suppress false positive last_block warning As Andrew mentioned, some rare specific gcc versions could report last_block uninitialized warning. Actually last_block doesn't need to be uninitialized first from its implementation due to bio == NULL condition. After a bio is allocated, last_block will be assigned then. The detailed analysis is in this thread [1]. So let's silence those confusing gccs simply. [1] https://lore.kernel.org/r/20200421072839.GA13867@hsiangkao-HP-ZHAN-66-Pro-G1 Cc: Andrew Morton Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20200528084844.23359-1-hsiangkao@redhat.com Signed-off-by: Gao Xiang --- fs/erofs/data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/erofs/data.c b/fs/erofs/data.c index fc3a8d8064f8..2812645b361e 100644 --- a/fs/erofs/data.c +++ b/fs/erofs/data.c @@ -265,7 +265,7 @@ submit_bio_out: */ static int erofs_raw_access_readpage(struct file *file, struct page *page) { - erofs_off_t last_block; + erofs_off_t uninitialized_var(last_block); struct bio *bio; trace_erofs_readpage(page, true); @@ -285,7 +285,7 @@ static int erofs_raw_access_readpages(struct file *filp, struct list_head *pages, unsigned int nr_pages) { - erofs_off_t last_block; + erofs_off_t uninitialized_var(last_block); struct bio *bio = NULL; gfp_t gfp = readahead_gfp_mask(mapping); struct page *page = list_last_entry(pages, struct page, lru);