on 2/9/2024 3:21 AM, Tim Chen wrote:
> 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.
I suppose not as there may be inodes in wb->b_dirty should be flushed.
For case that there is a inode in wb->b_io which is not flushed in last
wb_writeback and there are a lot of inodes in wb->dirty, the next background
flush is supposed to make dirty pages under threshold however only the inode
in wb->b_io is flushed.
>
> Tim
>
>> spin_unlock(&wb->list_lock);
>> break;
>> }
>
