From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-124.freemail.mail.aliyun.com (out30-124.freemail.mail.aliyun.com [115.124.30.124]) (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 89F0A27B358; Sun, 16 Nov 2025 12:01:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763294498; cv=none; b=AMbcZAMncBE6d4gAmEo2TLUrPlfRgPmgRNhQPVKv8VDiQafpUG5tyN4gD8zpFdHvx/DEQWslKBw+2rOpOHjQG+nyHqGU/w/B5pYZhSw+ODb4F3LGy8Udv7gu8kOup130g/QmRfHXl3t2trQOH0YgXcidVNSlUD4/qjaZ0UtMFyI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763294498; c=relaxed/simple; bh=6h324CmcpE1QnCciiHMSZSzKHPisb2qv228GlvfkZRg=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=XdzDVb9kodMv86UvU5T+62zZeL1XpfCH1UpuU6DdiL22H4gnZLxXI3VxOOYp7w4dkYtQUJl/tIFWVuWpk1I6oDmvTVK4s3EQd9JJs/gVHqOcqlT6sB3gl1E0AvSElihVKOBZ3nVqDICHFRwJbPDwzvESwWVpMIyV/jnHPqxgqxo= 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=ct94zb+Y; arc=none smtp.client-ip=115.124.30.124 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="ct94zb+Y" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1763294485; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=gneyMDW+YpWlILj4VMhSNxwPUQEUD67fBdmsbKndoUU=; b=ct94zb+Ye/jPP4TzyHNuArUBTFGNIutZGWjOmU7MAhqka1fByfD+mgoH65z6crqANcmZC5tKkFHJ+NVzfgpOrYxFq2IdhD2Oc/ChY/QqOXoLK9jwd9j5zqkSDC0GaVZgz9uCPcKAuMgo3RoYqgBoG/sdFsgnTcqFu/YIUAXzhgg= Received: from 30.170.196.81(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0WsSHSBM_1763294484 cluster:ay36) by smtp.aliyun-inc.com; Sun, 16 Nov 2025 20:01:25 +0800 Message-ID: Date: Sun, 16 Nov 2025 20:01:24 +0800 Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 2/9] erofs: hold read context in iomap_iter if needed To: Hongbo Li , chao@kernel.org, brauner@kernel.org, djwong@kernel.org, amir73il@gmail.com, joannelkoong@gmail.com Cc: linux-fsdevel@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-kernel@vger.kernel.org References: <20251114095516.207555-1-lihongbo22@huawei.com> <20251114095516.207555-3-lihongbo22@huawei.com> From: Gao Xiang In-Reply-To: <20251114095516.207555-3-lihongbo22@huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2025/11/14 17:55, Hongbo Li wrote: > Uncoming page cache sharing needs pass read context to iomap_iter, > here we unify the way of passing the read context in EROFS. Moreover, > bmap and fiemap don't need to map the inline data. > > Note that we keep `struct page *` in `struct erofs_iomap_iter_ctx` as > well to avoid bogus kmap_to_page usage. > > Signed-off-by: Hongbo Li > --- > fs/erofs/data.c | 79 ++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 59 insertions(+), 20 deletions(-) > > diff --git a/fs/erofs/data.c b/fs/erofs/data.c > index bb13c4cb8455..bd3d85c61341 100644 > --- a/fs/erofs/data.c > +++ b/fs/erofs/data.c > @@ -266,14 +266,23 @@ void erofs_onlinefolio_end(struct folio *folio, int err, bool dirty) > folio_end_read(folio, !(v & BIT(EROFS_ONLINEFOLIO_EIO))); > } > > +struct erofs_iomap_iter_ctx { > + struct page *page; > + void *base; > +}; > + > static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length, > unsigned int flags, struct iomap *iomap, struct iomap *srcmap) > { > int ret; > + struct erofs_iomap_iter_ctx *ctx; > struct super_block *sb = inode->i_sb; > struct erofs_map_blocks map; > struct erofs_map_dev mdev; > + struct iomap_iter *iter; > > + iter = container_of(iomap, struct iomap_iter, iomap); > + ctx = iter->private; Can you just rearrange it as: struct iomap_iter *iter = container_of(iomap, struct iomap_iter, iomap); struct erofs_iomap_iter_ctx *ctx = iter->private; ? > map.m_la = offset; > map.m_llen = length; > ret = erofs_map_blocks(inode, &map); > @@ -283,7 +292,8 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length, > iomap->offset = map.m_la; > iomap->length = map.m_llen; > iomap->flags = 0; > - iomap->private = NULL; > + if (ctx) > + ctx->base = NULL; I think this line is unnecessary if iter->private == ctx; > iomap->addr = IOMAP_NULL_ADDR; > if (!(map.m_flags & EROFS_MAP_MAPPED)) { > iomap->type = IOMAP_HOLE; > @@ -309,16 +319,20 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length, > } > > if (map.m_flags & EROFS_MAP_META) { > - void *ptr; > - struct erofs_buf buf = __EROFS_BUF_INITIALIZER; > - > iomap->type = IOMAP_INLINE; > - ptr = erofs_read_metabuf(&buf, sb, map.m_pa, > - erofs_inode_in_metabox(inode)); > - if (IS_ERR(ptr)) > - return PTR_ERR(ptr); > - iomap->inline_data = ptr; > - iomap->private = buf.base; > + /* read context should read the inlined data */ > + if (ctx) { > + void *ptr; > + struct erofs_buf buf = __EROFS_BUF_INITIALIZER; better to resort them as: struct erofs_buf buf = __EROFS_BUF_INITIALIZER; void *ptr; > + > + ptr = erofs_read_metabuf(&buf, sb, map.m_pa, > + erofs_inode_in_metabox(inode)); > + if (IS_ERR(ptr)) > + return PTR_ERR(ptr); > + iomap->inline_data = ptr; > + ctx->page = buf.page; > + ctx->base = buf.base; > + } > } else { > iomap->type = IOMAP_MAPPED; > } > @@ -328,18 +342,19 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length, > static int erofs_iomap_end(struct inode *inode, loff_t pos, loff_t length, > ssize_t written, unsigned int flags, struct iomap *iomap) > { > - void *ptr = iomap->private; > + struct erofs_iomap_iter_ctx *ctx; > + struct iomap_iter *iter; > > - if (ptr) { > + iter = container_of(iomap, struct iomap_iter, iomap); > + ctx = iter->private; > + if (ctx && ctx->base) { > struct erofs_buf buf = { > - .page = kmap_to_page(ptr), > - .base = ptr, > + .page = ctx->page, > + .base = ctx->base, > }; > > DBG_BUGON(iomap->type != IOMAP_INLINE); > erofs_put_metabuf(&buf); so need to nullify ctx->base here: ctx->base = NULL; > - } else { > - DBG_BUGON(iomap->type == IOMAP_INLINE); > } > return written; > } > @@ -369,18 +384,36 @@ int erofs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > */ > static int erofs_read_folio(struct file *file, struct folio *folio) > { > + struct iomap_read_folio_ctx read_ctx = { > + .ops = &iomap_bio_read_ops, > + .cur_folio = folio, > + }; > + struct erofs_iomap_iter_ctx iter_ctx = { > + .page = NULL, > + .base = NULL, > + }; it can be initialized just by: struct erofs_iomap_iter_ctx iter_ctx = {}; > + > trace_erofs_read_folio(folio, true); > > - iomap_bio_read_folio(folio, &erofs_iomap_ops); > + iomap_read_folio(&erofs_iomap_ops, &read_ctx, &iter_ctx); > return 0; > } > > static void erofs_readahead(struct readahead_control *rac) > { > + struct iomap_read_folio_ctx read_ctx = { > + .ops = &iomap_bio_read_ops, > + .rac = rac, > + }; > + struct erofs_iomap_iter_ctx iter_ctx = { > + .page = NULL, > + .base = NULL, > + }; Same here. > + > trace_erofs_readahead(rac->mapping->host, readahead_index(rac), > readahead_count(rac), true); > > - iomap_bio_readahead(rac, &erofs_iomap_ops); > + iomap_readahead(&erofs_iomap_ops, &read_ctx, &iter_ctx); > } > > static sector_t erofs_bmap(struct address_space *mapping, sector_t block) > @@ -400,9 +433,15 @@ static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to) > if (IS_DAX(inode)) > return dax_iomap_rw(iocb, to, &erofs_iomap_ops); > #endif > - if ((iocb->ki_flags & IOCB_DIRECT) && inode->i_sb->s_bdev) > + if ((iocb->ki_flags & IOCB_DIRECT) && inode->i_sb->s_bdev) { > + struct erofs_iomap_iter_ctx iter_ctx = { > + .page = NULL, > + .base = NULL, > + }; Same here again. Thanks, Gao Xiang