Re: [f2fs-dev] [PATCH] f2fs: cache: wake up the writeback thread while checkpoint is disabled

[Chao Yu <chao@kernel.org>]

On 10/11/26 23:34, Daeho Jeong wrote:
> On Sun, Oct 11, 2026 at 1:22 AM Chao Yu <chao@kernel.org> wrote:
>>
>> On 10/8/26 03:31, Daeho Jeong wrote:
>>> From: Daeho Jeong <daehojeong@google.com>
>>>
>>> Dirty node and meta caches are written back by the cache writeback
>>> thread and by checkpoint. The thread wakes up every cache_wb_interval
>>> (5 seconds by default) and writes at most 512 node caches and one
>>> contiguous run of meta caches. Checkpoint, which f2fs_balance_fs_bg()
>>> triggers when there are too many dirty caches, writes the rest.
>>>
>>> While checkpoint is disabled, checkpoint does not run: f2fs_sync_fs()
>>> returns early and f2fs_balance_fs() returns before doing anything. So
>>> dirty node caches pile up without a limit. They are kmalloc'd, and the
>>> shrinker skips dirty entries, so this memory cannot be reclaimed. When
>>> node and meta blocks were in the page cache, the dirty pages were
>>> counted as dirty page cache, and the flusher threads wrote them back in
>>> the background.
>>>
>>> A checkpoint=disable test that remounts the filesystem with
>>> checkpoint=disable and runs fsstress -p 32 for 300 seconds left about
>>> 160K dirty node caches (2.7GB of kmalloc-16k) with 16KB blocks and
>>> about 226K with 4KB blocks in QEMU, and drop_caches freed none of them.
>>>
>>> While checkpoint is disabled, wake up the thread from
>>> f2fs_mark_cache_dirty() once there are as many dirty node or meta
>>> caches as f2fs_write_node_caches() and f2fs_write_meta_caches() collect
>>> before writing, and let the thread go on without waiting while that is
>>> still true and each round writes some of them. Each round writes the
>>> same number of caches as before. Split nr_caches_to_collect() out of
>>> nr_caches_to_skip() to share these numbers without the dirty_exceeded
>>> check.
>>>
>>> With this, the same test keeps about 4K dirty node caches with both
>>> 4KB and 16KB blocks. Nothing changes while checkpoint is enabled.
>>
>> I've sent a patch to wake up f2fs_writeback to flush dirty data according
>> to configurable sysfs value, I guess it can fix your checkpoint disable
>> case, please take a look.
>>
>> [PATCH 3/5] f2fs: cache: wake up f2fs_writeback when exceeding threshold
>>
>> Thanks,
> 
> Hi Chao,
> 
> Thanks, I think it covers my case.

Hi Daeho,

> 
> Two things in v2:
> 
> 1. The wait condition stays true when the thread can't reduce the dirty
>     count, so it loops without sleeping: on f2fs_cp_error(), readonly,
>     freezing(current) or a failed sb_start_write_trylock() (dirty caches
>     stay after shutdown/freeze with checkpoint=disable, and during
>     suspend the thread may never freeze), or when the threshold is below
>     nr_caches_to_skip() so the write functions write nothing. How about
>     sleeping for the interval when a round doesn't reduce the dirty
>     count?

Yeah, I see, I figured out a throttle field to avoid unnecessary wakeup in v3.

> 
> 2. With both thresholds 0, dirty caches grow again under
>     checkpoint=disable. Should that case wake the thread anyway?

Fixed to not wake up kthread for this case in v3.

> 
> I'm fine to drop my patch if you take this in your series.

Thanks a lot for your review, I prefer to keep the one in my series, I feel
it will be more flexible to control writeback via the new sysfs nodes.

