From: Tejun Heo <tj@kernel.org>
To: Liz Fong-Jones <lizf@honeycomb.io>
Cc: Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Jens Axboe <axboe@kernel.dk>,
Andrew Morton <akpm@linux-foundation.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Roman Gushchin <roman.gushchin@linux.dev>,
Shakeel Butt <shakeel.butt@linux.dev>,
Xin Yin <yinxin.x@bytedance.com>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
ian@honeycomb.io
Subject: Re: [PATCH v4 2/2] writeback: switch a replaced cgwb's inodes to its successor
Date: Thu, 01 Oct 2026 08:43:42 -1000 [thread overview]
Message-ID: <ef728276c74f15e11f42301b4e6f011f@kernel.org> (raw)
In-Reply-To: <20260930-wb-dying-cgwb-flush-v4-2-bde637803a96@honeycomb.io>
Hello, Liz.
On Wed, Sep 30, 2026 at 07:35:37PM +0000, Liz Fong-Jones wrote:
> + new_wb = wb_get_lookup(wb->bdi, wb->memcg_css);
> + if (!new_wb)
> + return false;
If the successor was already released, the slot is empty and the inodes
stay on a wb that foreign flushes can't find. Maybe fall back to
&wb->bdi->wb like cleanup_offline_cgwb() does?
> + spin_lock(&wb->list_lock);
> + restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_attached, &nr) ||
> + isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty, &nr) ||
> + isw_prepare_wbs_switch(new_wb, isw, &wb->b_io, &nr) ||
> + isw_prepare_wbs_switch(new_wb, isw, &wb->b_more_io, &nr) ||
> + isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time, &nr);
> + spin_unlock(&wb->list_lock);
Switching a dirty inode away leaves WB_has_dirty_io set on the old wb.
inode_do_switch_wbs() moves it onto new_wb->b_dirty through
inode_io_list_move_locked(), which only updates new_wb, and nothing calls
wb_io_lists_depopulated() on old_wb. A live wb clears it on its next dirty
to clean transition, but a replaced wb never gets another inode, so it's
freed with its avg_write_bandwidth still in bdi->tot_write_bandwidth,
which wb_split_bdi_pages() and wb_min_max_ratio() divide by. Can you add a
prep patch which calls wb_io_lists_depopulated(old_wb) after the switch
loop in process_inode_switch_wbs(), while old_wb->list_lock is still held?
> + while (switch_replaced_cgwb(wb))
> + cond_resched();
cond_resched() is a no-op on PREEMPTION kernels, so a wb with a lot of
inodes keeps this worker from reporting a Tasks-RCU quiescent state. See
407a5d205179 ("writeback: report a Tasks-RCU quiescent state per cgwb
drain pass"). Can you use the same do { } while () shape with
cond_resched_tasks_rcu_qs() as cleanup_offline_cgwbs_workfn()?
Thanks.
--
tejun
prev parent reply other threads:[~2026-10-01 18:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-30 19:35 [PATCH v4 0/2] writeback: let foreign flushes reach dying cgwbs Liz Fong-Jones
2026-09-30 19:35 ` [PATCH v4 1/2] " Liz Fong-Jones
2026-10-01 18:43 ` Tejun Heo
2026-09-30 19:35 ` [PATCH v4 2/2] writeback: switch a replaced cgwb's inodes to its successor Liz Fong-Jones
2026-10-01 18:43 ` Tejun Heo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ef728276c74f15e11f42301b4e6f011f@kernel.org \
--to=tj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=ian@honeycomb.io \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lizf@honeycomb.io \
--cc=roman.gushchin@linux.dev \
--cc=shakeel.butt@linux.dev \
--cc=viro@zeniv.linux.org.uk \
--cc=yinxin.x@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.