Re: [RFCv5 4/5] iomap: Allocate iop in ->write_begin() early

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Christoph Hellwig <hch@infradead.org> writes:

> On Mon, May 08, 2023 at 12:57:59AM +0530, Ritesh Harjani (IBM) wrote:
>> Earlier when the folio is uptodate, we only allocate iop at writeback
>
> s/Earlier/Currently/ ?
>
>> time (in iomap_writepage_map()). This is ok until now, but when we are
>> going to add support for per-block dirty state bitmap in iop, this
>> could cause some performance degradation. The reason is that if we don't
>> allocate iop during ->write_begin(), then we will never mark the
>> necessary dirty bits in ->write_end() call. And we will have to mark all
>> the bits as dirty at the writeback time, that could cause the same write
>> amplification and performance problems as it is now.
>>
>> However, for all the writes with (pos, len) which completely overlaps
>> the given folio, there is no need to allocate an iop during
>> ->write_begin(). So skip those cases.
>
> This reads a bit backwards, I'd suggest to mention early
> allocation only happens for sub-page writes before going into the
> details.
>

sub-page is a bit confusing here. Because we can have a large folio too
with blocks within that folio. So we decided to go with per-block
terminology [1].

[1]: https://lore.kernel.org/linux-xfs/ZFR%2FGuVca5nFlLYF@casper.infradead.org/

I am guessing you would like to me to re-write the above para. Is this better?

"We dont need to allocate an iop in ->write_begin() for writes where the
position and length completely overlap with the given folio.
Therefore, such cases are skipped."

> The changes themselves looks good to me.

Sure. Thanks!

-ritesh