Re: [PATCH] mm/page-writeback: avoid unnecessary strictlimit background wakeups

[Jan Kara]

On Sat 31-10-26 09:05:15, Avi Weiss wrote:
> balance_dirty_pages() includes pages under writeback in
> gdtc->wb_dirty when checking the per-wb background threshold for
> strictlimit BDIs. However, wb_over_bg_thresh() only considers
> reclaimable dirty pages when deciding whether to start more
> background writeback.
> 
> This can repeatedly wake the background worker even when there are
> too few dirty pages to write. In a 15-second FUSE writeback test,
> background wakeup requests dropped from 2657 to 682 (74% drop),
> while writeback_start events remained nearly unchanged at 99 and 98.
> 
> Check wb_over_bg_thresh() before starting background writeback through
> the strictlimit condition. Leave the global background threshold
> check and the wakeup required for dirty throttling unchanged.

I assume you've left the gdtc->wb_dirty > gdtc->wb_bg_thresh check in to
reduce times when wb_over_bg_thresh() is called (because it is relatively
costly and this condition is evaluated relatively often)? Maybe it would be
good to add there a comment with explanation like:

/*
 * gdtc->wb_dirty includes pages under writeback so it serves only as a
 * quick check and we use wb_over_bg_thresh() for exact evaluation whether
 * a wakeup is needed
 */

> Fixes: 0437906841d0 ("mm: start background writeback based on per-wb threshold for strictlimit BDIs")
> Signed-off-by: Avi Weiss <thnkslprpt@xxxxxxxxx>

Otherwise the patch looks good so feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  mm/page-writeback.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 499a35473e4f..cb0c58715b93 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1826,7 +1826,8 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
>  
>  		if (!writeback_in_progress(wb) &&
>  		    (nr_dirty > gdtc->bg_thresh ||
> -		     (strictlimit && gdtc->wb_dirty > gdtc->wb_bg_thresh)))
> +		     (strictlimit && gdtc->wb_dirty > gdtc->wb_bg_thresh &&
> +		      wb_over_bg_thresh(wb))))
>  			wb_start_background_writeback(wb);
>  
>  		/*
> 
> base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR