Matthew Wilcox <willy@xxxxxxxxxxxxx> writes: > On Fri, May 19, 2023 at 08:48:37PM +0530, Ritesh Harjani wrote: >> Christoph Hellwig <hch@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx/ >> >> 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." > > ... and reorder that paragraph to be first. Sure. -ritesh
