On Mon, Feb 27, 2023 at 01:13:30AM +0530, Ritesh Harjani (IBM) wrote: > Earlier when the folio is uptodate, we only allocate iop at writeback > time (in iomap_writepage_map()). This is ok until now, but when we are > going to add support for subpage size dirty bitmap tracking 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 (w/o subpage dirty > bitmap tracking in iop). > > 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. > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> > --- > fs/iomap/buffered-io.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 356193e44cf0..c5b51ab1184e 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -535,11 +535,16 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos, > size_t from = offset_in_folio(folio, pos), to = from + len; > size_t poff, plen; > > + if (pos <= folio_pos(folio) && > + pos + len >= folio_pos(folio) + folio_size(folio)) > + return 0; This is magic without a comment explaining why it exists. You have that explanation in the commit message, but that doesn't help anyone looking at the code: /* * If the write completely overlaps the current folio, then * entire folio will be dirtied so there is no need for * sub-folio state tracking structures to be attached to this folio. */ -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
