Function to do end-writeback in bulk

Function to do end-writeback in bulk

[David Howells]

Hi Willy,

Can you take a look at the function below?  This is what I've come up with for
the moment for a basic bulk end-writeback (and I'll need something similar for
bulk unlock).  At the moment, it returns true/false depending on whether it
made progress, but I'm thinking that it might be better if it returns the
remaining size of any folio that partially overlaps the end position and so
didn't get unlocked.  This would could allow me to set a minimum amount to
accrue before I call it again.  Whether progress was made can also be
determined by comparing @from before and after.

Thanks,
David
---
/**
 * folio_end_writeback_range - End writeback for the folios within the range
 * @mapping: The pagecache to modify
 * @from: Pointer to the starting position (updated)
 * @to: The end position (exclusive)
 * @cleaner_func: Function to clean up the folios in the range
 * @cleaner_priv: Private data for the cleaner func
 *
 * Unlock folios that are entirely within in the given range, where @from is
 * included in the range, but @to is excluded from the range.
 *
 * Return: True if at least one folio got cleaned, false otherwise.  @from will
 * be updated to point past the last folio cleaned.
 */
static inline
bool folio_end_writeback_range(struct address_space *mapping,
			       uoff_t *from, uoff_t to,
			       void (*cleaner_func)(struct folio *folio,
						    void *cleaner_priv),
			       void *cleaner_priv)
{
	struct folio *folio;
	XA_STATE(xas, &mapping->i_pages, *from / PAGE_SIZE);
	bool cleaned = false;

	rcu_read_lock();
	xas_for_each(&xas, folio, (to - 1) / PAGE_SIZE) {
		uoff_t fend;

		if (xas_retry(&xas, folio))
			continue;

		fend = folio_next_pos(folio);
		if (fend > to)
			break;

		cleaner_func(folio, cleaner_priv);
		folio_end_writeback(folio);
		*from = fend;
		cleaned = true;
	}
	rcu_read_unlock();
	return cleaned;
}

Re: Function to do end-writeback in bulk

[Matthew Wilcox]

On Mon, Aug 24, 2026 at 04:09:20PM +0100, David Howells wrote:
> Can you take a look at the function below?  This is what I've come up with for
> the moment for a basic bulk end-writeback (and I'll need something similar for
> bulk unlock).  At the moment, it returns true/false depending on whether it
> made progress, but I'm thinking that it might be better if it returns the
> remaining size of any folio that partially overlaps the end position and so
> didn't get unlocked.  This would could allow me to set a minimum amount to
> accrue before I call it again.  Whether progress was made can also be
> determined by comparing @from before and after.
> 
> Thanks,
> David
> ---
> /**
>  * folio_end_writeback_range - End writeback for the folios within the range
>  * @mapping: The pagecache to modify
>  * @from: Pointer to the starting position (updated)
>  * @to: The end position (exclusive)
>  * @cleaner_func: Function to clean up the folios in the range
>  * @cleaner_priv: Private data for the cleaner func

I hate this API.  I much prefer the _iter() style:

struct end_writeback_ctrl {
	struct xa_state xas;
	uoff_t fend;
};

struct folio *end_writeback_iter(struct address_space *mapping,
		uoff_t from, uoff_t to,
		struct end_writeback_ctrl *ctrl, struct folio *folio)
{
	if (!folio) {
		ctrl->xas = __XA_STATE(&mapping->i_pages, from / PAGE_SIZE,
				0, 0);
		folio = xas_find(&ctrl->xas, to / PAGE_SIZE);
	} else {
retry:
		folio_end_writeback(folio);
		folio = xas_next_entry(&ctrl->xas, to / PAGE_SIZE);
	}

	if (xas_retry(&xas, folio))
		goto retry;
	ctrl->fend = folio_next_pos(folio);
	if (ctrl->fend > to)
		folio = NULL;

	return folio;
}

(you can embed the rcu_read_lock() / unlock in here too, but probably
better to do it in the caller)

>  * Unlock folios that are entirely within in the given range, where @from is
>  * included in the range, but @to is excluded from the range.
>  *
>  * Return: True if at least one folio got cleaned, false otherwise.  @from will
>  * be updated to point past the last folio cleaned.
>  */
> static inline
> bool folio_end_writeback_range(struct address_space *mapping,
> 			       uoff_t *from, uoff_t to,
> 			       void (*cleaner_func)(struct folio *folio,
> 						    void *cleaner_priv),
> 			       void *cleaner_priv)
> {
> 	struct folio *folio;
> 	XA_STATE(xas, &mapping->i_pages, *from / PAGE_SIZE);
> 	bool cleaned = false;
> 
> 	rcu_read_lock();
> 	xas_for_each(&xas, folio, (to - 1) / PAGE_SIZE) {
> 		uoff_t fend;
> 
> 		if (xas_retry(&xas, folio))
> 			continue;
> 
> 		fend = folio_next_pos(folio);
> 		if (fend > to)
> 			break;
> 
> 		cleaner_func(folio, cleaner_priv);
> 		folio_end_writeback(folio);
> 		*from = fend;
> 		cleaned = true;
> 	}
> 	rcu_read_unlock();
> 	return cleaned;
> }
> 

