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

[Gao Xiang <hsiangkao@linux.alibaba.com>]

(... 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@linux.alibaba.com>
>> Signed-off-by: Hongbo Li <lihongbo22@huawei.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