linux-kernel - Re: [PATCH v9 01/10] iomap: stash iomap read ctx in the private field of iomap

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <add21bbf-1359-4659-9518-bdb1ef34ea48@linux.alibaba.com>
Date: Tue, 18 Nov 2025 15:35:45 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: brauner@...nel.org, djwong@...nel.org, Christoph Hellwig <hch@....de>
Cc: linux-fsdevel@...r.kernel.org, linux-erofs@...ts.ozlabs.org,
 linux-kernel@...r.kernel.org, Chao Yu <chao@...nel.org>,
 Amir Goldstein <amir73il@...il.com>, Joanne Koong <joannelkoong@...il.com>,
 Hongbo Li <lihongbo22@...wei.com>
Subject: Re: [PATCH v9 01/10] iomap: stash iomap read ctx in the private field
 of iomap_iter

(... try to add Christoph..)

On 2025/11/18 01:08, Gao Xiang wrote:
> Hi Darrick, Christian,
> 
> On 2025/11/17 21:25, Hongbo Li wrote:
>> It's useful to get filesystem-specific information using the
>> existing private field in the @iomap_iter passed to iomap_{begin,end}
>> for advanced usage for iomap buffered reads, which is much like the
>> current iomap DIO.
>>
>> For example, EROFS needs it to:
>>
>>   - implement an efficient page cache sharing feature, since iomap
>>     needs to apply to anon inode page cache but we'd like to get the
>>     backing inode/fs instead, so filesystem-specific private data is
>>     needed to keep such information;
>>
>>   - pass in both struct page * and void * for inline data to avoid
>>     kmap_to_page() usage (which is bogus).
>>
>> Reviewed-by: Gao Xiang <hsiangkao@...ux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@...wei.com>
> 
> Could you help review this iomap change, since erofs uses iomap
> and erofs page cache sharing needs this change, as I told
> Joanne months ago.
> 
> Even without the page cache sharing feature, introducing
> iomap_iter_ctx for .iomap_{begin,end}, like the current DIO
> does, is still useful for erofs, as patch 2 mentioned.

I know it could be somewhat too late to introduce the entire
feature for 6.19, but could we consider the first two patches
(patch 1 and 2) if possible? because:

  - patch 1 just adds a way to specify iter->private for buffered
    read since there was no way to pass on-stack fs-specific
    contexts from .iomap_begin() to .iomap_end() for iomap
    buffered read.

    Actually patch 1 doesn't introduce any new logic or behavior
    to iomap itself, just add a way to specify iter->private, I
    think it does no harm to the iomap stability.

  - patch 2 tries to avoid kmap_to_page() usage since previously
    there is no way to pass both `void *` and `struct page *`
    from .iomap_begin() to .iomap_end() because inline data
    handling needs both.

    Actually people would like to get rid of kmap_to_page(), for
    example:
    https://lore.kernel.org/r/Y5u+oOLkJs6jehik@iweiny-desk3

    So I wonder if patch 1 and 2 can be considered as an
    individual improvement for 6.19.

  - Currently the page cache sharing series is coupled with iomap,
    but the main change is still in erofs itself.  If the first
    two patches can be applied in advance, that would make the
    remaining part through erofs tree without treewide conflict
    like this.

Just my two cents.

Thanks,
Gao Xiang