Re: [PATCH 2/7] fs/writeback: bail out if there is no more inodes for IO and queued once

From: Tim Chen
Date: Thu Feb 08 2024 - 14:22:50 EST


On Fri, 2024-02-09 at 01:20 +0800, Kemeng Shi wrote:
> For case there is no more inodes for IO in io list from last wb_writeback,
> We may bail out early even there is inode in dirty list should be written
> back. Only bail out when we queued once to avoid missing dirtied inode.
>
> This is from code reading...
>
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> ---
> fs/fs-writeback.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index a9a918972719..edb0cff51673 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -2086,6 +2086,7 @@ static long wb_writeback(struct bdi_writeback *wb,
> struct inode *inode;
> long progress;
> struct blk_plug plug;
> + bool queued = false;
>
> if (work->for_kupdate)
> filter_expired_io(wb);
> @@ -2131,8 +2132,10 @@ static long wb_writeback(struct bdi_writeback *wb,
> dirtied_before = jiffies;
>
> trace_writeback_start(wb, work);
> - if (list_empty(&wb->b_io))
> + if (list_empty(&wb->b_io)) {
> queue_io(wb, work, dirtied_before);
> + queued = true;
> + }
> if (work->sb)
> progress = writeback_sb_inodes(work->sb, wb, work);
> else
> @@ -2155,7 +2158,7 @@ static long wb_writeback(struct bdi_writeback *wb,
> /*
> * No more inodes for IO, bail
> */
> - if (list_empty(&wb->b_more_io)) {
> + if (list_empty(&wb->b_more_io) && queued) {

Wonder if we can simply do
if (list_empty(&wb->b_more_io) && list_empty(&wb->b_io)) {

if the intention is to not bail if there are still inodes to be be flushed.

Tim

> spin_unlock(&wb->list_lock);
> break;
> }