Re: Function to do end-writeback in bulk

[David Howells]

Matthew Wilcox <willy@infradead.org> wrote:

> I hate this API.  I much prefer the _iter() style:

I'm not that keen on the _iter() style, but whatever.

However, does that make it harder to do the stats manipulation in bulk in
future?

I was looking at __folio_end_writeback(), and I see:

	....
			wb = inode_to_wb(inode);
			wb_stat_mod(wb, WB_WRITEBACK, -nr);
			__wb_writeout_add(wb, nr);
			if (!mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
				wb_inode_writeback_end(wb);
				if (mapping->host)
					sb_clear_inode_writeback(mapping->host);
			}
	...
		lruvec_stat_mod_folio(folio, NR_WRITEBACK, -nr);
		zone_stat_mod_folio(folio, NR_ZONE_WRITE_PENDING, -nr);
		node_stat_mod_folio(folio, NR_WRITTEN, nr);
	...

And I was thinking those could be done in bulk... but there seems to be an IRQ
disablement requirement around them.  Does the folio_xor_flags_has_waiters()
have to be done inside?  I presume this is to prevent a set/clear race on the
master WRITEBACK tag.

David

Re: Function to do end-writeback in bulk

[Matthew Wilcox]

On Mon, Aug 24, 2026 at 04:47:08PM +0100, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
> 
> > I hate this API.  I much prefer the _iter() style:
> 
> I'm not that keen on the _iter() style, but whatever.

The implementation is more contorted (for the iter style), but it makes
the callers easier to write as they don't need the callback function and
to package all the data it needs up into a struct.

> However, does that make it harder to do the stats manipulation in bulk in
> future?
> 
> I was looking at __folio_end_writeback(), and I see:
> 
> 	....
> 			wb = inode_to_wb(inode);
> 			wb_stat_mod(wb, WB_WRITEBACK, -nr);
> 			__wb_writeout_add(wb, nr);
> 			if (!mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
> 				wb_inode_writeback_end(wb);
> 				if (mapping->host)
> 					sb_clear_inode_writeback(mapping->host);
> 			}
> 	...
> 		lruvec_stat_mod_folio(folio, NR_WRITEBACK, -nr);
> 		zone_stat_mod_folio(folio, NR_ZONE_WRITE_PENDING, -nr);
> 		node_stat_mod_folio(folio, NR_WRITTEN, nr);
> 	...
> 
> And I was thinking those could be done in bulk... but there seems to be an IRQ
> disablement requirement around them.  Does the folio_xor_flags_has_waiters()
> have to be done inside?  I presume this is to prevent a set/clear race on the
> master WRITEBACK tag.

I was envisaging embedding a folio_batch into the ctrl struct and when
that fills up, do the entire batch at once.  That gives us a 31x
reduction in overhead, which is usually enough.

Jan's more of an expert on the writeback path than I am, but once we've
cleared the PG_writeback flag on the folio, there's nothing preventing
us from removing the folio from the pagecache, right?  So the inode could
then be evicted, and then doing mapping->host would be a UAF.

Re: Function to do end-writeback in bulk

[David Howells]

Matthew Wilcox <willy@infradead.org> wrote:

> Jan's more of an expert on the writeback path than I am, but once we've
> cleared the PG_writeback flag on the folio, there's nothing preventing
> us from removing the folio from the pagecache, right?  So the inode could
> then be evicted, and then doing mapping->host would be a UAF.

netfs inode cleanup waits for all outstanding requests to complete, which
would include writeback.

David

Re: Function to do end-writeback in bulk

[Matthew Wilcox]

On Mon, Aug 24, 2026 at 08:14:25PM +0100, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
> 
> > Jan's more of an expert on the writeback path than I am, but once we've
> > cleared the PG_writeback flag on the folio, there's nothing preventing
> > us from removing the folio from the pagecache, right?  So the inode could
> > then be evicted, and then doing mapping->host would be a UAF.
> 
> netfs inode cleanup waits for all outstanding requests to complete, which
> would include writeback.

I don't think core kernel code can depend on that behaviour from all
filesystems that do writeback though.

Re: Function to do end-writeback in bulk

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> 		lruvec_stat_mod_folio(folio, NR_WRITEBACK, -nr);
> 		zone_stat_mod_folio(folio, NR_ZONE_WRITE_PENDING, -nr);
> 		node_stat_mod_folio(folio, NR_WRITTEN, nr);

Actually, those can't necessarily be bulked.  The first is in the folio's
memcg, the second is in the folio's zone and the third in the folio's pgdat.
I don't know how much these can vary between between folios attached to the
same inode.

David