[PATCH] erofs: use dedicated meta inodes for file-backed mounts

[PATCH] erofs: use dedicated meta inodes for file-backed mounts

[Gao Xiang]

Currently, metadata access for file-backed mounts reuses the page cache
of backing inodes directly.

Switch to per-sb dedicated pseudo inodes ("managed cache") for metadata
access on file-backed mounts (although I still don't think it is
necessary due to the EROFS immutable model). As the result, metadata
cache won't use the page cache of backing inodes anymore.

The "managed cache" was originally used to cache physical compressed
data according to the current cache strategy and I/O patterns; since
file-backed mounts also need to access physical data for metadata
access, it's natural to reuse the managed cache for this too, providing
a unique inode for all physical data access.

Signed-off-by: Gao Xiang <xiang@kernel.org>
---
Anyway, this is a late follow-up to address the previous Christoph's
comments: https://lore.kernel.org/r/af2kDfHe0B3LnvVm@infradead.org

 fs/erofs/data.c     | 28 ++++++++--------------------
 fs/erofs/fileio.c   | 21 +++++++++++++++++++++
 fs/erofs/internal.h | 12 ++++++++----
 fs/erofs/super.c    | 33 ++++++++++++++++++++++++---------
 fs/erofs/zdata.c    | 13 ++++---------
 5 files changed, 65 insertions(+), 42 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 9aa48c8d67d1..e3f3073779ac 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -30,20 +30,6 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 {
 	pgoff_t index = (buf->off + offset) >> PAGE_SHIFT;
 	struct folio *folio = NULL;
-	loff_t fpos;
-	int err;
-
-	/*
-	 * Metadata access for file-backed mounts reuses page cache of backing
-	 * fs inodes (only folio data will be needed) to prevent double caching.
-	 * However, the data access range must be verified here in advance.
-	 */
-	if (buf->file) {
-		fpos = (loff_t)index << PAGE_SHIFT;
-		err = rw_verify_area(READ, buf->file, &fpos, PAGE_SIZE);
-		if (err < 0)
-			return ERR_PTR(err);
-	}
 
 	if (buf->page) {
 		folio = page_folio(buf->page);
@@ -52,7 +38,8 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	}
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);
-		folio = read_mapping_folio(buf->mapping, index, buf->file);
+		folio = read_cache_folio(buf->mapping, index,
+				buf->mc ? erofs_read_meta_folio : NULL, NULL);
 		if (IS_ERR(folio))
 			return folio;
 	}
@@ -69,19 +56,20 @@ int erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb,
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
-	buf->file = NULL;
+	buf->mc = false;
 	if (in_metabox) {
 		if (unlikely(!sbi->metabox_inode))
 			return -EFSCORRUPTED;
 		buf->mapping = sbi->metabox_inode->i_mapping;
 		return 0;
 	}
-	buf->off = sbi->dif0.fsoff;
 	if (erofs_is_fileio_mode(sbi)) {
-		buf->file = sbi->dif0.file;	/* some fs like FUSE needs it */
-		buf->mapping = buf->file->f_mapping;
-	} else
+		buf->mapping = sbi->managed_cache->i_mapping;
+		buf->mc = true;
+	} else {
+		buf->off = sbi->dif0.fsoff;
 		buf->mapping = sb->s_bdev->bd_mapping;
+	}
 	return 0;
 }
 
diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index 98cdaa1cd1a7..ebb81a7ffd4c 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -194,3 +194,24 @@ const struct address_space_operations erofs_fileio_aops = {
 	.read_folio = erofs_fileio_read_folio,
 	.readahead = erofs_fileio_readahead,
 };
