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

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

[Zhu Jia]

Since commit cc5095747edf ("ext4: don't BUG if someone dirty pages
without asking ext4 first"), mpage_prepare_extent_to_map() handles dirty
folios without buffer heads by warning, clearing PG_dirty, and skipping
them. ext4 cannot write these folios because there are no buffer heads to
map and submit.

That recovery leaves dirty accounting behind: folio_clear_dirty() clears
PG_dirty but does not undo the accounting charged when the folio was
dirtied. We have seen this in production as Dirty/nr_dirty staying high
while Writeback/nr_writeback and device write IO stayed near zero, with
many writer tasks blocked in balance_dirty_pages() throttling. Thus the
warning-and-skip recovery can still become a dirty-throttle DoS.

Use folio_cancel_dirty() so dropping PG_dirty also cancels the dirty
accounting.

Fixes: cc5095747edf ("ext4: don't BUG if someone dirty pages without asking ext4 first")
Cc: stable@vger.kernel.org
Signed-off-by: Zhu Jia <zhujia.zj@bytedance.com>
---
 fs/ext4/inode.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c2c2d6ac7f3d1..7ea280e70c06e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2715,7 +2715,13 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
 			 */
 			if (!folio_buffers(folio)) {
 				ext4_warning_inode(mpd->inode, "page %lu does not have buffers attached", folio->index);
-				folio_clear_dirty(folio);
+				/*
+				 * folio_cancel_dirty() pairs the dropped dirty
+				 * state with dirty accounting, but leaves stale
+				 * PAGECACHE_TAG_DIRTY/TOWRITE tags behind. Later
+				 * writeback may rescan this clean folio.
+				 */
+				folio_cancel_dirty(folio);
 				folio_unlock(folio);
 				continue;
 			}
-- 
2.20.1

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

[Zhang Yi]

On 6/23/2026 5:49 PM, Zhu Jia wrote:
> Since commit cc5095747edf ("ext4: don't BUG if someone dirty pages
> without asking ext4 first"), mpage_prepare_extent_to_map() handles dirty
> folios without buffer heads by warning, clearing PG_dirty, and skipping
> them. ext4 cannot write these folios because there are no buffer heads to
> map and submit.
> 
> That recovery leaves dirty accounting behind: folio_clear_dirty() clears
> PG_dirty but does not undo the accounting charged when the folio was
> dirtied. We have seen this in production as Dirty/nr_dirty staying high
> while Writeback/nr_writeback and device write IO stayed near zero, with
> many writer tasks blocked in balance_dirty_pages() throttling. Thus the
> warning-and-skip recovery can still become a dirty-throttle DoS.
> 
> Use folio_cancel_dirty() so dropping PG_dirty also cancels the dirty
> accounting.

Hi, Zhu jia!

Thanks for the patch. This overall looks good to me. But should we also
clear PAGECACHE_TAG_DIRTY and PAGECACHE_TAG_TOWRITE here? Since the folio
won't be written back again until it gets dirtied, it seems cleaner to
remove these tags as well. Are there any side-effects I'm missing?

Thanks,
Yi.

> 
> Fixes: cc5095747edf ("ext4: don't BUG if someone dirty pages without asking ext4 first")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhu Jia <zhujia.zj@bytedance.com>
> ---
>  fs/ext4/inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c2c2d6ac7f3d1..7ea280e70c06e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2715,7 +2715,13 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
>  			 */
>  			if (!folio_buffers(folio)) {
>  				ext4_warning_inode(mpd->inode, "page %lu does not have buffers attached", folio->index);
> -				folio_clear_dirty(folio);
> +				/*
> +				 * folio_cancel_dirty() pairs the dropped dirty
> +				 * state with dirty accounting, but leaves stale
> +				 * PAGECACHE_TAG_DIRTY/TOWRITE tags behind. Later
> +				 * writeback may rescan this clean folio.
> +				 */
> +				folio_cancel_dirty(folio);
>  				folio_unlock(folio);
>  				continue;
>  			}

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

[Zhu Jia]

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.

Thanks,
Jia

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

[Jan Kara]

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
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Jan Kara]

On Tue 23-06-26 17:49:47, Zhu Jia wrote:
> Since commit cc5095747edf ("ext4: don't BUG if someone dirty pages
> without asking ext4 first"), mpage_prepare_extent_to_map() handles dirty
> folios without buffer heads by warning, clearing PG_dirty, and skipping
> them. ext4 cannot write these folios because there are no buffer heads to
> map and submit.
> 
> That recovery leaves dirty accounting behind: folio_clear_dirty() clears
> PG_dirty but does not undo the accounting charged when the folio was
> dirtied. We have seen this in production as Dirty/nr_dirty staying high
> while Writeback/nr_writeback and device write IO stayed near zero, with
> many writer tasks blocked in balance_dirty_pages() throttling. Thus the
> warning-and-skip recovery can still become a dirty-throttle DoS.
> 
> Use folio_cancel_dirty() so dropping PG_dirty also cancels the dirty
> accounting.
> 
> Fixes: cc5095747edf ("ext4: don't BUG if someone dirty pages without asking ext4 first")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhu Jia <zhujia.zj@bytedance.com>

Good point. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c2c2d6ac7f3d1..7ea280e70c06e 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2715,7 +2715,13 @@ static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
>  			 */
>  			if (!folio_buffers(folio)) {
>  				ext4_warning_inode(mpd->inode, "page %lu does not have buffers attached", folio->index);
> -				folio_clear_dirty(folio);
> +				/*
> +				 * folio_cancel_dirty() pairs the dropped dirty
> +				 * state with dirty accounting, but leaves stale
> +				 * PAGECACHE_TAG_DIRTY/TOWRITE tags behind. Later
> +				 * writeback may rescan this clean folio.
> +				 */
> +				folio_cancel_dirty(folio);
>  				folio_unlock(folio);
>  				continue;
>  			}
> -- 
> 2.20.1
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Zhu Jia]

On Wed, Jun 24, 2026 at 02:32:27PM +0200, Jan Kara wrote:
> On Wed 24-06-26 17:52:06, Zhu Jia wrote:
> > 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

Thanks, makes sense. I'll keep the fix as-is and leave the xarray tags
alone.

Thanks,
Jia

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

[Zhang Yi]

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?

Thanks,
Yi.

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@suse.com>
SUSE Labs, CR

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

[Zhu Jia]

On Thu, Jun 25, 2026 at 01:18:31PM +0200, Jan Kara wrote:
> 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:
> > > > 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).
> > 
> > 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!

Thanks Jan and Yi, your discussion makes sense. I will send v2 shortly.

Thanks,
Jia