lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <de446b03-b311-4630-aa7c-b503bcd82b69@huawei.com>
Date: Tue, 18 Nov 2025 10:16:29 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Gao Xiang <hsiangkao@...ux.alibaba.com>, <chao@...nel.org>,
	<brauner@...nel.org>, <djwong@...nel.org>, <amir73il@...il.com>,
	<joannelkoong@...il.com>
CC: <linux-fsdevel@...r.kernel.org>, <linux-erofs@...ts.ozlabs.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 05/10] erofs: support user-defined fingerprint name

Hi Xiang,

On 2025/11/18 1:26, Gao Xiang wrote:
> 
> 
> On 2025/11/17 21:25, Hongbo Li wrote:
>> From: Hongzhen Luo <hongzhen@...ux.alibaba.com>
>>
>> When creating the EROFS image, users can specify the fingerprint name.
>> This is to prepare for the upcoming inode page cache share.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen@...ux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
>> ---
>>   fs/erofs/Kconfig    |  9 +++++++++
>>   fs/erofs/erofs_fs.h |  6 ++++--
>>   fs/erofs/internal.h |  2 ++
>>   fs/erofs/super.c    |  5 +++--
>>   fs/erofs/xattr.c    | 15 +++++++++++++++
>>   5 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
>> index d81f3318417d..c88b6d0714a4 100644
>> --- a/fs/erofs/Kconfig
>> +++ b/fs/erofs/Kconfig
>> @@ -194,3 +194,12 @@ config EROFS_FS_PCPU_KTHREAD_HIPRI
>>         at higher priority.
>>         If unsure, say N.
>> +
>> +config EROFS_FS_PAGE_CACHE_SHARE
>> +    bool "EROFS page cache share support (experimental)"
>> +    depends on EROFS_FS && EROFS_FS_XATTR && !EROFS_FS_ONDEMAND
>> +    help
>> +      This enables page cache sharing among inodes with identical
>> +      content fingerprints on the same device.
>> +
>> +      If unsure, say N.
>> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
>> index 3d5738f80072..9b9fe1abe0b9 100644
>> --- a/fs/erofs/erofs_fs.h
>> +++ b/fs/erofs/erofs_fs.h
>> @@ -35,8 +35,9 @@
>>   #define EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES    0x00000040
>>   #define EROFS_FEATURE_INCOMPAT_48BIT        0x00000080
>>   #define EROFS_FEATURE_INCOMPAT_METABOX        0x00000100
>> +#define EROFS_FEATURE_INCOMPAT_ISHARE_KEY    0x00000200
> 
> It seems that you didn't address this part I
> mentioned in the previous reply:
> https://lore.kernel.org/r/a3b0bac9-d08f-44dc-8adb-7cc85cae7b13@linux.alibaba.com
> 

Thanks for pointing it out. I will update here in next version (include 
the following issues).

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 9b9fe1abe0b9..28955456d2e1 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -17,7 +17,7 @@
  #define EROFS_FEATURE_COMPAT_XATTR_FILTER              0x00000004
  #define EROFS_FEATURE_COMPAT_SHARED_EA_IN_METABOX      0x00000008
  #define EROFS_FEATURE_COMPAT_PLAIN_XATTR_PFX           0x00000010
