On Sat, Aug 17, 2024 at 12:27:49PM +0800, Zhang Yi wrote: > On 2024/8/14 13:32, Christoph Hellwig wrote: > > On Mon, Aug 12, 2024 at 08:11:56PM +0800, Zhang Yi wrote: > >> From: Zhang Yi <yi.zhang@xxxxxxxxxx> > >> > >> Now we allocate ifs if i_blocks_per_folio is larger than one when > >> writing back dirty folios in iomap_writepage_map(), so we don't attach > >> an ifs after buffer write to an entire folio until it starts writing > >> back, if we partial truncate that folio, iomap_invalidate_folio() can't > >> clear counterpart block's dirty bit as expected. Fix this by advance the > >> ifs allocation to __iomap_write_begin(). > > > > Wouldn't it make more sense to only allocate the ifѕ in > > iomap_invalidate_folio when it actually is needed? > > > > I forget to mention that truncate_inode_partial_folio() call > folio_invalidate()->iomap_invalidate_folio() only when the folio has > private, if the folio doesn't has ifs, the iomap_invalidate_folio() > would nerver be called, hence allocate the ifs in > iomap_invalidate_folio() is useless. > > In my opinion, one solution is change to always call folio_invalidate() > in truncate_inode_partial_folio(), all callbacks should handle the no > private case. Another solution is add a magic (a fake ifs) to > folio->private and then convert it to a real one in > iomap_invalidate_folio(), any thoughts? Why do we need iomap_invalidate_folio() to be called if there is no ifs? Even today, all it does is call ifs_free() if we're freeing the entire folio (which is done by truncate_cleanup_folio() and not by truncate_inode_partial_folio().
