Message-ID: <5aad8772-458d-4040-a8b6-feff924c1d30@huawei.com>
Date: Wed, 24 Dec 2025 17:26:16 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Gao Xiang <hsiangkao@...ux.alibaba.com>
CC: <linux-fsdevel@...r.kernel.org>, <linux-erofs@...ts.ozlabs.org>,
	<linux-kernel@...r.kernel.org>, Chao Yu <chao@...nel.org>, Christian Brauner
	<brauner@...nel.org>, "Darrick J. Wong" <djwong@...nel.org>, Amir Goldstein
	<amir73il@...il.com>, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v11 07/10] erofs: introduce the page cache share feature


Hi, Xiang
On 2025/12/24 16:09, Gao Xiang wrote:
> 
> 
> On 2025/12/24 12:09, Hongbo Li wrote:
>> From: Hongzhen Luo <hongzhen@...ux.alibaba.com>
>>
>> Currently, reading files with different paths (or names) but the same
>> content will consume multiple copies of the page cache, even if the
>> content of these page caches is the same. For example, reading
>> identical files (e.g., *.so files) from two different minor versions of
>> container images will cost multiple copies of the same page cache,
>> since different containers have different mount points. Therefore,
>> sharing the page cache for files with the same content can save memory.
>>
>> This introduces the page cache share feature in erofs. It allocate a
>> deduplicated inode and use its page cache as shared. Reads for files
>> with identical content will ultimately be routed to the page cache of
>> the deduplicated inode. In this way, a single page cache satisfies
>> multiple read requests for different files with the same contents.
>>
>> We introduce inode_share mount option to enable the page sharing mode
>> during mounting.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@...ux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
>> ---
>>   Documentation/filesystems/erofs.rst |   4 +
>>   fs/erofs/Makefile                   |   1 +
>>   fs/erofs/internal.h                 |  31 +++++
>>   fs/erofs/ishare.c                   | 171 ++++++++++++++++++++++++++++
>>   fs/erofs/super.c                    |  52 ++++++++-
>>   fs/erofs/xattr.c                    |  40 ++++++-
>>   fs/erofs/xattr.h                    |  12 +-
>>   7 files changed, 300 insertions(+), 11 deletions(-)
>>   create mode 100644 fs/erofs/ishare.c
>>
>> diff --git a/Documentation/filesystems/erofs.rst 
>> b/Documentation/filesystems/erofs.rst
>> index 08194f194b94..1ef91b967b62 100644
>> --- a/Documentation/filesystems/erofs.rst
>> +++ b/Documentation/filesystems/erofs.rst
>> @@ -128,7 +128,11 @@ device=%s              Specify a path to an extra 
>> device to be used together.
>>   fsid=%s                Specify a filesystem image ID for Fscache 
>> back-end.
>>   domain_id=%s           Specify a domain ID in fscache mode so that 
>> different images
>>                          with the same blobs under a given domain ID 
>> can share storage.
>> +                       Also used for inode page sharing mode which 
>> defines a sharing
>> +                       domain.
>>   fsoffset=%llu          Specify block-aligned filesystem offset for 
>> the primary device.
>> +inode_share            Enable inode page sharing mode. Files with the 
>> same content
>> +                       can reuse page cache under the same domain_id.
> 
> Enable inode page sharing for this filesystem.  Inodes with identical 
> content
> within the same domain ID can share the page cache.
> 
>>   ===================    
>> =========================================================
>>   Sysfs Entries
>> diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
>> index 549abc424763..a80e1762b607 100644
>> --- a/fs/erofs/Makefile
>> +++ b/fs/erofs/Makefile
>> @@ -10,3 +10,4 @@ erofs-$(CONFIG_EROFS_FS_ZIP_ZSTD) += 
>> decompressor_zstd.o
>>   erofs-$(CONFIG_EROFS_FS_ZIP_ACCEL) += decompressor_crypto.o
>>   erofs-$(CONFIG_EROFS_FS_BACKED_BY_FILE) += fileio.o
>>   erofs-$(CONFIG_EROFS_FS_ONDEMAND) += fscache.o
>> +erofs-$(CONFIG_EROFS_FS_PAGE_CACHE_SHARE) += ishare.o
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 99e2857173c3..a2b2434ee3c8 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -179,6 +179,7 @@ struct erofs_sb_info {
>>   #define EROFS_MOUNT_DAX_ALWAYS        0x00000040
>>   #define EROFS_MOUNT_DAX_NEVER        0x00000080
>>   #define EROFS_MOUNT_DIRECT_IO        0x00000100
>> +#define EROFS_MOUNT_INODE_SHARE        0x00000200
>>   #define clear_opt(opt, option)    ((opt)->mount_opt &= 
>> ~EROFS_MOUNT_##option)
>>   #define set_opt(opt, option)    ((opt)->mount_opt |= 
>> EROFS_MOUNT_##option)
>> @@ -269,6 +270,13 @@ static inline u64 erofs_nid_to_ino64(struct 
>> erofs_sb_info *sbi, erofs_nid_t nid)
>>   /* default readahead size of directories */
>>   #define EROFS_DIR_RA_BYTES    16384
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +struct erofs_inode_fingerprint {
>> +    u8 *opaque;
>> +    int size;
>> +};
>> +#endif
>> +
>>   struct erofs_inode {
>>       erofs_nid_t nid;
>> @@ -304,6 +312,16 @@ struct erofs_inode {
>>           };
>>   #endif    /* CONFIG_EROFS_FS_ZIP */
>>       };
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +    struct list_head ishare_list;
>> +    union {
> 
>          /* for each anon shared inode */
>> +        struct {
>> +            struct erofs_inode_fingerprint fingerprint;
>> +            spinlock_t ishare_lock;
>> +        };
> 
>          /* for each real inode */
> 
>> +        struct inode *realinode;
> 
> 
> Sorry, I think this variable indicates the anon inode instead
> of some real inode, so it should be called
> 
>          struct inode *sharedinode;
> 
>> +    };
>> +#endif
>>       /* the corresponding vfs inode */
>>       struct inode vfs_inode;
>>   };
>> @@ -410,6 +428,7 @@ extern const struct inode_operations erofs_dir_iops;
>>   extern const struct file_operations erofs_file_fops;
>>   extern const struct file_operations erofs_dir_fops;
>> +extern const struct file_operations erofs_ishare_fops;
>>   extern const struct iomap_ops z_erofs_iomap_report_ops;
>> @@ -541,6 +560,18 @@ static inline struct bio 
>> *erofs_fscache_bio_alloc(struct erofs_map_dev *mdev) {
>>   static inline void erofs_fscache_submit_bio(struct bio *bio) {}
>>   #endif
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +int __init erofs_init_ishare(void);
>> +void erofs_exit_ishare(void);
>> +bool erofs_ishare_fill_inode(struct inode *inode);
>> +void erofs_ishare_free_inode(struct inode *inode);
>> +#else
>> +static inline int erofs_init_ishare(void) { return 0; }
>> +static inline void erofs_exit_ishare(void) {}
>> +static inline bool erofs_ishare_fill_inode(struct inode *inode) { 
>> return false; }
>> +static inline void erofs_ishare_free_inode(struct inode *inode) {}
>> +#endif // CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +
>>   long erofs_ioctl(struct file *filp, unsigned int cmd, unsigned long 
>> arg);
>>   long erofs_compat_ioctl(struct file *filp, unsigned int cmd,
>>               unsigned long arg);
>> diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
>> new file mode 100644
>> index 000000000000..09ea456f2eab
>> --- /dev/null
>> +++ b/fs/erofs/ishare.c
>> @@ -0,0 +1,171 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024, Alibaba Cloud
>> + */
>> +#include <linux/xxhash.h>
>> +#include <linux/mount.h>
>> +#include "internal.h"
>> +#include "xattr.h"
>> +
>> +#include "../internal.h"
>> +
>> +static struct vfsmount *erofs_ishare_mnt;
>> +
>> +static int erofs_ishare_iget5_eq(struct inode *inode, void *data)
>> +{
>> +    struct erofs_inode_fingerprint *fp1 = &EROFS_I(inode)->fingerprint;
>> +    struct erofs_inode_fingerprint *fp2 = data;
>> +
>> +    return fp1->size == fp2->size &&
>> +        !memcmp(fp1->opaque, fp2->opaque, fp2->size);
>> +}
>> +
>> +static int erofs_ishare_iget5_set(struct inode *inode, void *data)
>> +{
>> +    struct erofs_inode *vi = EROFS_I(inode);
>> +
>> +    vi->fingerprint = *(struct erofs_inode_fingerprint *)data;
>> +    INIT_LIST_HEAD(&vi->ishare_list);
>> +    spin_lock_init(&vi->ishare_lock);
>> +    return 0;
>> +}
>> +
>> +bool erofs_ishare_fill_inode(struct inode *inode)
>> +{
>> +    struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
>> +    struct erofs_inode *vi = EROFS_I(inode);
>> +    struct erofs_inode_fingerprint fp;
>> +    struct inode *idedup;
> 
> maybe just aligned with the term above
> 
>      struct inode *sharedinode;
> 
>> +    unsigned long hash;
>> +
>> +    if (!test_opt(&sbi->opt, INODE_SHARE))
>> +        return false;
>> +    fp = erofs_xattr_get_ishare_fp(inode, sbi->domain_id);
>> +    if (!fp.size)
>> +        return false;
>> +    hash = xxh32(fp.opaque, fp.size, 0);
>> +    idedup = iget5_locked(erofs_ishare_mnt->mnt_sb, hash,
>> +                  erofs_ishare_iget5_eq, erofs_ishare_iget5_set,
>> +                  &fp);
>> +    if (!idedup) {
>> +        kfree(fp.opaque);
>> +        return false;
>> +    }
>> +
>> +    INIT_LIST_HEAD(&vi->ishare_list);
> 
> I think we could move this line after the following if block
> and just before spin_lock.
> 
>> +    vi->realinode = idedup;
>> +    if (inode_state_read_once(idedup) & I_NEW) {
>> +        if (erofs_inode_is_data_compressed(vi->datalayout))
>> +            idedup->i_mapping->a_ops = &z_erofs_aops;
>> +        else
>> +            idedup->i_mapping->a_ops = &erofs_aops;
>> +        idedup->i_mode = vi->vfs_inode.i_mode;
>> +        idedup->i_size = vi->vfs_inode.i_size;
>> +        unlock_new_inode(idedup);
>> +    } else {
>> +        kfree(fp.opaque);
>> +    }
>> +    spin_lock(&EROFS_I(idedup)->ishare_lock);> +    
>> list_add(&vi->ishare_list, &EROFS_I(idedup)->ishare_list);
>> +    spin_unlock(&EROFS_I(idedup)->ishare_lock);
>> +    return true;
>> +}
>> +
>> +void erofs_ishare_free_inode(struct inode *inode)
>> +{
>> +    struct erofs_inode *vi = EROFS_I(inode);
>> +    struct inode *idedup = vi->realinode;
> 
>      struct inode *sharedinode = vi->sharedinode;
> 
>> +
>> +    if (!idedup)
>> +        return;
>> +    spin_lock(&EROFS_I(idedup)->ishare_lock);
>> +    list_del(&vi->ishare_list);
>> +    spin_unlock(&EROFS_I(idedup)->ishare_lock);
>> +    iput(idedup);
>> +    vi->realinode = NULL;
>> +}
>> +
>> +static int erofs_ishare_file_open(struct inode *inode, struct file 
>> *file)
>> +{
>> +    struct file *realfile;
>> +    struct inode *dedup;
> 
> same here, sharedinode;
> 
>> +
>> +    dedup = EROFS_I(inode)->realinode;
>> +    realfile = alloc_empty_backing_file(O_RDONLY|O_NOATIME, 
>> current_cred());
>> +    if (IS_ERR(realfile))
>> +        return PTR_ERR(realfile);
>> +    ihold(dedup);
>> +    realfile->f_op = &erofs_file_fops;
>> +    realfile->f_inode = dedup;
>> +    realfile->f_mapping = dedup->i_mapping;
>> +    path_get(&file->f_path);
>> +    backing_file_set_user_path(realfile, &file->f_path);
>> +
>> +    file_ra_state_init(&realfile->f_ra, file->f_mapping);
>> +    realfile->private_data = EROFS_I(inode);
>> +    file->private_data = realfile;
>> +    return 0;
>> +}
>> +
>> +static int erofs_ishare_file_release(struct inode *inode, struct file 
>> *file)
>> +{
>> +    struct file *realfile = file->private_data;
>> +
>> +    iput(realfile->f_inode);
>> +    fput(realfile);
>> +    file->private_data = NULL;
>> +    return 0;
>> +}
>> +
>> +static ssize_t erofs_ishare_file_read_iter(struct kiocb *iocb,
>> +                            struct iov_iter *to)
>> +{
>> +    struct file *realfile = iocb->ki_filp->private_data;
>> +    struct inode *inode = file_inode(iocb->ki_filp);
>> +    struct kiocb dedup_iocb;
>> +    ssize_t nread;
>> +
>> +    if (!iov_iter_count(to))
>> +        return 0;
>> +
>> +    /* fallback to the original file in DAX or DIRECT mode */
> 
> I think we should just disallow FSDAX if inode_share is on.
> 
> 
>> +    if (IS_DAX(inode) || (iocb->ki_flags & IOCB_DIRECT))
>> +        realfile = iocb->ki_filp;
>> +
>> +    kiocb_clone(&dedup_iocb, iocb, realfile);
>> +    nread = filemap_read(&dedup_iocb, to, 0);
>> +    iocb->ki_pos = dedup_iocb.ki_pos;
>> +    file_accessed(iocb->ki_filp);
>> +    return nread;
>> +}
>> +
>> +static int erofs_ishare_mmap(struct file *file, struct vm_area_struct 
>> *vma)
>> +{
>> +    struct file *realfile = file->private_data;
>> +
>> +    vma_set_file(vma, realfile);
>> +    return generic_file_readonly_mmap(file, vma);
>> +}
>> +
>> +const struct file_operations erofs_ishare_fops = {
>> +    .open        = erofs_ishare_file_open,
>> +    .llseek        = generic_file_llseek,
>> +    .read_iter    = erofs_ishare_file_read_iter,
>> +    .mmap        = erofs_ishare_mmap,
>> +    .release    = erofs_ishare_file_release,
>> +    .get_unmapped_area = thp_get_unmapped_area,
>> +    .splice_read    = filemap_splice_read,
>> +};
>> +
>> +int __init erofs_init_ishare(void)
>> +{
>> +    erofs_ishare_mnt = kern_mount(&erofs_anon_fs_type);
>> +    if (IS_ERR(erofs_ishare_mnt))
>> +        return PTR_ERR(erofs_ishare_mnt);
>> +    return 0;
>> +}
>> +
>> +void erofs_exit_ishare(void)
>> +{
>> +    kern_unmount(erofs_ishare_mnt);
>> +}
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 819ab0ae9950..b3b0dedcadab 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -390,6 +390,7 @@ static void erofs_default_options(struct 
>> erofs_sb_info *sbi)
>>   enum {
>>       Opt_user_xattr, Opt_acl, Opt_cache_strategy, Opt_dax, Opt_dax_enum,
>>       Opt_device, Opt_fsid, Opt_domain_id, Opt_directio, Opt_fsoffset,
>> +    Opt_inode_share,
>>   };
>>   static const struct constant_table erofs_param_cache_strategy[] = {
>> @@ -417,6 +418,7 @@ static const struct fs_parameter_spec 
>> erofs_fs_parameters[] = {
>>       fsparam_string("domain_id",    Opt_domain_id),
>>       fsparam_flag_no("directio",    Opt_directio),
>>       fsparam_u64("fsoffset",        Opt_fsoffset),
>> +    fsparam_flag("inode_share",    Opt_inode_share),
>>       {}
>>   };
>> @@ -545,6 +547,14 @@ static int erofs_fc_parse_param(struct fs_context 
>> *fc,
>>       case Opt_fsoffset:
>>           sbi->dif0.fsoff = result.uint_64;
>>           break;
>> +#if defined(CONFIG_EROFS_FS_PAGE_CACHE_SHARE)
>> +    case Opt_inode_share:
>> +        set_opt(&sbi->opt, INODE_SHARE);
>> +#else
>> +    case Opt_inode_share:
>> +        errorfc(fc, "%s option not supported", 
>> erofs_fs_parameters[opt].name);
>> +#endif
>> +        break;
>>       }
>>       return 0;
>>   }
>> @@ -643,6 +653,12 @@ static int erofs_fc_fill_super(struct super_block 
>> *sb, struct fs_context *fc)
>>       sb->s_maxbytes = MAX_LFS_FILESIZE;
>>       sb->s_op = &erofs_sops;
>> +    if (sbi->domain_id &&
>> +        (!sbi->fsid && !test_opt(&sbi->opt, INODE_SHARE))) {
>> +        errorfc(fc, "domain_id should be with fsid or inode_share 
>> option");
>> +        return -EINVAL;
>> +    }
>> +
>>       sbi->blkszbits = PAGE_SHIFT;
>>       if (!sb->s_bdev) {
>>           /*
>> @@ -942,14 +958,34 @@ static struct file_system_type erofs_fs_type = {
>>   };
>>   MODULE_ALIAS_FS("erofs");
>> -#if defined(CONFIG_EROFS_FS_ONDEMAND)
>> +#if defined(CONFIG_EROFS_FS_ONDEMAND) || 
>> defined(CONFIG_EROFS_FS_PAGE_CACHE_SHARE)
>> +static void erofs_free_anon_inode(struct inode *inode)
>> +{
>> +    struct erofs_inode *vi = EROFS_I(inode);
>> +
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +    kfree(vi->fingerprint.opaque);
>> +#endif
>> +    kmem_cache_free(erofs_inode_cachep, vi);
>> +}
>> +
>> +static const struct super_operations erofs_anon_sops = {
>> +    .alloc_inode = erofs_alloc_inode,
>> +    .free_inode = erofs_free_anon_inode,
>> +};
>> +
>>   static int erofs_anon_init_fs_context(struct fs_context *fc)
>>   {
>> -    return init_pseudo(fc, EROFS_SUPER_MAGIC) ? 0 : -ENOMEM;
>> +    struct pseudo_fs_context *ctx;
>> +
>> +    ctx = init_pseudo(fc, EROFS_SUPER_MAGIC);
>> +    if (ctx)
>> +        ctx->ops = &erofs_anon_sops;
>> +
>> +    return ctx ? 0 : -ENOMEM;
>>   }
>>   struct file_system_type erofs_anon_fs_type = {
>> -    .owner        = THIS_MODULE,
>>       .name           = "pseudo_erofs",
>>       .init_fs_context = erofs_anon_init_fs_context,
>>       .kill_sb        = kill_anon_super,
>> @@ -981,6 +1017,10 @@ static int __init erofs_module_init(void)
>>       if (err)
>>           goto sysfs_err;
>> +    err = erofs_init_ishare();
>> +    if (err)
>> +        goto ishare_err;
>> +
>>       err = register_filesystem(&erofs_fs_type);
>>       if (err)
>>           goto fs_err;
>> @@ -988,6 +1028,8 @@ static int __init erofs_module_init(void)
>>       return 0;
>>   fs_err:
>> +    erofs_exit_ishare();
>> +ishare_err:
>>       erofs_exit_sysfs();
>>   sysfs_err:
>>       z_erofs_exit_subsystem();
>> @@ -1005,6 +1047,7 @@ static void __exit erofs_module_exit(void)
>>       /* Ensure all RCU free inodes / pclusters are safe to be 
>> destroyed. */
>>       rcu_barrier();
>> +    erofs_exit_ishare();
>>       erofs_exit_sysfs();
>>       z_erofs_exit_subsystem();
>>       erofs_exit_shrinker();
>> @@ -1057,6 +1100,8 @@ static int erofs_show_options(struct seq_file 
>> *seq, struct dentry *root)
>>           seq_printf(seq, ",domain_id=%s", sbi->domain_id);
>>       if (sbi->dif0.fsoff)
>>           seq_printf(seq, ",fsoffset=%llu", sbi->dif0.fsoff);
>> +    if (test_opt(opt, INODE_SHARE))
>> +        seq_puts(seq, ",inode_share");
>>       return 0;
>>   }
>> @@ -1067,6 +1112,7 @@ static void erofs_evict_inode(struct inode *inode)
>>           dax_break_layout_final(inode);
>>   #endif
>> +    erofs_ishare_free_inode(inode);
>>       truncate_inode_pages_final(&inode->i_data);
>>       clear_inode(inode);
>>   }
>> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
>> index 969e77efd038..7542aee01609 100644
>> --- a/fs/erofs/xattr.c
>> +++ b/fs/erofs/xattr.c
>> @@ -25,6 +25,9 @@ struct erofs_xattr_iter {
>>       struct dentry *dentry;
>>   };
>> +static int erofs_getxattr(struct inode *inode, int index, const char 
>> *name,
>> +            void *buffer, size_t buffer_size);
> 
> could we clean it up in a seperate patch?
> It can be an independent patch against this series.
> 
>> +
>>   static int erofs_init_inode_xattrs(struct inode *inode)
>>   {
>>       struct erofs_inode *const vi = EROFS_I(inode);
>> @@ -391,7 +394,7 @@ static int erofs_xattr_iter_shared(struct 
>> erofs_xattr_iter *it,
>>       return i ? ret : -ENODATA;
>>   }
>> -int erofs_getxattr(struct inode *inode, int index, const char *name,
>> +static int erofs_getxattr(struct inode *inode, int index, const char 
>> *name,
>>              void *buffer, size_t buffer_size)
>>   {
>>       int ret;
>> @@ -577,3 +580,38 @@ struct posix_acl *erofs_get_acl(struct inode 
>> *inode, int type, bool rcu)
>>       return acl;
>>   }
>>   #endif
>> +
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
>> +struct erofs_inode_fingerprint erofs_xattr_get_ishare_fp(struct inode 
>> *inode,
> 
> why return `struct erofs_inode_fingerprint` instead of:
> 
> int erofs_xattr_get_ishare_fp(struct erofs_inode_fingerprint *fp,
>                    struct inode *inode, const char *domain_id)
> 
> instead?
> 

How about declaring this as void erofs_xattr_fill_ishare_fp(struct 
erofs_inode_fingerprint *fp, struct inode *inode, const char 
*domain_id)? Because the return value seems useless.

Thanks,
Hongbo

>> +                            const char *domain_id)
>> +{
>> +    struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
>> +    struct erofs_inode_fingerprint fp = {};
>> +    struct erofs_xattr_prefix_item *prefix;
>> +    const char *infix;
>> +    int valuelen, base_index, domainlen;
>> +
>> +    if (!erofs_sb_has_ishare_xattrs(sbi))
>> +        goto out;
> 
> Just use return 0; instead?
> 
>> +    prefix = sbi->xattr_prefixes + sbi->ishare_xattr_pfx;
>> +    infix = prefix->prefix->infix;
>> +    base_index = prefix->prefix->base_index;
>> +    valuelen = erofs_getxattr(inode, base_index, infix, NULL, 0);
>> +    if (valuelen <= 0 || valuelen > (1 << sbi->blkszbits))
>> +        goto out;
> 
> `return -EFSCORRUPTED` here.
> 
>> +    domainlen = domain_id ? strlen(domain_id) : 0;
>> +    fp.opaque = kmalloc(valuelen + domainlen, GFP_KERNEL);
>> +    if (!fp.opaque)
>> +        goto out;
> 
> `return -ENOMEM;` here.
> 
>> +    if (valuelen != erofs_getxattr(inode, base_index, infix,
>> +                       fp.opaque, valuelen)) {
>> +        kfree(fp.opaque);
>> +        fp.opaque = NULL;
>> +        goto out;
> 
>          return -EFSCORRUPTED;
> 
>> +    }
>> +    memcpy(fp.opaque + valuelen, domain_id, domainlen);
>> +    fp.size = valuelen + domainlen;
>> +out:
>> +    return fp;
> 
>      return 0;
> 
>> +}
>> +#endif
>> diff --git a/fs/erofs/xattr.h b/fs/erofs/xattr.h
>> index 6317caa8413e..32e08ed9cfc9 100644
>> --- a/fs/erofs/xattr.h
>> +++ b/fs/erofs/xattr.h
>> @@ -45,17 +45,10 @@ extern const struct xattr_handler * const 
>> erofs_xattr_handlers[];
>>   int erofs_xattr_prefixes_init(struct super_block *sb);
>>   void erofs_xattr_prefixes_cleanup(struct super_block *sb);
>> -int erofs_getxattr(struct inode *, int, const char *, void *, size_t);
>>   ssize_t erofs_listxattr(struct dentry *, char *, size_t);
>>   #else
>>   static inline int erofs_xattr_prefixes_init(struct super_block *sb) 
>> { return 0; }
>>   static inline void erofs_xattr_prefixes_cleanup(struct super_block 
>> *sb) {}
>> -static inline int erofs_getxattr(struct inode *inode, int index,
>> -                 const char *name, void *buffer,
>> -                 size_t buffer_size)
>> -{
>> -    return -EOPNOTSUPP;
>> -}
> 
> As I said earlier, I will clean this part in a seperate patch.
> 
> 
>>   #define erofs_listxattr (NULL)
>>   #define erofs_xattr_handlers (NULL)
>> @@ -67,4 +60,9 @@ struct posix_acl *erofs_get_acl(struct inode *inode, 
>> int type, bool rcu);
>>   #define erofs_get_acl    (NULL)
>>   #endif
>> +#ifdef CONFIG_EROFS_FS_PAGE_CACHE_SHARE
> 
> No need to wrap it with a `#ifdef`.
> 
> Thanks,
> Gao Xiang
> 
>> +struct erofs_inode_fingerprint erofs_xattr_get_ishare_fp(struct inode 
>> *inode,
>> +                        const char *domain_id);
>> +#endif
>> +
>>   #endif
> 

Powered by blists - more mailing lists