-
+#define EROFS_FEATURE_COMPAT_ISHARE_KEY                        0x00000020

  /*
   * Any bits that aren't in EROFS_ALL_FEATURE_INCOMPAT should
@@ -35,9 +35,8 @@
  #define EROFS_FEATURE_INCOMPAT_XATTR_PREFIXES  0x00000040
  #define EROFS_FEATURE_INCOMPAT_48BIT           0x00000080
  #define EROFS_FEATURE_INCOMPAT_METABOX         0x00000100
-#define EROFS_FEATURE_INCOMPAT_ISHARE_KEY      0x00000200
  #define EROFS_ALL_FEATURE_INCOMPAT             \
-       ((EROFS_FEATURE_INCOMPAT_ISHARE_KEY << 1) - 1)
+       ((EROFS_FEATURE_INCOMPAT_METABOX << 1) - 1)

  #define EROFS_SB_EXTSLOT_SIZE  16

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 37b536eebc3d..bb5dff36c9b8 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -248,11 +248,11 @@ EROFS_FEATURE_FUNCS(dedupe, incompat, INCOMPAT_DEDUPE)
  EROFS_FEATURE_FUNCS(xattr_prefixes, incompat, INCOMPAT_XATTR_PREFIXES)
  EROFS_FEATURE_FUNCS(48bit, incompat, INCOMPAT_48BIT)
  EROFS_FEATURE_FUNCS(metabox, incompat, INCOMPAT_METABOX)
-EROFS_FEATURE_FUNCS(ishare_key, incompat, INCOMPAT_ISHARE_KEY)
  EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
  EROFS_FEATURE_FUNCS(xattr_filter, compat, COMPAT_XATTR_FILTER)
  EROFS_FEATURE_FUNCS(shared_ea_in_metabox, compat, 
COMPAT_SHARED_EA_IN_METABOX)
  EROFS_FEATURE_FUNCS(plain_xattr_pfx, compat, COMPAT_PLAIN_XATTR_PFX)
+EROFS_FEATURE_FUNCS(ishare_key, compat, COMPAT_ISHARE_KEY)

  static inline u64 erofs_nid_to_ino64(struct erofs_sb_info *sbi, 
erofs_nid_t nid)
  {

Thanks,
Hongbo

>>   #define EROFS_ALL_FEATURE_INCOMPAT        \
>> -    ((EROFS_FEATURE_INCOMPAT_METABOX << 1) - 1)
>> +    ((EROFS_FEATURE_INCOMPAT_ISHARE_KEY << 1) - 1)
>>   #define EROFS_SB_EXTSLOT_SIZE    16
>> @@ -83,7 +84,8 @@ struct erofs_super_block {
>>       __le32 xattr_prefix_start;    /* start of long xattr prefixes */
>>       __le64 packed_nid;    /* nid of the special packed inode */
>>       __u8 xattr_filter_reserved; /* reserved for xattr name filter */
>> -    __u8 reserved[3];
>> +    __u8 ishare_xattr_prefix_id;    /* indice the ishare key in 
>> prefix xattr */
> 
> /* indexes the ishare key in prefix xattres */ ?
> 
>> +    __u8 reserved[2];
>>       __le32 build_time;    /* seconds added to epoch for mkfs time */
>>       __le64 rootnid_8b;    /* (48BIT on) nid of root directory */
>>       __le64 reserved2;
>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>> index 98fe652aea33..3033252211ba 100644
>> --- a/fs/erofs/internal.h
>> +++ b/fs/erofs/internal.h
>> @@ -134,6 +134,7 @@ struct erofs_sb_info {
>>       u32 xattr_blkaddr;
>>       u32 xattr_prefix_start;
>>       u8 xattr_prefix_count;
>> +    u8 ishare_xattr_pfx;    /* ishare prefix xattr index */
> 
> I think either giving a meaningful comment or just
> get rid of the comment entirely.
> 
>>       struct erofs_xattr_prefix_item *xattr_prefixes;
>>       unsigned int xattr_filter_reserved;
>>   #endif
>> @@ -234,6 +235,7 @@ EROFS_FEATURE_FUNCS(dedupe, incompat, 
>> INCOMPAT_DEDUPE)
>>   EROFS_FEATURE_FUNCS(xattr_prefixes, incompat, INCOMPAT_XATTR_PREFIXES)
>>   EROFS_FEATURE_FUNCS(48bit, incompat, INCOMPAT_48BIT)
>>   EROFS_FEATURE_FUNCS(metabox, incompat, INCOMPAT_METABOX)
>> +EROFS_FEATURE_FUNCS(ishare_key, incompat, INCOMPAT_ISHARE_KEY)
>>   EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
>>   EROFS_FEATURE_FUNCS(xattr_filter, compat, COMPAT_XATTR_FILTER)
>>   EROFS_FEATURE_FUNCS(shared_ea_in_metabox, compat, 
>> COMPAT_SHARED_EA_IN_METABOX)
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index 0d88c04684b9..80f032cb2cc3 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -298,6 +298,9 @@ static int erofs_read_superblock(struct 
>> super_block *sb)
>>           if (ret)
>>               goto out;
>>       }
>> +    if (erofs_sb_has_ishare_key(sbi))
>> +        sbi->ishare_xattr_pfx =
>> +            dsb->ishare_xattr_prefix_id & EROFS_XATTR_LONG_PREFIX_MASK;
>>       ret = -EINVAL;
>>       sbi->feature_incompat = le32_to_cpu(dsb->feature_incompat);
>> @@ -339,7 +342,6 @@ static int erofs_read_superblock(struct 
>> super_block *sb)
>>               return -EFSCORRUPTED;    /* self-loop detection */
>>       }
>>       sbi->inos = le64_to_cpu(dsb->inos);
>> -
> 
> Unnecessary change here.
> 
>>       sbi->epoch = (s64)le64_to_cpu(dsb->epoch);
>>       sbi->fixed_nsec = le32_to_cpu(dsb->fixed_nsec);
>>       super_set_uuid(sb, (void *)dsb->uuid, sizeof(dsb->uuid));
>> @@ -737,7 +739,6 @@ static int erofs_fc_fill_super(struct super_block 
>> *sb, struct fs_context *fc)
>>       err = erofs_xattr_prefixes_init(sb);
>>       if (err)
>>           return err;
>> -
> 
> Unnecessary change here.
> 
>>       erofs_set_sysfs_name(sb);
>>       err = erofs_register_sysfs(sb);
>>       if (err)
>> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
>> index 396536d9a862..6cb76313c14c 100644
>> --- a/fs/erofs/xattr.c
>> +++ b/fs/erofs/xattr.c
>> @@ -519,6 +519,21 @@ int erofs_xattr_prefixes_init(struct super_block 
>> *sb)
>>       }
>>       erofs_put_metabuf(&buf);
>> +    if (!ret && erofs_sb_has_ishare_key(sbi)) {
>> +        struct erofs_xattr_long_prefix *new_pfx, *pfx =
>> +                    pfs[sbi->ishare_xattr_pfx].prefix;
> 
>          struct erofs_xattr_prefix_item *pf = pfs + sbi->ishare_xattr_pfx;
>          struct erofs_xattr_long_prefix *newpfx;
> 
>          newpfx = krealloc(pf->prefix,
>                  sizeof(*newpfx) + pf->infix_len + 1, GFP_KERNEL);
>          if (newpfx) {
>              newpfx->infix[pf->infix_len] = '\0';
>              pf->prefix = newpfx;
>          } else {
>              ret = -ENOMEM;
>          }
> 
> Thanks,
> Gao Xiang
> 
>> +
>> +        new_pfx = krealloc(pfx,
>> +                   sizeof(struct erofs_xattr_long_prefix) +
>> +                   pfs[sbi->ishare_xattr_pfx].infix_len + 1,
>> +                   GFP_KERNEL);
>> +        if (new_pfx) {
>> +            new_pfx->infix[pfs[sbi->ishare_xattr_pfx].infix_len] = '\0';
>> +            pfs[sbi->ishare_xattr_pfx].prefix = new_pfx;
>> +        } else {
>> +            ret = -ENOMEM;
>> +        }
>> +    }
>>       sbi->xattr_prefixes = pfs;
>>       if (ret)
>>           erofs_xattr_prefixes_cleanup(sb);
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