Re: [PATCH] ext4: cancel dirty accounting for folios without buffers

[Jan Kara]

On Wed 24-06-26 21:29:58, Zhang Yi wrote:
> On 6/24/2026 8:32 PM, Jan Kara wrote:
> > On Wed 24-06-26 17:52:06, Zhu Jia wrote:
> > > Hi Yi,
> > > 
> > > Thanks for taking a look.
> > > 
> > > Yes, clearing PAGECACHE_TAG_DIRTY/TOWRITE would make the page-cache state
> > > cleaner. I had a version that did this by adding a helper around
> > > folio_cancel_dirty() and clearing the xarray tags after confirming the
> > > folio was still the same clean page-cache entry.
> > > 
> > > It looked like this:
> > > 
> > > static void ext4_cancel_dirty_folio(struct address_space *mapping,
> > > 				    struct folio *folio)
> > > {
> > > 	XA_STATE(xas, &mapping->i_pages, folio->index);
> > > 	unsigned long flags;
> > > 
> > > 	folio_cancel_dirty(folio);
> > > 
> > > 	xas_lock_irqsave(&xas, flags);
> > > 	if (xas_load(&xas) == folio && !folio_test_dirty(folio)) {
> > > 		xas_clear_mark(&xas, PAGECACHE_TAG_DIRTY);
> > > 		xas_clear_mark(&xas, PAGECACHE_TAG_TOWRITE);
> > > 	}
> > > 	xas_unlock_irqrestore(&xas, flags);
> > > }
> > > 
> > > The reason I left the tags unchanged in this version is that I was not sure
> > > whether it is appropriate for ext4 to open-code xarray tag cleanup directly.
> > > 
> > > If you think this is the right direction, I can add the helper back and
> > > send a v2.
> > 
> > That was a good judgement! Playing with xarray tags like this in filesystem
> > code is certainly not a good thing. For now, I'd leave the xarray tags
> > dangling - they will be eventually synced with reality on next writeback
> > attempt. If this inconsistency of tags needs to be fixed, the fix belongs
> > to the generic code (so that it can be used in other places as well).
> > 
> > 								Honza
> 
> Yes, I agree. Directly clearing the tag via open code is not a good
> approach. However, I took a look at the !nr_to_submit branch in
> ext4_bio_write_folio(), and it seems to have a similar simple handling
> pattern—it directly calls __folio_start_writeback() and
> folio_end_writeback(), which appears to be an elegant way to clear them.
> Could we also call these two helpers just after folio_cancel_dirty()
> here?

Right, that would be actually doable there and would keep things more
consistent so I think that's a good idea! Thanks!

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR