[PATCH v2 1/2] erofs: set error to bio if file-backed IO fails

[PATCH v2 1/2] erofs: set error to bio if file-backed IO fails

[Sheng Yong]

From: Sheng Yong <shengyong1@xiaomi.com>

If a file-backed IO fails before submitting the bio to the lower
filesystem, an error is returned, but the bio->bi_status is not
marked as an error. However, the error information should be passed
to the end_io handler. Otherwise, the IO request will be treated as
successful.

Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
---
 fs/erofs/fileio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index bec4b56b3826..4fa0a0121288 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -32,6 +32,8 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
 		ret = 0;
 	}
 	if (rq->bio.bi_end_io) {
+		if (ret < 0 && !rq->bio.bi_status)
+			rq->bio.bi_status = errno_to_blk_status(ret);
 		rq->bio.bi_end_io(&rq->bio);
 	} else {
 		bio_for_each_folio_all(fi, &rq->bio) {
-- 
2.43.0

[PATCH v2 2/2] erofs: add 'offset' mount option for file-backed & bdev-based mounts

[Sheng Yong]

From: Sheng Yong <shengyong1@xiaomi.com>

When attempting to use an archive file, such as APEX on android,
as a file-backed mount source, it fails because EROFS image within
the archive file does not start at offset 0. As a result, a loop
device is still needed to attach the image file at an appropriate
offset first. Similarly, if an EROFS image within a block device
does not start at offset 0, it cannot be mounted directly either.

To address this issue, this patch adds a new mount option `offset=x'
to accept a start offset for both file-backed and bdev-based mounts.
The offset should be aligned to PAGE_SIZE. EROFS will add this offset
before performing read requests.

Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
Signed-off-by: Wang Shuai <wangshuai12@xiaomi.com>
---
 Documentation/filesystems/erofs.rst |  1 +
 fs/erofs/data.c                     |  8 ++++++--
 fs/erofs/fileio.c                   |  8 ++++++--
 fs/erofs/internal.h                 |  2 ++
 fs/erofs/super.c                    | 29 ++++++++++++++++++++++++++++-
 fs/erofs/zdata.c                    | 13 +++++++++++--
 6 files changed, 54 insertions(+), 7 deletions(-)
---
Hi, all

Sorry for late :-(

v2: * add a new mount option `offset=X' for start offset, and offset
      should be aligned to PAGE_SIZE
    * add start offset for both file-backed and bdev-based mounts
v1: https://lore.kernel.org/all/20250324022849.2715578-1-shengyong1@xiaomi.com/

diff --git a/Documentation/filesystems/erofs.rst b/Documentation/filesystems/erofs.rst
index c293f8e37468..44dbfa6cffb1 100644
--- a/Documentation/filesystems/erofs.rst
+++ b/Documentation/filesystems/erofs.rst
@@ -128,6 +128,7 @@ 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.
+offset=%s              Specify image offset for file-backed or bdev-based mounts.
 ===================    =========================================================
 
 Sysfs Entries
diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 2409d2ab0c28..8dfdd0352c79 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -27,9 +27,12 @@ void erofs_put_metabuf(struct erofs_buf *buf)
 
 void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
 {
-	pgoff_t index = offset >> PAGE_SHIFT;
+	pgoff_t index;
 	struct folio *folio = NULL;
 
+	offset += buf->offs;
+	index = offset >> PAGE_SHIFT;
+
 	if (buf->page) {
 		folio = page_folio(buf->page);
 		if (folio_file_page(folio, index) != buf->page)
@@ -54,6 +57,7 @@ void erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb)
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 
 	buf->file = NULL;
+	buf->offs = sbi->dif0.offs;
 	if (erofs_is_fileio_mode(sbi)) {
 		buf->file = sbi->dif0.file;	/* some fs like FUSE needs it */
 		buf->mapping = buf->file->f_mapping;
@@ -299,7 +303,7 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 		iomap->private = buf.base;
 	} else {
 		iomap->type = IOMAP_MAPPED;
-		iomap->addr = mdev.m_pa;
+		iomap->addr = EROFS_SB(sb)->dif0.offs + mdev.m_pa;
 		if (flags & IOMAP_DAX)
 			iomap->addr += mdev.m_dif->dax_part_off;
 	}
diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index 4fa0a0121288..d213bcb8c6c2 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -47,15 +47,19 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
 
 static void erofs_fileio_rq_submit(struct erofs_fileio_rq *rq)
 {
+	struct erofs_sb_info *sbi;
 	struct iov_iter iter;
 	int ret;
 
 	if (!rq)
 		return;
-	rq->iocb.ki_pos = rq->bio.bi_iter.bi_sector << SECTOR_SHIFT;
+
+	sbi = EROFS_SB(rq->sb);
+	rq->iocb.ki_pos = sbi->dif0.offs +
+				(rq->bio.bi_iter.bi_sector << SECTOR_SHIFT);
 	rq->iocb.ki_ioprio = get_current_ioprio();
 	rq->iocb.ki_complete = erofs_fileio_ki_complete;
-	if (test_opt(&EROFS_SB(rq->sb)->opt, DIRECT_IO) &&
+	if (test_opt(&sbi->opt, DIRECT_IO) &&
 	    rq->iocb.ki_filp->f_mode & FMODE_CAN_ODIRECT)
 		rq->iocb.ki_flags = IOCB_DIRECT;
 	iov_iter_bvec(&iter, ITER_DEST, rq->bvecs, rq->bio.bi_vcnt,
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 4ac188d5d894..93fc111fa675 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -43,6 +43,7 @@ struct erofs_device_info {
 	char *path;
 	struct erofs_fscache *fscache;
 	struct file *file;
+	loff_t offs;
 	struct dax_device *dax_dev;
 	u64 dax_part_off;
 
@@ -199,6 +200,7 @@ enum {
 struct erofs_buf {
 	struct address_space *mapping;
 	struct file *file;
+	loff_t offs;
 	struct page *page;
 	void *base;
 };
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index cadec6b1b554..0569f62a76a8 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -356,7 +356,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_device, Opt_fsid, Opt_domain_id, Opt_directio, Opt_offset,
 	Opt_err
 };
 
@@ -384,6 +384,7 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
 	fsparam_string("fsid",		Opt_fsid),
 	fsparam_string("domain_id",	Opt_domain_id),
 	fsparam_flag_no("directio",	Opt_directio),
+	fsparam_string("offset",	Opt_offset),
 	{}
 };
 
@@ -411,6 +412,22 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
 #endif
 }
 
+static bool erofs_fc_set_offset(struct fs_context *fc,
+				struct fs_parameter *param)
+{
+	struct erofs_sb_info *sbi = fc->s_fs_info;
+	loff_t offs;
+
+	if (kstrtoll(param->string, 0, &offs) < 0 ||
+	    offs < 0 || offs & ~PAGE_MASK) {
+		errorfc(fc, "Invalid offset %s", param->string);
+		return false;
+	}
+	sbi->dif0.offs = offs;
+
+	return true;
+}
+
 static int erofs_fc_parse_param(struct fs_context *fc,
 				struct fs_parameter *param)
 {
@@ -507,6 +524,10 @@ static int erofs_fc_parse_param(struct fs_context *fc,
 		errorfc(fc, "%s option not supported", erofs_fs_parameters[opt].name);
 #endif
 		break;
+	case Opt_offset:
+		if (!erofs_fc_set_offset(fc, param))
+			return -EINVAL;
+		break;
 	}
 	return 0;
 }
@@ -697,6 +718,10 @@ static int erofs_fc_get_tree(struct fs_context *fc)
 		file = filp_open(fc->source, O_RDONLY | O_LARGEFILE, 0);
 		if (IS_ERR(file))
 			return PTR_ERR(file);
+		if (sbi->dif0.offs + PAGE_SIZE >= i_size_read(file_inode(file))) {
+			fput(file);
+			return invalf(fc, "Start offset too large");
+		}
 		sbi->dif0.file = file;
 
 		if (S_ISREG(file_inode(sbi->dif0.file)->i_mode) &&
@@ -948,6 +973,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 	if (sbi->domain_id)
 		seq_printf(seq, ",domain_id=%s", sbi->domain_id);
 #endif
+	if (sbi->dif0.offs)
+		seq_printf(seq, ",offset=%lld", sbi->dif0.offs);
 	return 0;
 }
 
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 0671184d9cf1..dc2aa3418094 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -1619,6 +1619,15 @@ static void z_erofs_endio(struct bio *bio)
 		bio_put(bio);
 }
 
+static void z_erofs_submit_bio(struct bio *bio)
+{
+	struct z_erofs_decompressqueue *q = bio->bi_private;
+	struct erofs_sb_info *sbi = EROFS_SB(q->sb);
+
+	bio->bi_iter.bi_sector += sbi->dif0.offs >> SECTOR_SHIFT;
+	submit_bio(bio);
+}
+
 static void z_erofs_submit_queue(struct z_erofs_frontend *f,
 				 struct z_erofs_decompressqueue *fgq,
 				 bool *force_fg, bool readahead)
@@ -1678,7 +1687,7 @@ static void z_erofs_submit_queue(struct z_erofs_frontend *f,
 				else if (erofs_is_fscache_mode(sb))
 					erofs_fscache_submit_bio(bio);
 				else
-					submit_bio(bio);
+					z_erofs_submit_bio(bio);
 
 				if (memstall) {
 					psi_memstall_leave(&pflags);
@@ -1737,7 +1746,7 @@ static void z_erofs_submit_queue(struct z_erofs_frontend *f,
 		else if (erofs_is_fscache_mode(sb))
 			erofs_fscache_submit_bio(bio);
 		else
-			submit_bio(bio);
+			z_erofs_submit_bio(bio);
 	}
 	if (memstall)
 		psi_memstall_leave(&pflags);
-- 
2.43.0

Re: [PATCH v2 1/2] erofs: set error to bio if file-backed IO fails

[Gao Xiang]

On 2025/3/31 10:29, Sheng Yong wrote:
> From: Sheng Yong <shengyong1@xiaomi.com>
> 
> If a file-backed IO fails before submitting the bio to the lower
> filesystem, an error is returned, but the bio->bi_status is not
> marked as an error. However, the error information should be passed
> to the end_io handler. Otherwise, the IO request will be treated as
> successful.
> 
> Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

Re: [PATCH v2 2/2] erofs: add 'offset' mount option for file-backed & bdev-based mounts

[Gao Xiang]

On 2025/3/31 10:29, Sheng Yong wrote:
> From: Sheng Yong <shengyong1@xiaomi.com>
> 
> When attempting to use an archive file, such as APEX on android,
> as a file-backed mount source, it fails because EROFS image within
> the archive file does not start at offset 0. As a result, a loop
> device is still needed to attach the image file at an appropriate
> offset first. Similarly, if an EROFS image within a block device
> does not start at offset 0, it cannot be mounted directly either.
> 
> To address this issue, this patch adds a new mount option `offset=x'
> to accept a start offset for both file-backed and bdev-based mounts.
> The offset should be aligned to PAGE_SIZE. EROFS will add this offset
> before performing read requests.
> 
> Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
> Signed-off-by: Wang Shuai <wangshuai12@xiaomi.com>
> ---
>   Documentation/filesystems/erofs.rst |  1 +
>   fs/erofs/data.c                     |  8 ++++++--
>   fs/erofs/fileio.c                   |  8 ++++++--
>   fs/erofs/internal.h                 |  2 ++
>   fs/erofs/super.c                    | 29 ++++++++++++++++++++++++++++-
>   fs/erofs/zdata.c                    | 13 +++++++++++--
>   6 files changed, 54 insertions(+), 7 deletions(-)
> ---
> Hi, all
> 
> Sorry for late :-(
> 
> v2: * add a new mount option `offset=X' for start offset, and offset
>        should be aligned to PAGE_SIZE
>      * add start offset for both file-backed and bdev-based mounts
> v1: https://lore.kernel.org/all/20250324022849.2715578-1-shengyong1@xiaomi.com/
> 
> diff --git a/Documentation/filesystems/erofs.rst b/Documentation/filesystems/erofs.rst
> index c293f8e37468..44dbfa6cffb1 100644
> --- a/Documentation/filesystems/erofs.rst
> +++ b/Documentation/filesystems/erofs.rst
> @@ -128,6 +128,7 @@ 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.
> +offset=%s              Specify image offset for file-backed or bdev-based mounts.
>   ===================    =========================================================
>   
>   Sysfs Entries
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 2409d2ab0c28..8dfdd0352c79 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -27,9 +27,12 @@ void erofs_put_metabuf(struct erofs_buf *buf)
>   
>   void *erofs_bread(struct erofs_buf *buf, erofs_off_t offset, bool need_kmap)
>   {
> -	pgoff_t index = offset >> PAGE_SHIFT;
> +	pgoff_t index;
>   	struct folio *folio = NULL;
>   
> +	offset += buf->offs;
> +	index = offset >> PAGE_SHIFT;
> +
>   	if (buf->page) {
>   		folio = page_folio(buf->page);
>   		if (folio_file_page(folio, index) != buf->page)
> @@ -54,6 +57,7 @@ void erofs_init_metabuf(struct erofs_buf *buf, struct super_block *sb)
>   	struct erofs_sb_info *sbi = EROFS_SB(sb);
>   
>   	buf->file = NULL;
> +	buf->offs = sbi->dif0.offs;
>   	if (erofs_is_fileio_mode(sbi)) {
>   		buf->file = sbi->dif0.file;	/* some fs like FUSE needs it */
>   		buf->mapping = buf->file->f_mapping;
> @@ -299,7 +303,7 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>   		iomap->private = buf.base;
>   	} else {
>   		iomap->type = IOMAP_MAPPED;
> -		iomap->addr = mdev.m_pa;
> +		iomap->addr = EROFS_SB(sb)->dif0.offs + mdev.m_pa;
>   		if (flags & IOMAP_DAX)
>   			iomap->addr += mdev.m_dif->dax_part_off;
>   	}
> diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
> index 4fa0a0121288..d213bcb8c6c2 100644
> --- a/fs/erofs/fileio.c
> +++ b/fs/erofs/fileio.c
> @@ -47,15 +47,19 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
>   
>   static void erofs_fileio_rq_submit(struct erofs_fileio_rq *rq)
>   {
> +	struct erofs_sb_info *sbi;

	struct erofs_sb_info *sbi = EROFS_SB(rq->sb);

>   	struct iov_iter iter;
>   	int ret;
>   
>   	if (!rq)
>   		return;
> -	rq->iocb.ki_pos = rq->bio.bi_iter.bi_sector << SECTOR_SHIFT;
> +
> +	sbi = EROFS_SB(rq->sb);
> +	rq->iocb.ki_pos = sbi->dif0.offs +
> +				(rq->bio.bi_iter.bi_sector << SECTOR_SHIFT);
>   	rq->iocb.ki_ioprio = get_current_ioprio();
>   	rq->iocb.ki_complete = erofs_fileio_ki_complete;
> -	if (test_opt(&EROFS_SB(rq->sb)->opt, DIRECT_IO) &&
> +	if (test_opt(&sbi->opt, DIRECT_IO) &&
>   	    rq->iocb.ki_filp->f_mode & FMODE_CAN_ODIRECT)
>   		rq->iocb.ki_flags = IOCB_DIRECT;
>   	iov_iter_bvec(&iter, ITER_DEST, rq->bvecs, rq->bio.bi_vcnt,
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 4ac188d5d894..93fc111fa675 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -43,6 +43,7 @@ struct erofs_device_info {
>   	char *path;
>   	struct erofs_fscache *fscache;
>   	struct file *file;
> +	loff_t offs;

I think `off` is enough..

>   	struct dax_device *dax_dev;
>   	u64 dax_part_off;
>   
> @@ -199,6 +200,7 @@ enum {
>   struct erofs_buf {
>   	struct address_space *mapping;
>   	struct file *file;
> +	loff_t offs;

same here...

>   	struct page *page;>   	void *base;
>   };
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index cadec6b1b554..0569f62a76a8 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -356,7 +356,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_device, Opt_fsid, Opt_domain_id, Opt_directio, Opt_offset,
>   	Opt_err
>   };
>   
> @@ -384,6 +384,7 @@ static const struct fs_parameter_spec erofs_fs_parameters[] = {
>   	fsparam_string("fsid",		Opt_fsid),
>   	fsparam_string("domain_id",	Opt_domain_id),
>   	fsparam_flag_no("directio",	Opt_directio),
> +	fsparam_string("offset",	Opt_offset),
>   	{}
>   };
>   
> @@ -411,6 +412,22 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, unsigned int mode)
>   #endif
>   }
>   
> +static bool erofs_fc_set_offset(struct fs_context *fc,
> +				struct fs_parameter *param)
> +{
> +	struct erofs_sb_info *sbi = fc->s_fs_info;
> +	loff_t offs;
> +
> +	if (kstrtoll(param->string, 0, &offs) < 0 ||
> +	    offs < 0 || offs & ~PAGE_MASK) {
> +		errorfc(fc, "Invalid offset %s", param->string);
> +		return false;
> +	}


Can we validate this offset in blocks rather than
PAGE_SIZE, I mean, we could validate it in fill_super().

> +	sbi->dif0.offs = offs;
> +
> +	return true;
> +}
> +
>   static int erofs_fc_parse_param(struct fs_context *fc,
>   				struct fs_parameter *param)
>   {
> @@ -507,6 +524,10 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>   		errorfc(fc, "%s option not supported", erofs_fs_parameters[opt].name);
>   #endif
>   		break;
> +	case Opt_offset:
> +		if (!erofs_fc_set_offset(fc, param))
> +			return -EINVAL;
> +		break;
>   	}
>   	return 0;
>   }
> @@ -697,6 +718,10 @@ static int erofs_fc_get_tree(struct fs_context *fc)
>   		file = filp_open(fc->source, O_RDONLY | O_LARGEFILE, 0);
>   		if (IS_ERR(file))
>   			return PTR_ERR(file);
> +		if (sbi->dif0.offs + PAGE_SIZE >= i_size_read(file_inode(file))) {
> +			fput(file);
> +			return invalf(fc, "Start offset too large");
> +		}

same here.

>   		sbi->dif0.file = file;
>   
>   		if (S_ISREG(file_inode(sbi->dif0.file)->i_mode) &&
> @@ -948,6 +973,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>   	if (sbi->domain_id)
>   		seq_printf(seq, ",domain_id=%s", sbi->domain_id);
>   #endif
> +	if (sbi->dif0.offs)
> +		seq_printf(seq, ",offset=%lld", sbi->dif0.offs);
>   	return 0;
>   }
>   
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 0671184d9cf1..dc2aa3418094 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -1619,6 +1619,15 @@ static void z_erofs_endio(struct bio *bio)
>   		bio_put(bio);
>   }
>   
> +static void z_erofs_submit_bio(struct bio *bio)
> +{
> +	struct z_erofs_decompressqueue *q = bio->bi_private;
> +	struct erofs_sb_info *sbi = EROFS_SB(q->sb);
> +
> +	bio->bi_iter.bi_sector += sbi->dif0.offs >> SECTOR_SHIFT;

just update bi_sector inline?

Thanks,
Gao Xiang