+
+int erofs_read_meta_folio(struct file *file, struct folio *folio)
+{
+	struct erofs_fileio io = {
+		.dev = { .m_pa = folio_pos(folio), },
+	};
+	struct inode *inode = folio_inode(folio);
+	int err;
+
+	err = erofs_map_dev(inode->i_sb, &io.dev);
+	if (err)
+		return err;
+
+	io.rq = erofs_fileio_rq_alloc(&io.dev);
+	io.rq->bio.bi_iter.bi_sector =
+		(io.dev.m_dif->fsoff + io.dev.m_pa) >> 9;
+	erofs_onlinefolio_init(folio);
+	bio_add_folio_nofail(&io.rq->bio, folio, folio_size(folio), 0);
+	erofs_fileio_rq_submit(io.rq);
+	return 0;
+}
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 57bd21859c65..6de6e5a58e6b 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -98,11 +98,9 @@ struct erofs_sb_info {
 	unsigned int sync_decompress;	/* strategy for sync decompression */
 	unsigned int shrinker_run_no;
 
-	/* pseudo inode to manage cached pages */
-	struct inode *managed_cache;
-
 	struct erofs_sb_lz4_info lz4;
 #endif	/* CONFIG_EROFS_FS_ZIP */
+	struct inode *managed_cache; /* pseudo inode to cache physical data */
 	struct inode *packed_inode;
 	struct inode *metabox_inode;
 	struct erofs_dev_context *devs;
@@ -176,10 +174,10 @@ enum {
 
 struct erofs_buf {
 	struct address_space *mapping;
-	struct file *file;
 	u64 off;
 	struct page *page;
 	void *base;
+	bool mc;
 };
 #define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
 
@@ -399,6 +397,12 @@ extern const struct file_operations erofs_ishare_fops;
 
 extern const struct iomap_ops z_erofs_iomap_report_ops;
 
+int erofs_setup_managed_cache(struct super_block *sb);
+#ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
+int erofs_read_meta_folio(struct file *file, struct folio *folio);
+#else
+#define erofs_read_meta_folio NULL
+#endif
 void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
 			  erofs_off_t *offset, int *lengthp);
 void erofs_unmap_metabuf(struct erofs_buf *buf);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3d92caec8d3a..ff10b346ef4e 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -633,6 +633,21 @@ static const struct export_operations erofs_export_ops = {
 	.get_parent = erofs_get_parent,
 };
 
+int erofs_setup_managed_cache(struct super_block *sb)
+{
+	if (!EROFS_SB(sb)->managed_cache) {
+		struct inode *inode = new_inode(sb);
+
+		if (!inode)
+			return -ENOMEM;
+		set_nlink(inode, 1);
+		inode->i_size = OFFSET_MAX;
+		mapping_set_gfp_mask(inode->i_mapping, GFP_KERNEL);
+		EROFS_SB(sb)->managed_cache = inode;
+	}
+	return 0;
+}
+
 static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct inode *inode;
@@ -654,7 +669,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	}
 
 	sbi->blkszbits = PAGE_SHIFT;
