lkml.org 
[lkml]   [2022]   [Nov]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [External] [PATCH] erofs: check the uniqueness of fsid in shared domain in advance
    From
    Hi Tao,
    We've noticed this warning during development. It seems not a bug, so
    I ignored that.
    Many thanks for cacthing and clearing up the annoying warning.

    在 2022/11/25 15:40, Hou Tao 写道:
    > From: Hou Tao <houtao1@huawei.com>
    >
    > When shared domain is enabled, doing mount twice with the same fsid and
    > domain_id will trigger sysfs warning as shown below:
    >
    > sysfs: cannot create duplicate filename '/fs/erofs/d0,meta.bin'
    > CPU: 15 PID: 1051 Comm: mount Not tainted 6.1.0-rc6+ #1
    > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
    > Call Trace:
    > <TASK>
    > dump_stack_lvl+0x38/0x49
    > dump_stack+0x10/0x12
    > sysfs_warn_dup.cold+0x17/0x27
    > sysfs_create_dir_ns+0xb8/0xd0
    > kobject_add_internal+0xb1/0x240
    > kobject_init_and_add+0x71/0xa0
    > erofs_register_sysfs+0x89/0x110
    > erofs_fc_fill_super+0x98c/0xaf0
    > vfs_get_super+0x7d/0x100
    > get_tree_nodev+0x16/0x20
    > erofs_fc_get_tree+0x20/0x30
    > vfs_get_tree+0x24/0xb0
    > path_mount+0x2fa/0xa90
    > do_mount+0x7c/0xa0
    > __x64_sys_mount+0x8b/0xe0
    > do_syscall_64+0x30/0x60
    > entry_SYSCALL_64_after_hwframe+0x46/0xb0
    >
    > The reason is erofs_fscache_register_cookie() doesn't guarantee the primary
    > data blob (aka fsid) is unique in the shared domain and
    > erofs_register_sysfs() invoked by the second mount will fail due to the
    > duplicated fsid in the shared domain and report warning.
    >
    > It would be better to check the uniqueness of fsid before doing
    > erofs_register_sysfs(), so adding a new flags parameter for
    > erofs_fscache_register_cookie() and doing the uniqueness check if
    > EROFS_REG_COOKIE_NEED_NOEXIST is enabled.
    >
    > After the patch, the error in dmesg for the duplicated mount would be:
    >
    > erofs: ...: erofs_domain_register_cookie: XX already exists in domain YY
    >
    > Signed-off-by: Hou Tao <houtao1@huawei.com>
    > ---
    > fs/erofs/fscache.c | 47 +++++++++++++++++++++++++++++++++------------
    > fs/erofs/internal.h | 10 ++++++++--
    > fs/erofs/super.c | 2 +-
    > 3 files changed, 44 insertions(+), 15 deletions(-)
    >
    > diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
    > index af5ed6b9c54d..6a792a513d6b 100644
    > --- a/fs/erofs/fscache.c
    > +++ b/fs/erofs/fscache.c
    > @@ -494,7 +494,8 @@ static int erofs_fscache_register_domain(struct super_block *sb)
    >
    > static
    > struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
    > - char *name, bool need_inode)
    > + char *name,
    > + unsigned int flags)
    > {
    > struct fscache_volume *volume = EROFS_SB(sb)->volume;
    > struct erofs_fscache *ctx;
    > @@ -516,7 +517,7 @@ struct erofs_fscache *erofs_fscache_acquire_cookie(struct super_block *sb,
    > fscache_use_cookie(cookie, false);
    > ctx->cookie = cookie;
    >
    > - if (need_inode) {
    > + if (flags & EROFS_REG_COOKIE_NEED_INODE) {
    > struct inode *const inode = new_inode(sb);
    >
    > if (!inode) {
    > @@ -554,14 +555,15 @@ static void erofs_fscache_relinquish_cookie(struct erofs_fscache *ctx)
    >
    > static
    > struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
    > - char *name, bool need_inode)
    > + char *name,
    > + unsigned int flags)
    > {
    > int err;
    > struct inode *inode;
    > struct erofs_fscache *ctx;
    > struct erofs_domain *domain = EROFS_SB(sb)->domain;
    >
    > - ctx = erofs_fscache_acquire_cookie(sb, name, need_inode);
    > + ctx = erofs_fscache_acquire_cookie(sb, name, flags);
    > if (IS_ERR(ctx))
    > return ctx;
    >
    > @@ -589,7 +591,8 @@ struct erofs_fscache *erofs_fscache_domain_init_cookie(struct super_block *sb,
    >
    > static
    > struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
    > - char *name, bool need_inode)
    > + char *name,
    > + unsigned int flags)
    > {
    > struct inode *inode;
    > struct erofs_fscache *ctx;
    > @@ -602,23 +605,30 @@ struct erofs_fscache *erofs_domain_register_cookie(struct super_block *sb,
    > ctx = inode->i_private;
    > if (!ctx || ctx->domain != domain || strcmp(ctx->name, name))
    > continue;
    > - igrab(inode);
    > + if (!(flags & EROFS_REG_COOKIE_NEED_NOEXIST)) {
    > + igrab(inode);
    > + } else {
    > + erofs_err(sb, "%s already exists in domain %s", name,
    > + domain->domain_id);
    > + ctx = ERR_PTR(-EEXIST);
    > + }
    > spin_unlock(&psb->s_inode_list_lock);
    > mutex_unlock(&erofs_domain_cookies_lock);
    > return ctx;
    > }
    > spin_unlock(&psb->s_inode_list_lock);
    > - ctx = erofs_fscache_domain_init_cookie(sb, name, need_inode);
    > + ctx = erofs_fscache_domain_init_cookie(sb, name, flags);
    > mutex_unlock(&erofs_domain_cookies_lock);
    > return ctx;
    > }
    >
    > struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
    > - char *name, bool need_inode)
    > + char *name,
    > + unsigned int flags)
    > {
    > if (EROFS_SB(sb)->domain_id)
    > - return erofs_domain_register_cookie(sb, name, need_inode);
    > - return erofs_fscache_acquire_cookie(sb, name, need_inode);
    > + return erofs_domain_register_cookie(sb, name, flags);
    > + return erofs_fscache_acquire_cookie(sb, name, flags);
    > }
    >
    > void erofs_fscache_unregister_cookie(struct erofs_fscache *ctx)
    > @@ -647,6 +657,7 @@ int erofs_fscache_register_fs(struct super_block *sb)
    > int ret;
    > struct erofs_sb_info *sbi = EROFS_SB(sb);
    > struct erofs_fscache *fscache;
    > + unsigned int flags;
    >
    > if (sbi->domain_id)
    > ret = erofs_fscache_register_domain(sb);
    > @@ -655,8 +666,20 @@ int erofs_fscache_register_fs(struct super_block *sb)
    > if (ret)
    > return ret;
    >
    > - /* acquired domain/volume will be relinquished in kill_sb() on error */
    > - fscache = erofs_fscache_register_cookie(sb, sbi->fsid, true);
    > + /*
    > + * When shared domain is enabled, using NEED_NOEXIST to guarantee
    > + * the primary data blob (aka fsid) is unique in the shared domain.
    > + *
    > + * For non-shared-domain case, fscache_acquire_volume() invoked by
    > + * erofs_fscache_register_volume() has already guaranteed
    > + * the uniqueness of primary data blob.
    > + *
    > + * Acquired domain/volume will be relinquished in kill_sb() on error.
    > + */
    > + flags = EROFS_REG_COOKIE_NEED_INODE;
    > + if (sbi->domain_id)
    > + flags |= EROFS_REG_COOKIE_NEED_NOEXIST;
    > + fscache = erofs_fscache_register_cookie(sb, sbi->fsid, flags);
    > if (IS_ERR(fscache))
    > return PTR_ERR(fscache);
    >
    > diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
    > index 05dc68627722..a5f8c8fcdd17 100644
    > --- a/fs/erofs/internal.h
    > +++ b/fs/erofs/internal.h
    > @@ -604,13 +604,18 @@ static inline int z_erofs_load_lzma_config(struct super_block *sb,
    > }
    > #endif /* !CONFIG_EROFS_FS_ZIP */
    >
    > +/* flags for erofs_fscache_register_cookie() */
    > +#define EROFS_REG_COOKIE_NEED_INODE 1
    > +#define EROFS_REG_COOKIE_NEED_NOEXIST 2
    How about using TAB to align the columns?
    Apart from this, LGTM.
    Reviewed-by: Jia Zhu <zhujia.zj@bytedance.com>
    Thanks.
    > +
    > /* fscache.c */
    > #ifdef CONFIG_EROFS_FS_ONDEMAND
    > int erofs_fscache_register_fs(struct super_block *sb);
    > void erofs_fscache_unregister_fs(struct super_block *sb);
    >
    > struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
    > - char *name, bool need_inode);
    > + char *name,
    > + unsigned int flags);
    > void erofs_fscache_unregister_cookie(struct erofs_fscache *fscache);
    >
    > extern const struct address_space_operations erofs_fscache_access_aops;
    > @@ -623,7 +628,8 @@ static inline void erofs_fscache_unregister_fs(struct super_block *sb) {}
    >
    > static inline
    > struct erofs_fscache *erofs_fscache_register_cookie(struct super_block *sb,
    > - char *name, bool need_inode)
    > + char *name,
    > + unsigned int flags)
    > {
    > return ERR_PTR(-EOPNOTSUPP);
    > }
    > diff --git a/fs/erofs/super.c b/fs/erofs/super.c
    > index 1c7dcca702b3..481788c24a68 100644
    > --- a/fs/erofs/super.c
    > +++ b/fs/erofs/super.c
    > @@ -245,7 +245,7 @@ static int erofs_init_device(struct erofs_buf *buf, struct super_block *sb,
    > }
    >
    > if (erofs_is_fscache_mode(sb)) {
    > - fscache = erofs_fscache_register_cookie(sb, dif->path, false);
    > + fscache = erofs_fscache_register_cookie(sb, dif->path, 0);
    > if (IS_ERR(fscache))
    > return PTR_ERR(fscache);
    > dif->fscache = fscache;

    \
     
     \ /
      Last update: 2022-11-25 09:26    [W:7.626 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site