Re: [PATCH -next] iomap: fix inline data on buffered read

Gao Xiang
2025-03-19T08:34:50Z

Hi Christoph,

On 2025/3/19 16:23, Christoph Hellwig wrote:
> On Wed, Mar 19, 2025 at 09:17:30AM +0100, Christoph Hellwig wrote:
>> I'd move the iomap_iter_advance into iomap_read_inline_data, just like
>> we've pushed it down as far as possible elsewhere, e.g. something like
>> the patch below. Although with that having size and length puzzles
>> me a bit, so maybe someone more familar with the code could figure
>> out why we need both, how they can be different and either document
>> or eliminate that.
>
> ... and this doesn't even compile because it breaks write_begin.
> So we'll need to keep it in the caller, but maybe without the
> goto and just do the plain advance on length?

Yeah, I was just writing an email to your previous reply:

I think iomap_write_begin_inline() will break if
iomap_iter_advance() is in iomap_read_inline_data().

Because:
iomap_write_iter
iomap_write_begin
iomap_write_begin_inline
iomap_read_inline_data
iomap_iter_advance # 1
copy_folio_from_iter_atomic
iomap_write_end
...
iomap_iter_advance # 1

I will do a plain advance as your suggested instead, but commit
"iomap: advance the iter directly on buffered read" makes EROFS
unusable, and I think gfs2 too. It needs be fixed now.

Thanks,
Gao Xiang

Thanks,
Gao Xiang