| 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: <f714479d-703c-4fc6-ad5a-b18d92f0a9b7@linux.alibaba.com>
Date: Sun, 16 Nov 2025 20:01:24 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Hongbo Li <lihongbo22@...wei.com>, chao@...nel.org, brauner@...nel.org,
djwong@...nel.org, amir73il@...il.com, joannelkoong@...il.com
Cc: linux-fsdevel@...r.kernel.org, linux-erofs@...ts.ozlabs.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 2/9] erofs: hold read context in iomap_iter if needed
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 <lihongbo22@...wei.com>
> ---
> 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
Powered by blists - more mailing lists