From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-118.freemail.mail.aliyun.com (out30-118.freemail.mail.aliyun.com [115.124.30.118]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AEE305FB95 for ; Mon, 31 Mar 2025 05:45:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.118 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743399910; cv=none; b=MNONYutqYN2dHo1wzavs2oXIiKC3zqQLkKAVBLioRkndFWLpldg1QDBl8Q4tbyyIAFEcZpenhCb68H1yiNIWFbfITVGBD8DtoKjmdwXkU6nNRiGeU4goxU2IYjl7rjDBMGJSesIiioJccxmigDEsMenHCaqxpO6N+AjBk9y1S1Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1743399910; c=relaxed/simple; bh=nseH7wfMiCXDNqnHGuESu2Fyb2jbzz3QJpKZB/0z02Q=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=m3ROGSMDNkNP63z3GoapdK3KSOp7MF+ygjrpOEhvVne6OgseFGSADzlbLFiHDa0cmhol2UIzS2kqGqVAygH2M/bJHn2BFcA4aK9BNpcMrqMIjXn9QRzW8L/lI+ylHLtwkMfZ0NTnDs1VxyRooSaRepX1Bw07/sPHwSeYt08ghrQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=QSHzCEmq; arc=none smtp.client-ip=115.124.30.118 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="QSHzCEmq" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1743399903; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=jYiHigV/9QVCUX/RST2nWsvZk5MllFtjrbF7BcIgPaE=; b=QSHzCEmqnJ+8IRP5lyao16CtkbTsr57J4oeOXgOf4VE9o4doB7vS4YHL7IOcwhaimPk/ZMM6811XWMZlH3mSzAhPIcFxG7k46XpCrlug6ToJcGUeC3A5lIv9Vl4FsaAVsw03Kak0f4bfIS9hS8FQQ9ksH+Ae/pDyY+xmXZ33xcg= Received: from 30.221.130.198(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0WTQwMDe_1743399902 cluster:ay36) by smtp.aliyun-inc.com; Mon, 31 Mar 2025 13:45:02 +0800 Message-ID: <0725c2ec-528c-42a8-9557-4713e7e35153@linux.alibaba.com> Date: Mon, 31 Mar 2025 13:45:02 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 2/2] erofs: add 'offset' mount option for file-backed & bdev-based mounts To: Sheng Yong , xiang@kernel.org, chao@kernel.org, zbestahu@gmail.com, jefflexu@linux.alibaba.com, dhavale@google.com, linux-erofs@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org, wangshuai12@xiaomi.com, Sheng Yong References: <20250331022938.4090283-1-shengyong1@xiaomi.com> <20250331022938.4090283-2-shengyong1@xiaomi.com> From: Gao Xiang In-Reply-To: <20250331022938.4090283-2-shengyong1@xiaomi.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2025/3/31 10:29, Sheng Yong wrote: > From: Sheng Yong > > 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 > Signed-off-by: Wang Shuai > --- > 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