-	if (!sb->s_bdev) {
+	if (erofs_is_fileio_mode(sbi)) {
 		/*
 		 * (File-backed mounts) EROFS claims it's safe to nest other
 		 * fs contexts (including its own) due to self-controlled RO
@@ -669,19 +684,19 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 		 * It MUST change if another fs plans to support them, which
 		 * may also require adjusting FILESYSTEM_MAX_STACK_DEPTH.
 		 */
-		if (erofs_is_fileio_mode(sbi)) {
-			inode = file_inode(sbi->dif0.file);
-			if ((inode->i_sb->s_op == &erofs_sops &&
-			     !inode->i_sb->s_bdev) ||
-			    inode->i_sb->s_stack_depth) {
-				erofs_err(sb, "file-backed mounts cannot be applied to stacked fses");
-				return -ENOTBLK;
-			}
+		inode = file_inode(sbi->dif0.file);
+		if ((inode->i_sb->s_op == &erofs_sops &&
+		     !inode->i_sb->s_bdev) || inode->i_sb->s_stack_depth) {
+			erofs_err(sb, "file-backed mounts cannot be applied to stacked fses");
+			return -ENOTBLK;
 		}
 		sb->s_blocksize = PAGE_SIZE;
 		sb->s_blocksize_bits = PAGE_SHIFT;
 
 		err = super_setup_bdi(sb);
+		if (err)
+			return err;
+		err = erofs_setup_managed_cache(sb);
 		if (err)
 			return err;
 
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 74520e910259..602ba8b7cc79 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -666,21 +666,16 @@ static const struct address_space_operations z_erofs_cache_aops = {
 
 int z_erofs_init_super(struct super_block *sb)
 {
-	struct inode *inode;
 	int err;
 
 	err = z_erofs_init_pcpu_workers(sb);
 	if (err)
 		return err;
 
-	inode = new_inode(sb);
-	if (!inode)
-		return -ENOMEM;
-	set_nlink(inode, 1);
-	inode->i_size = OFFSET_MAX;
-	inode->i_mapping->a_ops = &z_erofs_cache_aops;
-	mapping_set_gfp_mask(inode->i_mapping, GFP_KERNEL);
-	EROFS_SB(sb)->managed_cache = inode;
+	err = erofs_setup_managed_cache(sb);
+	if (err)
+		return err;
+	EROFS_SB(sb)->managed_cache->i_mapping->a_ops = &z_erofs_cache_aops;
 	xa_init(&EROFS_SB(sb)->managed_pslots);
 	return 0;
 }
-- 
2.47.3

[PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Gao Xiang]

Currently, metadata access for file-backed mounts reuses the page cache
of backing inodes directly.

Switch to per-sb dedicated pseudo inodes ("managed cache") for metadata
access on file-backed mounts (although I still don't think it is
necessary due to the EROFS immutable model). As the result, metadata
cache won't use the page cache of backing inodes anymore.

The "managed cache" was originally used to cache physical compressed
data according to the current cache strategy and I/O patterns; since
file-backed mounts also need to access physical data for metadata
access, it's natural to reuse the managed cache for this too, providing
a unique inode for all physical data access.

Signed-off-by: Gao Xiang <xiang@kernel.org>
---
v2:
 - move iput(managed_cache) out of #ifdef CONFIG_EROFS_FS_ZIP, reported
   by sashiko:
    https://sashiko.dev/#/patchset/20260811080852.29418-1-xiang%40kernel.org

 fs/erofs/data.c     | 28 ++++++++--------------------
 fs/erofs/fileio.c   | 21 +++++++++++++++++++++
 fs/erofs/internal.h | 12 ++++++++----
 fs/erofs/super.c    | 35 ++++++++++++++++++++++++-----------
 fs/erofs/zdata.c    | 13 ++++---------
 5 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 9aa48c8d67d1..e3f3073779ac 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -30,20 +30,6 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 {
 	pgoff_t index = (buf->off + offset) >> PAGE_SHIFT;
 	struct folio *folio = NULL;
-	loff_t fpos;
-	int err;
-
-	/*
-	 * Metadata access for file-backed mounts reuses page cache of backing
-	 * fs inodes (only folio data will be needed) to prevent double caching.
-	 * However, the data access range must be verified here in advance.
-	 */
-	if (buf->file) {
-		fpos = (loff_t)index << PAGE_SHIFT;
-		err = rw_verify_area(READ, buf->file, &fpos, PAGE_SIZE);
-		if (err < 0)
-			return ERR_PTR(err);
-	}
 
 	if (buf->page) {
 		folio = page_folio(buf->page);
@@ -52,7 +38,8 @@ void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 	}
 	if (!folio || !folio_contains(folio, index)) {
 		erofs_put_metabuf(buf);
-		folio = read_mapping_folio(buf->mapping, index, buf->file);
+		folio = read_cache_folio(buf->mapping, index,
+				buf->mc ? erofs_read_meta_folio : NULL, NULL);
 		if (IS_ERR(folio))
 			return folio;
 	}
@@ -69,19 +56,20 @@ int erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb,
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
-	buf->file = NULL;
+	buf->mc = false;
 	if (in_metabox) {
 		if (unlikely(!sbi->metabox_inode))
 			return -EFSCORRUPTED;
 		buf->mapping = sbi->metabox_inode->i_mapping;
 		return 0;
 	}
-	buf->off = sbi->dif0.fsoff;
 	if (erofs_is_fileio_mode(sbi)) {
-		buf->file = sbi->dif0.file;	/* some fs like FUSE needs it */
-		buf->mapping = buf->file->f_mapping;
-	} else
+		buf->mapping = sbi->managed_cache->i_mapping;
+		buf->mc = true;
+	} else {
+		buf->off = sbi->dif0.fsoff;
 		buf->mapping = sb->s_bdev->bd_mapping;
+	}
 	return 0;
 }
 
diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index 98cdaa1cd1a7..ebb81a7ffd4c 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -194,3 +194,24 @@ const struct address_space_operations erofs_fileio_aops = {
 	.read_folio = erofs_fileio_read_folio,
 	.readahead = erofs_fileio_readahead,
 };
+
+int erofs_read_meta_folio(struct file *file, struct folio *folio)
+{
+	struct erofs_fileio io = {
+		.dev = { .m_pa = folio_pos(folio), },
+	};
+	struct inode *inode = folio_inode(folio);
+	int err;
+
+	err = erofs_map_dev(inode->i_sb, &io.dev);
+	if (err)
+		return err;
+
+	io.rq = erofs_fileio_rq_alloc(&io.dev);
+	io.rq->bio.bi_iter.bi_sector =
+		(io.dev.m_dif->fsoff + io.dev.m_pa) >> 9;
+	erofs_onlinefolio_init(folio);
+	bio_add_folio_nofail(&io.rq->bio, folio, folio_size(folio), 0);
+	erofs_fileio_rq_submit(io.rq);
+	return 0;
+}
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 57bd21859c65..6de6e5a58e6b 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -98,11 +98,9 @@ struct erofs_sb_info {
 	unsigned int sync_decompress;	/* strategy for sync decompression */
 	unsigned int shrinker_run_no;
 
-	/* pseudo inode to manage cached pages */
-	struct inode *managed_cache;
-
 	struct erofs_sb_lz4_info lz4;
 #endif	/* CONFIG_EROFS_FS_ZIP */
+	struct inode *managed_cache; /* pseudo inode to cache physical data */
 	struct inode *packed_inode;
 	struct inode *metabox_inode;
 	struct erofs_dev_context *devs;
@@ -176,10 +174,10 @@ enum {
 
 struct erofs_buf {
 	struct address_space *mapping;
-	struct file *file;
 	u64 off;
 	struct page *page;
 	void *base;
+	bool mc;
 };
 #define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
 
@@ -399,6 +397,12 @@ extern const struct file_operations erofs_ishare_fops;
 
 extern const struct iomap_ops z_erofs_iomap_report_ops;
 
+int erofs_setup_managed_cache(struct super_block *sb);
+#ifdef CONFIG_EROFS_FS_BACKED_BY_FILE
+int erofs_read_meta_folio(struct file *file, struct folio *folio);
+#else
+#define erofs_read_meta_folio NULL
+#endif
 void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
 			  erofs_off_t *offset, int *lengthp);
 void erofs_unmap_metabuf(struct erofs_buf *buf);
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3d92caec8d3a..33e308e349f6 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -633,6 +633,21 @@ static const struct export_operations erofs_export_ops = {
 	.get_parent = erofs_get_parent,
 };
 
+int erofs_setup_managed_cache(struct super_block *sb)
+{
+	if (!EROFS_SB(sb)->managed_cache) {
+		struct inode *inode = new_inode(sb);
+
+		if (!inode)
+			return -ENOMEM;
+		set_nlink(inode, 1);
+		inode->i_size = OFFSET_MAX;
+		mapping_set_gfp_mask(inode->i_mapping, GFP_KERNEL);
+		EROFS_SB(sb)->managed_cache = inode;
+	}
+	return 0;
+}
+
 static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct inode *inode;
@@ -654,7 +669,7 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 	}
 
 	sbi->blkszbits = PAGE_SHIFT;
-	if (!sb->s_bdev) {
+	if (erofs_is_fileio_mode(sbi)) {
 		/*
 		 * (File-backed mounts) EROFS claims it's safe to nest other
 		 * fs contexts (including its own) due to self-controlled RO
@@ -669,19 +684,19 @@ static int erofs_fc_fill_super(struct super_block *sb, struct fs_context *fc)
 		 * It MUST change if another fs plans to support them, which
 		 * may also require adjusting FILESYSTEM_MAX_STACK_DEPTH.
 		 */
-		if (erofs_is_fileio_mode(sbi)) {
-			inode = file_inode(sbi->dif0.file);
-			if ((inode->i_sb->s_op == &erofs_sops &&
-			     !inode->i_sb->s_bdev) ||
-			    inode->i_sb->s_stack_depth) {
-				erofs_err(sb, "file-backed mounts cannot be applied to stacked fses");
-				return -ENOTBLK;
-			}
+		inode = file_inode(sbi->dif0.file);
+		if ((inode->i_sb->s_op == &erofs_sops &&
+		     !inode->i_sb->s_bdev) || inode->i_sb->s_stack_depth) {
+			erofs_err(sb, "file-backed mounts cannot be applied to stacked fses");
+			return -ENOTBLK;
 		}
 		sb->s_blocksize = PAGE_SIZE;
 		sb->s_blocksize_bits = PAGE_SHIFT;
 
 		err = super_setup_bdi(sb);
+		if (err)
+			return err;
+		err = erofs_setup_managed_cache(sb);
 		if (err)
 			return err;
 
@@ -913,10 +928,8 @@ static void erofs_drop_internal_inodes(struct erofs_sb_info *sbi)
 	sbi->packed_inode = NULL;
 	iput(sbi->metabox_inode);
 	sbi->metabox_inode = NULL;
-#ifdef CONFIG_EROFS_FS_ZIP
 	iput(sbi->managed_cache);
 	sbi->managed_cache = NULL;
-#endif
 }
 
 static void erofs_kill_sb(struct super_block *sb)
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 74520e910259..602ba8b7cc79 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -666,21 +666,16 @@ static const struct address_space_operations z_erofs_cache_aops = {
 
 int z_erofs_init_super(struct super_block *sb)
 {
-	struct inode *inode;
 	int err;
 
 	err = z_erofs_init_pcpu_workers(sb);
 	if (err)
 		return err;
 
-	inode = new_inode(sb);
-	if (!inode)
-		return -ENOMEM;
-	set_nlink(inode, 1);
-	inode->i_size = OFFSET_MAX;
-	inode->i_mapping->a_ops = &z_erofs_cache_aops;
-	mapping_set_gfp_mask(inode->i_mapping, GFP_KERNEL);
-	EROFS_SB(sb)->managed_cache = inode;
+	err = erofs_setup_managed_cache(sb);
+	if (err)
+		return err;
+	EROFS_SB(sb)->managed_cache->i_mapping->a_ops = &z_erofs_cache_aops;
 	xa_init(&EROFS_SB(sb)->managed_pslots);
 	return 0;
 }
-- 
2.47.3

Re: [PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Chao Yu]

On 8/11/26 17:07, Gao Xiang wrote:
> Currently, metadata access for file-backed mounts reuses the page cache
> of backing inodes directly.
> 
> Switch to per-sb dedicated pseudo inodes ("managed cache") for metadata
> access on file-backed mounts (although I still don't think it is
> necessary due to the EROFS immutable model). As the result, metadata
> cache won't use the page cache of backing inodes anymore.
> 
> The "managed cache" was originally used to cache physical compressed
> data according to the current cache strategy and I/O patterns; since
> file-backed mounts also need to access physical data for metadata
> access, it's natural to reuse the managed cache for this too, providing
> a unique inode for all physical data access.
> 
> Signed-off-by: Gao Xiang <xiang@kernel.org>
Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Christoph Hellwig]

On Tue, Aug 11, 2026 at 05:07:17PM +0800, Gao Xiang wrote:
> access on file-backed mounts (although I still don't think it is
> necessary due to the EROFS immutable model). As the result, metadata
> cache won't use the page cache of backing inodes anymore.
> 
> The "managed cache" was originally used to cache physical compressed
> data according to the current cache strategy and I/O patterns; since
> file-backed mounts also need to access physical data for metadata
> access, it's natural to reuse the managed cache for this too, providing
> a unique inode for all physical data access.

This look sane.  Any reason to not use the meta inode unconditionally
to simplify the code?

Re: [PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Gao Xiang]

Hi Christoph,

On Tue, Aug 18, 2026 at 10:55:47PM -0700, Christoph Hellwig wrote:
> On Tue, Aug 11, 2026 at 05:07:17PM +0800, Gao Xiang wrote:
> > access on file-backed mounts (although I still don't think it is
> > necessary due to the EROFS immutable model). As the result, metadata
> > cache won't use the page cache of backing inodes anymore.
> > 
> > The "managed cache" was originally used to cache physical compressed
> > data according to the current cache strategy and I/O patterns; since
> > file-backed mounts also need to access physical data for metadata
> > access, it's natural to reuse the managed cache for this too, providing
> > a unique inode for all physical data access.
> 
> This look sane.  Any reason to not use the meta inode unconditionally
> to simplify the code?

I guess you meant avoid using bd_inode page cache?

I'm not sure if it simplifies a lot, also like fadvise(WILLNEED) to blkdev
can perform to bd_inode too so I think it can still have some benefits.

I think bd_inode page cache can be used for filesystems so maybe leave it
as-is for now, or maybe a follow-up work if it's really needed.

Thanks,
Gao Xiang

> 
> 

Re: [PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Christoph Hellwig]

On Thu, Aug 20, 2026 at 09:40:46PM +0800, Gao Xiang wrote:
> Hi Christoph,
> 
> On Tue, Aug 18, 2026 at 10:55:47PM -0700, Christoph Hellwig wrote:
> > On Tue, Aug 11, 2026 at 05:07:17PM +0800, Gao Xiang wrote:
> > > access on file-backed mounts (although I still don't think it is
> > > necessary due to the EROFS immutable model). As the result, metadata
> > > cache won't use the page cache of backing inodes anymore.
> > > 
> > > The "managed cache" was originally used to cache physical compressed
> > > data according to the current cache strategy and I/O patterns; since
> > > file-backed mounts also need to access physical data for metadata
> > > access, it's natural to reuse the managed cache for this too, providing
> > > a unique inode for all physical data access.
> > 
> > This look sane.  Any reason to not use the meta inode unconditionally
> > to simplify the code?
> 
> I guess you meant avoid using bd_inode page cache?


Yes.

> I'm not sure if it simplifies a lot, also like fadvise(WILLNEED) to blkdev
> can perform to bd_inode too so I think it can still have some benefits.

This is actually an artefact of why using the bdev page cache is a bad
idea, because it opens your metadata cache up to external modification
and side channel attacks.

> I think bd_inode page cache can be used for filesystems so maybe leave it
> as-is for now, or maybe a follow-up work if it's really needed.

That's how it has historically been done.  We generally try to avoid
it for newer designs.

Re: [PATCH v2] erofs: use dedicated meta inodes for file-backed mounts

[Gao Xiang]

Hi Christoph,

On Tue, Aug 25, 2026 at 09:44:03PM -0700, Christoph Hellwig wrote:
> On Thu, Aug 20, 2026 at 09:40:46PM +0800, Gao Xiang wrote:
> > Hi Christoph,
> > 
> > On Tue, Aug 18, 2026 at 10:55:47PM -0700, Christoph Hellwig wrote:
> > > On Tue, Aug 11, 2026 at 05:07:17PM +0800, Gao Xiang wrote:
> > > > access on file-backed mounts (although I still don't think it is
> > > > necessary due to the EROFS immutable model). As the result, metadata
> > > > cache won't use the page cache of backing inodes anymore.
> > > > 
> > > > The "managed cache" was originally used to cache physical compressed
> > > > data according to the current cache strategy and I/O patterns; since
> > > > file-backed mounts also need to access physical data for metadata
> > > > access, it's natural to reuse the managed cache for this too, providing
> > > > a unique inode for all physical data access.
> > > 
> > > This look sane.  Any reason to not use the meta inode unconditionally
> > > to simplify the code?
> > 
> > I guess you meant avoid using bd_inode page cache?
> 
> 
> Yes.
> 
> > I'm not sure if it simplifies a lot, also like fadvise(WILLNEED) to blkdev
> > can perform to bd_inode too so I think it can still have some benefits.
> 
> This is actually an artefact of why using the bdev page cache is a bad
> idea, because it opens your metadata cache up to external modification
> and side channel attacks.

See my reply to Christian before:
https://lore.kernel.org/r/95a5fc3a-4259-44a7-bb72-8ff35a49a26f@linux.alibaba.com

EROFS is unlike generic writable filesystems which need to consider
writing to the disk and keep various cache (extent cache, ...) and
complex metadata relationship.

EROFS itself is only designed for archive use cases (like tar/zip),
and it never changes its data and metadata itself at runtime, and it
intentionally only uses the physical view as the unique metadata
cache (never duplicate a different form of in-memory metadata
cache) by design.  For unexpected external modification, it generally
behaves undefined (nevertheless, the physical device can be modified
in various ways).  In short, our implementation is always designed
to harden and tolerate this so it won't cause severe in-memory <->
on-disk or on-disk <-> on-disk inconsistency.

You could doubt that, but here I claim again that one of EROFS main
ondisk/runtime design goals (which is very unlike to other generic
filesystems, and that is one of EROFS's main value), for human-made
random implementation bugs, we will fix them all after reported since
the implementation bug cannot be totally avoidable from an engineering
perspective (unless the implementation is totally auto code generated.)

For side channel attack, the Linux page cache model is always vulnerable
to that, and I really think it's a trade-off (for shared kernel
apporaches, cache side channel attack is unavoidable and no need to
avoid; if you need a strongest protection, you should use isolated kernels
or seperate VM instead.)

We could switch to use per-fs metadata cache, but in that way, we have to
develop a new way to warm up the cache instead, I guess in that way it
will be much like switched into an EROFS-specific ioctl interface instead
since fadvise no longer works.

> 
> > I think bd_inode page cache can be used for filesystems so maybe leave it
> > as-is for now, or maybe a follow-up work if it's really needed.
> 
> That's how it has historically been done.  We generally try to avoid
> it for newer designs.

TR;DR: I will switch later when I find another slot, but it should just
based on a fact is that bd_inode page cache is deprecated in the long
term simply.

Thanks,
Gao Xiang

> 
>