> 
> Thanks,
> Daeho
> 
>>
>>>
>>> Fixes: 7a1cf2a76b71 ("f2fs: cache: use meta cache")
>>> Fixes: 493b9dc8b52c ("f2fs: cache: use node cache")
>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>> ---
>>>    fs/f2fs/cache.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++--
>>>    fs/f2fs/cache.h   |  1 +
>>>    fs/f2fs/segment.h | 13 +++++++----
>>>    3 files changed, 67 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/cache.c b/fs/f2fs/cache.c
>>> index 04b5408cbc5..6b6f49fae25 100644
>>> --- a/fs/f2fs/cache.c
>>> +++ b/fs/f2fs/cache.c
>>> @@ -61,6 +61,42 @@ void f2fs_cache_update_tag(struct f2fs_cached_block *entry,
>>>        spin_unlock_irqrestore(&cache->tree_lock, flags);
>>>    }
>>>
>>> +/*
>>> + * While checkpoint is disabled, checkpoint does not write back dirty node and
>>> + * meta caches, and the writeback thread is the only one that writes them. Tell
>>> + * whether there are as many dirty caches as f2fs_write_node_caches() and
>>> + * f2fs_write_meta_caches() collect before writing, so that the thread should
>>> + * write them now instead of every cache_wb_interval.
>>> + */
>>> +static bool f2fs_cache_wb_needed(struct f2fs_sb_info *sbi)
>>> +{
>>> +     if (likely(!is_sbi_flag_set(sbi, SBI_CP_DISABLED)))
>>> +             return false;
>>> +
>>> +     return get_nr_caches(sbi, F2FS_DIRTY_NODES) >=
>>> +                             nr_caches_to_collect(sbi, NODE) ||
>>> +             get_nr_caches(sbi, F2FS_DIRTY_META) >=
>>> +                             nr_caches_to_collect(sbi, META);
>>> +}
>>> +
>>> +static void f2fs_wake_cache_wb_thread(struct f2fs_sb_info *sbi)
>>> +{
>>> +     struct f2fs_cache_kthread *cache_thread = &sbi->cache_thread;
>>> +
>>> +     if (!f2fs_cache_wb_needed(sbi))
>>> +             return;
>>> +
>>> +     /* pairs with smp_store_release() in f2fs_start_cache_wb_thread() */
>>> +     if (!smp_load_acquire(&cache_thread->cache_wb_task))
>>> +             return;
>>> +
>>> +     if (READ_ONCE(cache_thread->cache_wb_urgent))
>>> +             return;
>>> +
>>> +     WRITE_ONCE(cache_thread->cache_wb_urgent, true);
>>> +     wake_up(&cache_thread->cache_wb_wq);
>>> +}
>>> +
>>>    bool f2fs_mark_cache_dirty(struct f2fs_cached_block *entry)
>>>    {
>>>        struct f2fs_cached_block_list *cache = entry->cache;
>>> @@ -81,6 +117,7 @@ bool f2fs_mark_cache_dirty(struct f2fs_cached_block *entry)
>>>                f2fs_cache_update_tag(entry, F2FS_CACHE_TAG_NONE,
>>>                                                F2FS_CACHE_TAG_DIRTY);
>>>                inc_cache_count(cache->sbi, type);
>>> +             f2fs_wake_cache_wb_thread(cache->sbi);
>>>                return true;
>>>        }
>>>
>>> @@ -679,10 +716,13 @@ static int f2fs_cache_writeback_kthread(void *data)
>>>
>>>        while (!kthread_should_stop()) {
>>>                unsigned int interval = cache_thread->cache_wb_interval;
>>> +             s64 nr_dirty;
>>>
>>>                wait_event_freezable_timeout(*wq,
>>> -                             kthread_should_stop(),
>>> +                             kthread_should_stop() ||
>>> +                             READ_ONCE(cache_thread->cache_wb_urgent),
>>>                                msecs_to_jiffies(interval));
>>> +             WRITE_ONCE(cache_thread->cache_wb_urgent, false);
>>>
>>>                if (kthread_should_stop())
>>>                        break;
>>> @@ -699,10 +739,23 @@ static int f2fs_cache_writeback_kthread(void *data)
>>>                if (!sb_start_write_trylock(sbi->sb))
>>>                        continue;
>>>
>>> +             nr_dirty = get_nr_caches(sbi, F2FS_DIRTY_NODES) +
>>> +                             get_nr_caches(sbi, F2FS_DIRTY_META);
>>> +
>>>                f2fs_write_meta_caches(sbi);
>>>                f2fs_write_node_caches(sbi);
>>>
>>>                sb_end_write(sbi->sb);
>>> +
>>> +             /*
>>> +              * Each round writes a limited number of caches. Go on without
>>> +              * waiting while there are still many dirty caches, as long as
>>> +              * this round wrote some of them.
>>> +              */
>>> +             if (f2fs_cache_wb_needed(sbi) &&
>>> +                     get_nr_caches(sbi, F2FS_DIRTY_NODES) +
>>> +                     get_nr_caches(sbi, F2FS_DIRTY_META) < nr_dirty)
>>> +                     WRITE_ONCE(cache_thread->cache_wb_urgent, true);
>>>        }
>>>        return 0;
>>>    }
>>> @@ -719,6 +772,7 @@ int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi)
>>>
>>>        init_waitqueue_head(&cache_thread->cache_wb_wq);
>>>        cache_thread->cache_wb_interval = DEF_DIRTY_CACHE_TIMEOUT;
>>> +     cache_thread->cache_wb_urgent = false;
>>>        snprintf(name, sizeof(name), "f2fs_writeback-%u:%u",
>>>                        MAJOR(dev), MINOR(dev));
>>>
>>> @@ -726,7 +780,8 @@ int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi)
>>>        if (IS_ERR(task))
>>>                return PTR_ERR(task);
>>>
>>> -     cache_thread->cache_wb_task = task;
>>> +     /* pairs with smp_load_acquire() in f2fs_wake_cache_wb_thread() */
>>> +     smp_store_release(&cache_thread->cache_wb_task, task);
>>>        return 0;
>>>    }
>>>
>>> diff --git a/fs/f2fs/cache.h b/fs/f2fs/cache.h
>>> index 6c4db910d76..1d0216d2128 100644
>>> --- a/fs/f2fs/cache.h
>>> +++ b/fs/f2fs/cache.h
>>> @@ -232,6 +232,7 @@ struct f2fs_cache_kthread {
>>>        struct task_struct *cache_wb_task;
>>>        wait_queue_head_t cache_wb_wq;
>>>        unsigned int cache_wb_interval;
>>> +     bool cache_wb_urgent;           /* write back without waiting */
>>>    };
>>>
>>>    int f2fs_start_cache_wb_thread(struct f2fs_sb_info *sbi);
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index 526764ba31a..58804e9cc29 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -983,11 +983,8 @@ static inline bool sec_usage_check(struct f2fs_sb_info *sbi, unsigned int secno)
>>>     * 512 blocks (2MB) * 8 for nodes, and
>>>     * 256 blocks * 8 for meta are set.
>>>     */
>>> -static inline int nr_caches_to_skip(struct f2fs_sb_info *sbi, int type)
>>> +static inline int nr_caches_to_collect(struct f2fs_sb_info *sbi, int type)
>>>    {
>>> -     if (bdi_wb_dirty_exceeded(sbi->sb->s_bdi))
>>> -             return 0;
>>> -
>>>        if (type == DATA)
>>>                return BLKS_PER_SEG(sbi);
>>>        else if (type == NODE)
>>> @@ -998,6 +995,14 @@ static inline int nr_caches_to_skip(struct f2fs_sb_info *sbi, int type)
>>>                return 0;
>>>    }
>>>
>>> +static inline int nr_caches_to_skip(struct f2fs_sb_info *sbi, int type)
>>> +{
>>> +     if (bdi_wb_dirty_exceeded(sbi->sb->s_bdi))
>>> +             return 0;
>>> +
>>> +     return nr_caches_to_collect(sbi, type);
>>> +}
>>> +
>>>    /*
>>>     * When writing cache asynchronously, align nr_to_write to BIO_MAX_VECS.
>>>     */
>>