| 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
| ||
|
Message-ID: <07212138-c0fc-4a64-a323-9cab978bf610@huawei.com>
Date: Wed, 7 Jan 2026 15:17:41 +0800
From: Hongbo Li <lihongbo22@...wei.com>
To: Gao Xiang <hsiangkao@...ux.alibaba.com>
CC: <djwong@...nel.org>, <amir73il@...il.com>, <hch@....de>,
<linux-fsdevel@...r.kernel.org>, <linux-erofs@...ts.ozlabs.org>,
<linux-kernel@...r.kernel.org>, Chao Yu <chao@...nel.org>,
<brauner@...nel.org>
Subject: Re: [PATCH v12 07/10] erofs: introduce the page cache share feature
Hi, Xiang
On 2026/1/7 14:56, Gao Xiang wrote:
>
>
> On 2026/1/7 14:48, Hongbo Li wrote:
>> Hi, Xiang
>>
>> On 2026/1/7 14:08, Gao Xiang wrote:
>>>
>>>
>>> On 2025/12/31 17:01, 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 | 5 +
>>>> fs/erofs/Makefile | 1 +
>>>> fs/erofs/internal.h | 31 +++++
>>>> fs/erofs/ishare.c | 170
>>>> ++++++++++++++++++++++++++++
>>>> fs/erofs/super.c | 55 ++++++++-
>>>> fs/erofs/xattr.c | 34 ++++++
>>>> fs/erofs/xattr.h | 3 +
>>>> 7 files changed, 297 insertions(+), 2 deletions(-)
>>>> create mode 100644 fs/erofs/ishare.c
>>>>
>>>> diff --git a/Documentation/filesystems/erofs.rst
>>>> b/Documentation/filesystems/erofs.rst
>>>> index 08194f194b94..27d3caa3c73c 100644
>>>> --- a/Documentation/filesystems/erofs.rst
>>>> +++ b/Documentation/filesystems/erofs.rst
>>>> @@ -128,7 +128,12 @@ 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 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 ec79e8b44d3b..6ef1cdd9d651 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,11 @@ 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
>>>> +struct erofs_inode_fingerprint {
>>>> + u8 *opaque;
>>>> + int size;
>>>> +};
>>>> +
>>>> struct erofs_inode {
>>>> erofs_nid_t nid;
>>>> @@ -304,6 +310,18 @@ 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 *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..e93d379d4a3a
>>>> --- /dev/null
>>>> +++ b/fs/erofs/ishare.c
>>>> @@ -0,0 +1,170 @@
>>>> +// 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 *sharedinode;
>>>> + unsigned long hash;
>>>> +
>>>> + if (!test_opt(&sbi->opt, INODE_SHARE))
>>>> + return false;
>>>> + (void)erofs_xattr_fill_ishare_fp(&fp, inode, sbi->domain_id);
>>>> + if (!fp.size)
>>>> + return false;
>>>
>>> Why not just:
>>>
>>> if (erofs_xattr_fill_ishare_fp(&fp, inode, sbi->domain_id))
>>> return false;
>>>
>>
>> When erofs_sb_has_ishare_xattrs returns false,
>> erofs_xattr_fill_ishare_fp also considers success.
>
> Then why !test_opt(&sbi->opt, INODE_SHARE) didn't return?
>
The MOUNT_INODE_SHARE flag is passed from user's mount option. And it is
controllered by CONFIG_EROFS_FS_PAGE_CACHE_SHARE. I doesn't do the check
when the superblock without ishare_xattrs. (It seems the mount options
is static, although it is useless for mounting with inode_share on one
EROFS image without ishare_xattrs).
So should we check that if the superblock has not ishare_xattrs feature,
and we return -ENOSUPP?
Thanks,
Hongbo
> Thanks,
> Gao Xiang
Powered by blists - more mailing lists