From: Jan Kara <jack@suse.cz> stable inclusion from stable-v6.6.145 commit ac7b2c21f2269d815ad0cdf0f8258d55185b805c category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/16673 CVE: CVE-2026-64378 Reference: https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/... -------------------------------- [ Upstream commit e1b849cfa6b61f1c866a908c9e8dd9b5aaab820b ] There can be multiple inode switch works that are trying to switch inodes to / from the same wb. This can happen in particular if some cgroup exits which owns many (thousands) inodes and we need to switch them all. In this case several inode_switch_wbs_work_fn() instances will be just spinning on the same wb->list_lock while only one of them makes forward progress. This wastes CPU cycles and quickly leads to softlockup reports and unusable system. Instead of running several inode_switch_wbs_work_fn() instances in parallel switching to the same wb and contending on wb->list_lock, run just one work item per wb and manage a queue of isw items switching to this wb. Acked-by: Tejun Heo <tj@kernel.org> Signed-off-by: Jan Kara <jack@suse.cz> Stable-dep-of: cba38ec4cbd3 ("writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()") Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Conflicts: fs/fs-writeback.c include/linux/backing-dev-defs.h mm/backing-dev.c [Fix KABI broken, introduce struct wb_switch_ctx.] Signed-off-by: Zizhi Wo <wozizhi@huawei.com> --- fs/fs-writeback.c | 100 ++++++++++++++++++++----------- include/linux/backing-dev-defs.h | 15 ++++- include/linux/writeback.h | 2 + mm/backing-dev.c | 25 ++++++++ 4 files changed, 105 insertions(+), 37 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 2844e2d87387..81ff3b0acd16 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -342,21 +342,21 @@ static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) spin_lock(&inode->i_lock); return locked_inode_to_wb_and_lock_list(inode); } struct inode_switch_wbs_context { - struct rcu_work work; + /* List of queued switching contexts for the wb */ + struct llist_node list; /* * Multiple inodes can be switched at once. The switching procedure * consists of two parts, separated by a RCU grace period. To make * sure that the second part is executed for each inode gone through * the first part, all inode pointers are placed into a NULL-terminated * array embedded into struct inode_switch_wbs_context. Otherwise * an inode could be left in a non-consistent state. */ - struct bdi_writeback *new_wb; struct inode *inodes[]; }; static void bdi_down_write_wb_switch_rwsem(struct backing_dev_info *bdi) { @@ -461,17 +461,15 @@ static bool inode_do_switch_wbs(struct inode *inode, spin_unlock(&inode->i_lock); return switched; } -static void inode_switch_wbs_work_fn(struct work_struct *work) +static void process_inode_switch_wbs(struct bdi_writeback *new_wb, + struct inode_switch_wbs_context *isw) { - struct inode_switch_wbs_context *isw = - container_of(to_rcu_work(work), struct inode_switch_wbs_context, work); struct backing_dev_info *bdi = inode_to_bdi(isw->inodes[0]); struct bdi_writeback *old_wb = isw->inodes[0]->i_wb; - struct bdi_writeback *new_wb = isw->new_wb; unsigned long nr_switched = 0; struct inode **inodep; /* * If @inode switches cgwb membership while sync_inodes_sb() is @@ -527,10 +525,43 @@ static void inode_switch_wbs_work_fn(struct work_struct *work) wb_put(new_wb); kfree(isw); atomic_dec(&isw_nr_in_flight); } +void inode_switch_wbs_work_fn(struct work_struct *work) +{ + struct wb_switch_ctx *ctx = container_of(work, struct wb_switch_ctx, + switch_work); + struct bdi_writeback *new_wb = ctx->wb; + struct inode_switch_wbs_context *isw, *next_isw; + struct llist_node *list; + + /* + * Grab out reference to wb so that it cannot get freed under us + * after we process all the isw items. + */ + wb_get(new_wb); + while (1) { + list = llist_del_all(&ctx->switch_wbs_ctxs); + /* Nothing to do? */ + if (!list) + break; + /* + * In addition to synchronizing among switchers, I_WB_SWITCH + * tells the RCU protected stat update paths to grab the i_page + * lock so that stat transfer can synchronize against them. + * Let's continue after I_WB_SWITCH is guaranteed to be + * visible. + */ + synchronize_rcu(); + + llist_for_each_entry_safe(isw, next_isw, list, list) + process_inode_switch_wbs(new_wb, isw); + } + wb_put(new_wb); +} + static bool inode_prepare_wbs_switch(struct inode *inode, struct bdi_writeback *new_wb) { /* * Paired with smp_mb() in cgroup_writeback_umount(). @@ -556,10 +587,17 @@ static bool inode_prepare_wbs_switch(struct inode *inode, spin_unlock(&inode->i_lock); return true; } +static void wb_queue_isw(struct bdi_writeback *wb, + struct inode_switch_wbs_context *isw) +{ + if (llist_add(&isw->list, &wb->switch_ctx->switch_wbs_ctxs)) + queue_work(isw_wq, &wb->switch_ctx->switch_work); +} + /** * inode_switch_wbs - change the wb association of an inode * @inode: target inode * @new_wb_id: ID of the new wb * @@ -569,10 +607,11 @@ static bool inode_prepare_wbs_switch(struct inode *inode, static void inode_switch_wbs(struct inode *inode, int new_wb_id) { struct backing_dev_info *bdi = inode_to_bdi(inode); struct cgroup_subsys_state *memcg_css; struct inode_switch_wbs_context *isw; + struct bdi_writeback *new_wb = NULL; /* noop if seems to be already in progress */ if (inode->i_state & I_WB_SWITCH) return; @@ -593,44 +632,38 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id) memcg_css = NULL; rcu_read_unlock(); if (!memcg_css) goto out_free; - isw->new_wb = wb_get_create(bdi, memcg_css, GFP_ATOMIC); + new_wb = wb_get_create(bdi, memcg_css, GFP_ATOMIC); css_put(memcg_css); - if (!isw->new_wb) + if (!new_wb) goto out_free; - if (!inode_prepare_wbs_switch(inode, isw->new_wb)) + if (!inode_prepare_wbs_switch(inode, new_wb)) goto out_free; isw->inodes[0] = inode; - /* - * In addition to synchronizing among switchers, I_WB_SWITCH tells - * the RCU protected stat update paths to grab the i_page - * lock so that stat transfer can synchronize against them. - * Let's continue after I_WB_SWITCH is guaranteed to be visible. - */ - INIT_RCU_WORK(&isw->work, inode_switch_wbs_work_fn); - queue_rcu_work(isw_wq, &isw->work); + wb_queue_isw(new_wb, isw); return; out_free: atomic_dec(&isw_nr_in_flight); - if (isw->new_wb) - wb_put(isw->new_wb); + if (new_wb) + wb_put(new_wb); kfree(isw); } -static bool isw_prepare_wbs_switch(struct inode_switch_wbs_context *isw, +static bool isw_prepare_wbs_switch(struct bdi_writeback *new_wb, + struct inode_switch_wbs_context *isw, struct list_head *list, int *nr) { struct inode *inode; list_for_each_entry(inode, list, i_io_list) { - if (!inode_prepare_wbs_switch(inode, isw->new_wb)) + if (!inode_prepare_wbs_switch(inode, new_wb)) continue; isw->inodes[*nr] = inode; (*nr)++; @@ -650,10 +683,11 @@ static bool isw_prepare_wbs_switch(struct inode_switch_wbs_context *isw, */ bool cleanup_offline_cgwb(struct bdi_writeback *wb) { struct cgroup_subsys_state *memcg_css; struct inode_switch_wbs_context *isw; + struct bdi_writeback *new_wb; int nr; bool restart = false; isw = kzalloc(struct_size(isw, inodes, WB_MAX_INODES_PER_ISW), GFP_KERNEL); @@ -662,16 +696,16 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb) atomic_inc(&isw_nr_in_flight); for (memcg_css = wb->memcg_css->parent; memcg_css; memcg_css = memcg_css->parent) { - isw->new_wb = wb_get_create(wb->bdi, memcg_css, GFP_KERNEL); - if (isw->new_wb) + new_wb = wb_get_create(wb->bdi, memcg_css, GFP_KERNEL); + if (new_wb) break; } - if (unlikely(!isw->new_wb)) - isw->new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */ + if (unlikely(!new_wb)) + new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */ nr = 0; spin_lock(&wb->list_lock); /* * In addition to the inodes that have completed writeback, also switch @@ -679,31 +713,25 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb) * inodes won't be written back for a long time when lazytime is * enabled, and thus pinning the dying cgwbs. It won't break the * bandwidth restrictions, as writeback of inode metadata is not * accounted for. */ - restart = isw_prepare_wbs_switch(isw, &wb->b_attached, &nr); + restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_attached, &nr); if (!restart) - restart = isw_prepare_wbs_switch(isw, &wb->b_dirty_time, &nr); + restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time, + &nr); spin_unlock(&wb->list_lock); /* no attached inodes? bail out */ if (nr == 0) { atomic_dec(&isw_nr_in_flight); - wb_put(isw->new_wb); + wb_put(new_wb); kfree(isw); return restart; } - /* - * In addition to synchronizing among switchers, I_WB_SWITCH tells - * the RCU protected stat update paths to grab the i_page - * lock so that stat transfer can synchronize against them. - * Let's continue after I_WB_SWITCH is guaranteed to be visible. - */ - INIT_RCU_WORK(&isw->work, inode_switch_wbs_work_fn); - queue_rcu_work(isw_wq, &isw->work); + wb_queue_isw(new_wb, isw); return restart; } /** diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h index a6fdd8c0069f..37befb4da053 100644 --- a/include/linux/backing-dev-defs.h +++ b/include/linux/backing-dev-defs.h @@ -103,10 +103,21 @@ struct wb_completion { * that a new wb for the combination can be created. * * Each bdi_writeback that is not embedded into the backing_dev_info must hold * a reference to the parent backing_dev_info. See cgwb_create() for details. */ + +#ifdef CONFIG_CGROUP_WRITEBACK +struct wb_switch_ctx { + struct bdi_writeback *wb; + struct work_struct switch_work; /* work used to perform inode switching + * to this wb */ + struct llist_head switch_wbs_ctxs; /* queued contexts for + * writeback switching */ +}; +#endif + struct bdi_writeback { struct backing_dev_info *bdi; /* our parent bdi */ unsigned long state; /* Always use atomic bitops on this */ unsigned long last_old_flush; /* last old data flush */ @@ -158,13 +169,15 @@ struct bdi_writeback { union { struct work_struct release_work; struct rcu_head rcu; }; -#endif + KABI_USE(1, struct wb_switch_ctx *switch_ctx) +#else KABI_RESERVE(1) +#endif KABI_RESERVE(2) KABI_RESERVE(3) KABI_RESERVE(4) KABI_RESERVE(5) KABI_RESERVE(6) diff --git a/include/linux/writeback.h b/include/linux/writeback.h index 437b4ee42db8..6ea8c781b178 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -287,10 +287,12 @@ static inline void wbc_init_bio(struct writeback_control *wbc, struct bio *bio) */ if (wbc->wb) bio_associate_blkg_from_css(bio, wbc->wb->blkcg_css); } +void inode_switch_wbs_work_fn(struct work_struct *work); + #else /* CONFIG_CGROUP_WRITEBACK */ static inline void inode_attach_wb(struct inode *inode, struct folio *folio) { } diff --git a/mm/backing-dev.c b/mm/backing-dev.c index cfbdeb4b373c..7f51639d52bc 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -514,10 +514,11 @@ static void cgwb_free_rcu(struct rcu_head *rcu_head) { struct bdi_writeback *wb = container_of(rcu_head, struct bdi_writeback, rcu); percpu_ref_exit(&wb->refcnt); + kfree(wb->switch_ctx); kfree(wb); } static void cgwb_release_workfn(struct work_struct *work) { @@ -543,10 +544,11 @@ static void cgwb_release_workfn(struct work_struct *work) spin_unlock_irq(&cgwb_lock); wb_exit(wb); bdi_put(bdi); WARN_ON_ONCE(!list_empty(&wb->b_attached)); + WARN_ON_ONCE(work_pending(&wb->switch_ctx->switch_work)); call_rcu(&wb->rcu, cgwb_free_rcu); } static void cgwb_release(struct percpu_ref *refcnt) { @@ -649,10 +651,18 @@ static int cgwb_create(struct backing_dev_info *bdi, wb->blkcg_css = blkcg_css; INIT_LIST_HEAD(&wb->b_attached); INIT_WORK(&wb->release_work, cgwb_release_workfn); set_bit(WB_registered, &wb->state); bdi_get(bdi); + wb->switch_ctx = kzalloc(sizeof(struct wb_switch_ctx), gfp); + if (!wb->switch_ctx) { + ret = -ENOMEM; + goto err_fprop_exit; + } + wb->switch_ctx->wb = wb; + INIT_WORK(&wb->switch_ctx->switch_work, inode_switch_wbs_work_fn); + init_llist_head(&wb->switch_ctx->switch_wbs_ctxs); /* * The root wb determines the registered state of the whole bdi and * memcg_cgwb_list and blkcg_cgwb_list's next pointers indicate * whether they're still online. Don't link @wb if any is dead. @@ -680,10 +690,11 @@ static int cgwb_create(struct backing_dev_info *bdi, goto err_fprop_exit; } goto out_put; err_fprop_exit: + kfree(wb->switch_ctx); bdi_put(bdi); fprop_local_destroy_percpu(&wb->memcg_completions); err_ref_exit: percpu_ref_exit(&wb->refcnt); err_wb_exit: @@ -781,10 +792,21 @@ static int cgwb_bdi_init(struct backing_dev_info *bdi) ret = wb_init(&bdi->wb, bdi, GFP_KERNEL); if (!ret) { bdi->wb.memcg_css = &root_mem_cgroup->css; bdi->wb.blkcg_css = blkcg_root_css; + bdi->wb.switch_ctx = kzalloc(sizeof(struct wb_switch_ctx), + GFP_KERNEL); + if (!bdi->wb.switch_ctx) { + wb_exit(&bdi->wb); + ret = -ENOMEM; + } else { + bdi->wb.switch_ctx->wb = &bdi->wb; + INIT_WORK(&bdi->wb.switch_ctx->switch_work, + inode_switch_wbs_work_fn); + init_llist_head(&bdi->wb.switch_ctx->switch_wbs_ctxs); + } } return ret; } static void cgwb_bdi_unregister(struct backing_dev_info *bdi) @@ -1127,10 +1149,13 @@ static void release_bdi(struct kref *ref) struct backing_dev_info *bdi = container_of(ref, struct backing_dev_info, refcnt); WARN_ON_ONCE(test_bit(WB_registered, &bdi->wb.state)); WARN_ON_ONCE(bdi->dev); +#ifdef CONFIG_CGROUP_WRITEBACK + kfree(bdi->wb.switch_ctx); +#endif wb_exit(&bdi->wb); kfree(bdi); } void bdi_put(struct backing_dev_info *bdi) -- 2.52.0