* [PATCH AUTOSEL 6.12 059/486] cgroup/rstat: avoid disabling irqs for O(num_cpu) [not found] <20250505223922.2682012-1-sashal@kernel.org> @ 2025-05-05 22:32 ` Sasha Levin 2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 081/486] blk-cgroup: improve policy registration error handling Sasha Levin ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Sasha Levin @ 2025-05-05 22:32 UTC (permalink / raw) To: linux-kernel, stable Cc: Eric Dumazet, Greg Thelen, Michal Koutný, Yosry Ahmed, Tejun Heo, Sasha Levin, hannes, cgroups From: Eric Dumazet <edumazet@google.com> [ Upstream commit 0efc297a3c4974dbd609ee36fc6345720b6ca735 ] cgroup_rstat_flush_locked() grabs the irq safe cgroup_rstat_lock while iterating all possible cpus. It only drops the lock if there is scheduler or spin lock contention. If neither, then interrupts can be disabled for a long time. On large machines this can disable interrupts for a long enough time to drop network packets. On 400+ CPU machines I've seen interrupt disabled for over 40 msec. Prevent rstat from disabling interrupts while processing all possible cpus. Instead drop and reacquire cgroup_rstat_lock for each cpu. This approach was previously discussed in https://lore.kernel.org/lkml/ZBz%2FV5a7%2F6PZeM7S@slm.duckdns.org/, though this was in the context of an non-irq rstat spin lock. Benchmark this change with: 1) a single stat_reader process with 400 threads, each reading a test memcg's memory.stat repeatedly for 10 seconds. 2) 400 memory hog processes running in the test memcg and repeatedly charging memory until oom killed. Then they repeat charging and oom killing. v6.14-rc6 with CONFIG_IRQSOFF_TRACER with stat_reader and hogs, finds interrupts are disabled by rstat for 45341 usec: # => started at: _raw_spin_lock_irq # => ended at: cgroup_rstat_flush # # # _------=> CPU# # / _-----=> irqs-off/BH-disabled # | / _----=> need-resched # || / _---=> hardirq/softirq # ||| / _--=> preempt-depth # |||| / _-=> migrate-disable # ||||| / delay # cmd pid |||||| time | caller # \ / |||||| \ | / stat_rea-96532 52d.... 0us*: _raw_spin_lock_irq stat_rea-96532 52d.... 45342us : cgroup_rstat_flush stat_rea-96532 52d.... 45342us : tracer_hardirqs_on <-cgroup_rstat_flush stat_rea-96532 52d.... 45343us : <stack trace> => memcg1_stat_format => memory_stat_format => memory_stat_show => seq_read_iter => vfs_read => ksys_read => do_syscall_64 => entry_SYSCALL_64_after_hwframe With this patch the CONFIG_IRQSOFF_TRACER doesn't find rstat to be the longest holder. The longest irqs-off holder has irqs disabled for 4142 usec, a huge reduction from previous 45341 usec rstat finding. Running stat_reader memory.stat reader for 10 seconds: - without memory hogs: 9.84M accesses => 12.7M accesses - with memory hogs: 9.46M accesses => 11.1M accesses The throughput of memory.stat access improves. The mode of memory.stat access latency after grouping by of 2 buckets: - without memory hogs: 64 usec => 16 usec - with memory hogs: 64 usec => 8 usec The memory.stat latency improves. Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Greg Thelen <gthelen@google.com> Tested-by: Greg Thelen <gthelen@google.com> Acked-by: Michal Koutný <mkoutny@suse.com> Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev> Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- kernel/cgroup/rstat.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c index 3e01781aeb7bd..c4ce2f5a9745f 100644 --- a/kernel/cgroup/rstat.c +++ b/kernel/cgroup/rstat.c @@ -323,13 +323,11 @@ static void cgroup_rstat_flush_locked(struct cgroup *cgrp) rcu_read_unlock(); } - /* play nice and yield if necessary */ - if (need_resched() || spin_needbreak(&cgroup_rstat_lock)) { - __cgroup_rstat_unlock(cgrp, cpu); - if (!cond_resched()) - cpu_relax(); - __cgroup_rstat_lock(cgrp, cpu); - } + /* play nice and avoid disabling interrupts for a long time */ + __cgroup_rstat_unlock(cgrp, cpu); + if (!cond_resched()) + cpu_relax(); + __cgroup_rstat_lock(cgrp, cpu); } } -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH AUTOSEL 6.12 081/486] blk-cgroup: improve policy registration error handling [not found] <20250505223922.2682012-1-sashal@kernel.org> 2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 059/486] cgroup/rstat: avoid disabling irqs for O(num_cpu) Sasha Levin @ 2025-05-05 22:32 ` Sasha Levin 2025-05-05 22:34 ` [PATCH AUTOSEL 6.12 194/486] blk-throttle: don't take carryover for prioritized processing of metadata Sasha Levin 2025-05-05 22:39 ` [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent Sasha Levin 3 siblings, 0 replies; 6+ messages in thread From: Sasha Levin @ 2025-05-05 22:32 UTC (permalink / raw) To: linux-kernel, stable Cc: Chen Linxuan, Wen Tao, Michal Koutný, Tejun Heo, Yu Kuai, Jens Axboe, Sasha Levin, josef, cgroups, linux-block From: Chen Linxuan <chenlinxuan@uniontech.com> [ Upstream commit e1a0202c6bfda24002a3ae2115154fa90104c649 ] This patch improve the returned error code of blkcg_policy_register(). 1. Move the validation check for cpd/pd_alloc_fn and cpd/pd_free_fn function pairs to the start of blkcg_policy_register(). This ensures we immediately return -EINVAL if the function pairs are not correctly provided, rather than returning -ENOSPC after locking and unlocking mutexes unnecessarily. Those locks should not contention any problems, as error of policy registration is a super cold path. 2. Return -ENOMEM when cpd_alloc_fn() failed. Co-authored-by: Wen Tao <wentao@uniontech.com> Signed-off-by: Wen Tao <wentao@uniontech.com> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com> Reviewed-by: Michal Koutný <mkoutny@suse.com> Acked-by: Tejun Heo <tj@kernel.org> Reviewed-by: Yu Kuai <yukuai3@huawei.com> Link: https://lore.kernel.org/r/3E333A73B6B6DFC0+20250317022924.150907-1-chenlinxuan@uniontech.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org> --- block/blk-cgroup.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index f1cf7f2909f3a..643d6bf66522e 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1725,27 +1725,27 @@ int blkcg_policy_register(struct blkcg_policy *pol) struct blkcg *blkcg; int i, ret; + /* + * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy + * without pd_alloc_fn/pd_free_fn can't be activated. + */ + if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || + (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) + return -EINVAL; + mutex_lock(&blkcg_pol_register_mutex); mutex_lock(&blkcg_pol_mutex); /* find an empty slot */ - ret = -ENOSPC; for (i = 0; i < BLKCG_MAX_POLS; i++) if (!blkcg_policy[i]) break; if (i >= BLKCG_MAX_POLS) { pr_warn("blkcg_policy_register: BLKCG_MAX_POLS too small\n"); + ret = -ENOSPC; goto err_unlock; } - /* - * Make sure cpd/pd_alloc_fn and cpd/pd_free_fn in pairs, and policy - * without pd_alloc_fn/pd_free_fn can't be activated. - */ - if ((!pol->cpd_alloc_fn ^ !pol->cpd_free_fn) || - (!pol->pd_alloc_fn ^ !pol->pd_free_fn)) - goto err_unlock; - /* register @pol */ pol->plid = i; blkcg_policy[pol->plid] = pol; @@ -1756,8 +1756,10 @@ int blkcg_policy_register(struct blkcg_policy *pol) struct blkcg_policy_data *cpd; cpd = pol->cpd_alloc_fn(GFP_KERNEL); - if (!cpd) + if (!cpd) { + ret = -ENOMEM; goto err_free_cpds; + } blkcg->cpd[pol->plid] = cpd; cpd->blkcg = blkcg; -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH AUTOSEL 6.12 194/486] blk-throttle: don't take carryover for prioritized processing of metadata [not found] <20250505223922.2682012-1-sashal@kernel.org> 2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 059/486] cgroup/rstat: avoid disabling irqs for O(num_cpu) Sasha Levin 2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 081/486] blk-cgroup: improve policy registration error handling Sasha Levin @ 2025-05-05 22:34 ` Sasha Levin 2025-05-05 22:39 ` [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent Sasha Levin 3 siblings, 0 replies; 6+ messages in thread From: Sasha Levin @ 2025-05-05 22:34 UTC (permalink / raw) To: linux-kernel, stable Cc: Ming Lei, Tejun Heo, Josef Bacik, Yu Kuai, Jens Axboe, Sasha Levin, cgroups, linux-block From: Ming Lei <ming.lei@redhat.com> [ Upstream commit a9fc8868b350cbf4ff730a4ea9651319cc669516 ] Commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata") takes bytes/ios carryover for prioritized processing of metadata. Turns out we can support it by charging it directly without trimming slice, and the result is same with carryover. Cc: Tejun Heo <tj@kernel.org> Cc: Josef Bacik <josef@toxicpanda.com> Cc: Yu Kuai <yukuai3@huawei.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> Acked-by: Tejun Heo <tj@kernel.org> Link: https://lore.kernel.org/r/20250305043123.3938491-3-ming.lei@redhat.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org> --- block/blk-throttle.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 2c4192e12efab..6b82fcbd7e774 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1593,13 +1593,6 @@ static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw) return tg_may_dispatch(tg, bio, NULL); } -static void tg_dispatch_in_debt(struct throtl_grp *tg, struct bio *bio, bool rw) -{ - if (!bio_flagged(bio, BIO_BPS_THROTTLED)) - tg->carryover_bytes[rw] -= throtl_bio_data_size(bio); - tg->carryover_ios[rw]--; -} - bool __blk_throtl_bio(struct bio *bio) { struct request_queue *q = bdev_get_queue(bio->bi_bdev); @@ -1636,10 +1629,12 @@ bool __blk_throtl_bio(struct bio *bio) /* * IOs which may cause priority inversions are * dispatched directly, even if they're over limit. - * Debts are handled by carryover_bytes/ios while - * calculating wait time. + * + * Charge and dispatch directly, and our throttle + * control algorithm is adaptive, and extra IO bytes + * will be throttled for paying the debt */ - tg_dispatch_in_debt(tg, bio, rw); + throtl_charge_bio(tg, bio); } else { /* if above limits, break to queue */ break; -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent. [not found] <20250505223922.2682012-1-sashal@kernel.org> ` (2 preceding siblings ...) 2025-05-05 22:34 ` [PATCH AUTOSEL 6.12 194/486] blk-throttle: don't take carryover for prioritized processing of metadata Sasha Levin @ 2025-05-05 22:39 ` Sasha Levin 2026-11-13 12:30 ` Christian Loehle 3 siblings, 1 reply; 6+ messages in thread From: Sasha Levin @ 2025-05-05 22:39 UTC (permalink / raw) To: linux-kernel, stable Cc: Sebastian Andrzej Siewior, Tejun Heo, Yonghong Song, Greg Kroah-Hartman, Sasha Levin, reinette.chatre, tglx, mingo, bp, dave.hansen, x86, rafael, dakr, hannes, mkoutny, ast, daniel, andrii, eddyz87, shuah, jolsa, alan.maguire, cgroups, bpf, linux-kselftest From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> [ Upstream commit 633488947ef66b194377411322dc9e12aab79b65 ] kernfs_rename_lock is used to obtain stable kernfs_node::{name|parent} pointer. This is a preparation to access kernfs_node::parent under RCU and ensure that the pointer remains stable under the RCU lifetime guarantees. For a complete path, as it is done in kernfs_path_from_node(), the kernfs_rename_lock is still required in order to obtain a stable parent relationship while computing the relevant node depth. This must not change while the nodes are inspected in order to build the path. If the kernfs user never moves the nodes (changes the parent) then the kernfs_rename_lock is not required and the RCU guarantees are sufficient. This "restriction" can be set with KERNFS_ROOT_INVARIANT_PARENT. Otherwise the lock is required. Rename kernfs_node::parent to kernfs_node::__parent to denote the RCU access and use RCU accessor while accessing the node. Make cgroup use KERNFS_ROOT_INVARIANT_PARENT since the parent here can not change. Acked-by: Tejun Heo <tj@kernel.org> Cc: Yonghong Song <yonghong.song@linux.dev> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Link: https://lore.kernel.org/r/20250213145023.2820193-6-bigeasy@linutronix.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 65 +++++++++---- fs/kernfs/dir.c | 96 ++++++++++++------- fs/kernfs/kernfs-internal.h | 32 ++++++- fs/kernfs/mount.c | 10 +- fs/kernfs/symlink.c | 23 ++--- fs/sysfs/file.c | 24 +++-- include/linux/kernfs.h | 10 +- kernel/cgroup/cgroup-v1.c | 2 +- kernel/cgroup/cgroup.c | 24 ++++- .../selftests/bpf/progs/profiler.inc.h | 2 +- 10 files changed, 195 insertions(+), 93 deletions(-) diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 2d48db66fca85..d4d7ccf1253b8 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -948,10 +948,20 @@ static int rdt_last_cmd_status_show(struct kernfs_open_file *of, return 0; } +static void *rdt_kn_parent_priv(struct kernfs_node *kn) +{ + /* + * The parent pointer is only valid within RCU section since it can be + * replaced. + */ + guard(rcu)(); + return rcu_dereference(kn->__parent)->priv; +} + static int rdt_num_closids_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); seq_printf(seq, "%u\n", s->num_closid); return 0; @@ -960,7 +970,7 @@ static int rdt_num_closids_show(struct kernfs_open_file *of, static int rdt_default_ctrl_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%x\n", r->default_ctrl); @@ -970,7 +980,7 @@ static int rdt_default_ctrl_show(struct kernfs_open_file *of, static int rdt_min_cbm_bits_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%u\n", r->cache.min_cbm_bits); @@ -980,7 +990,7 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file *of, static int rdt_shareable_bits_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%x\n", r->cache.shareable_bits); @@ -1004,7 +1014,7 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of, static int rdt_bit_usage_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); /* * Use unsigned long even though only 32 bits are used to ensure * test_bit() is used safely. @@ -1086,7 +1096,7 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of, static int rdt_min_bw_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%u\n", r->membw.min_bw); @@ -1096,7 +1106,7 @@ static int rdt_min_bw_show(struct kernfs_open_file *of, static int rdt_num_rmids_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); seq_printf(seq, "%d\n", r->num_rmid); @@ -1106,7 +1116,7 @@ static int rdt_num_rmids_show(struct kernfs_open_file *of, static int rdt_mon_features_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); struct mon_evt *mevt; list_for_each_entry(mevt, &r->evt_list, list) { @@ -1121,7 +1131,7 @@ static int rdt_mon_features_show(struct kernfs_open_file *of, static int rdt_bw_gran_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%u\n", r->membw.bw_gran); @@ -1131,7 +1141,7 @@ static int rdt_bw_gran_show(struct kernfs_open_file *of, static int rdt_delay_linear_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%u\n", r->membw.delay_linear); @@ -1149,7 +1159,7 @@ static int max_threshold_occ_show(struct kernfs_open_file *of, static int rdt_thread_throttle_mode_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; if (r->membw.throttle_mode == THREAD_THROTTLE_PER_THREAD) @@ -1214,7 +1224,7 @@ static enum resctrl_conf_type resctrl_peer_type(enum resctrl_conf_type my_type) static int rdt_has_sparse_bitmasks_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct resctrl_schema *s = of->kn->parent->priv; + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); struct rdt_resource *r = s->res; seq_printf(seq, "%u\n", r->cache.arch_has_sparse_bitmasks); @@ -1626,7 +1636,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid static int mbm_total_bytes_config_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID); @@ -1636,7 +1646,7 @@ static int mbm_total_bytes_config_show(struct kernfs_open_file *of, static int mbm_local_bytes_config_show(struct kernfs_open_file *of, struct seq_file *seq, void *v) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID); @@ -1742,7 +1752,7 @@ static ssize_t mbm_total_bytes_config_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); int ret; /* Valid input requires a trailing newline */ @@ -1768,7 +1778,7 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { - struct rdt_resource *r = of->kn->parent->priv; + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); int ret; /* Valid input requires a trailing newline */ @@ -2430,12 +2440,13 @@ static struct rdtgroup *kernfs_to_rdtgroup(struct kernfs_node *kn) * resource. "info" and its subdirectories don't * have rdtgroup structures, so return NULL here. */ - if (kn == kn_info || kn->parent == kn_info) + if (kn == kn_info || + rcu_access_pointer(kn->__parent) == kn_info) return NULL; else return kn->priv; } else { - return kn->parent->priv; + return rdt_kn_parent_priv(kn); } } @@ -3759,9 +3770,18 @@ static int rdtgroup_rmdir_ctrl(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask) return 0; } +static struct kernfs_node *rdt_kn_parent(struct kernfs_node *kn) +{ + /* + * Valid within the RCU section it was obtained or while rdtgroup_mutex + * is held. + */ + return rcu_dereference_check(kn->__parent, lockdep_is_held(&rdtgroup_mutex)); +} + static int rdtgroup_rmdir(struct kernfs_node *kn) { - struct kernfs_node *parent_kn = kn->parent; + struct kernfs_node *parent_kn; struct rdtgroup *rdtgrp; cpumask_var_t tmpmask; int ret = 0; @@ -3774,6 +3794,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn) ret = -EPERM; goto out; } + parent_kn = rdt_kn_parent(kn); /* * If the rdtgroup is a ctrl_mon group and parent directory @@ -3842,6 +3863,7 @@ static void mongrp_reparent(struct rdtgroup *rdtgrp, static int rdtgroup_rename(struct kernfs_node *kn, struct kernfs_node *new_parent, const char *new_name) { + struct kernfs_node *kn_parent; struct rdtgroup *new_prdtgrp; struct rdtgroup *rdtgrp; cpumask_var_t tmpmask; @@ -3876,8 +3898,9 @@ static int rdtgroup_rename(struct kernfs_node *kn, goto out; } - if (rdtgrp->type != RDTMON_GROUP || !kn->parent || - !is_mon_groups(kn->parent, kn->name)) { + kn_parent = rdt_kn_parent(kn); + if (rdtgrp->type != RDTMON_GROUP || !kn_parent || + !is_mon_groups(kn_parent, kn->name)) { rdt_last_cmd_puts("Source must be a MON group\n"); ret = -EPERM; goto out; diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 5a1fea414996e..d2306641b569c 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -17,7 +17,7 @@ #include "kernfs-internal.h" -static DEFINE_RWLOCK(kernfs_rename_lock); /* kn->parent and ->name */ +DEFINE_RWLOCK(kernfs_rename_lock); /* kn->parent and ->name */ /* * Don't use rename_lock to piggy back on pr_cont_buf. We don't want to * call pr_cont() while holding rename_lock. Because sometimes pr_cont() @@ -56,7 +56,7 @@ static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen) if (!kn) return strscpy(buf, "(null)", buflen); - return strscpy(buf, kn->parent ? kn->name : "/", buflen); + return strscpy(buf, rcu_access_pointer(kn->__parent) ? kn->name : "/", buflen); } /* kernfs_node_depth - compute depth from @from to @to */ @@ -64,9 +64,9 @@ static size_t kernfs_depth(struct kernfs_node *from, struct kernfs_node *to) { size_t depth = 0; - while (to->parent && to != from) { + while (rcu_dereference(to->__parent) && to != from) { depth++; - to = to->parent; + to = rcu_dereference(to->__parent); } return depth; } @@ -84,18 +84,18 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a, db = kernfs_depth(rb->kn, b); while (da > db) { - a = a->parent; + a = rcu_dereference(a->__parent); da--; } while (db > da) { - b = b->parent; + b = rcu_dereference(b->__parent); db--; } /* worst case b and a will be the same at root */ while (b != a) { - b = b->parent; - a = a->parent; + b = rcu_dereference(b->__parent); + a = rcu_dereference(a->__parent); } return a; @@ -168,8 +168,9 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, /* Calculate how many bytes we need for the rest */ for (i = depth_to - 1; i >= 0; i--) { + for (kn = kn_to, j = 0; j < i; j++) - kn = kn->parent; + kn = rcu_dereference(kn->__parent); len += scnprintf(buf + len, buflen - len, "/%s", kn->name); } @@ -226,6 +227,7 @@ int kernfs_path_from_node(struct kernfs_node *to, struct kernfs_node *from, unsigned long flags; int ret; + guard(rcu)(); read_lock_irqsave(&kernfs_rename_lock, flags); ret = kernfs_path_from_node_locked(to, from, buf, buflen); read_unlock_irqrestore(&kernfs_rename_lock, flags); @@ -295,7 +297,7 @@ struct kernfs_node *kernfs_get_parent(struct kernfs_node *kn) unsigned long flags; read_lock_irqsave(&kernfs_rename_lock, flags); - parent = kn->parent; + parent = kernfs_parent(kn); kernfs_get(parent); read_unlock_irqrestore(&kernfs_rename_lock, flags); @@ -360,8 +362,12 @@ static int kernfs_sd_compare(const struct kernfs_node *left, */ static int kernfs_link_sibling(struct kernfs_node *kn) { - struct rb_node **node = &kn->parent->dir.children.rb_node; struct rb_node *parent = NULL; + struct kernfs_node *kn_parent; + struct rb_node **node; + + kn_parent = kernfs_parent(kn); + node = &kn_parent->dir.children.rb_node; while (*node) { struct kernfs_node *pos; @@ -380,13 +386,13 @@ static int kernfs_link_sibling(struct kernfs_node *kn) /* add new node and rebalance the tree */ rb_link_node(&kn->rb, parent, node); - rb_insert_color(&kn->rb, &kn->parent->dir.children); + rb_insert_color(&kn->rb, &kn_parent->dir.children); /* successfully added, account subdir number */ down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); if (kernfs_type(kn) == KERNFS_DIR) - kn->parent->dir.subdirs++; - kernfs_inc_rev(kn->parent); + kn_parent->dir.subdirs++; + kernfs_inc_rev(kn_parent); up_write(&kernfs_root(kn)->kernfs_iattr_rwsem); return 0; @@ -407,16 +413,19 @@ static int kernfs_link_sibling(struct kernfs_node *kn) */ static bool kernfs_unlink_sibling(struct kernfs_node *kn) { + struct kernfs_node *kn_parent; + if (RB_EMPTY_NODE(&kn->rb)) return false; + kn_parent = kernfs_parent(kn); down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); if (kernfs_type(kn) == KERNFS_DIR) - kn->parent->dir.subdirs--; - kernfs_inc_rev(kn->parent); + kn_parent->dir.subdirs--; + kernfs_inc_rev(kn_parent); up_write(&kernfs_root(kn)->kernfs_iattr_rwsem); - rb_erase(&kn->rb, &kn->parent->dir.children); + rb_erase(&kn->rb, &kn_parent->dir.children); RB_CLEAR_NODE(&kn->rb); return true; } @@ -562,7 +571,7 @@ void kernfs_put(struct kernfs_node *kn) * Moving/renaming is always done while holding reference. * kn->parent won't change beneath us. */ - parent = kn->parent; + parent = kernfs_parent(kn); WARN_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS, "kernfs_put: %s/%s: released with incorrect active_ref %d\n", @@ -701,7 +710,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent, name, mode, uid, gid, flags); if (kn) { kernfs_get(parent); - kn->parent = parent; + rcu_assign_pointer(kn->__parent, parent); } return kn; } @@ -769,13 +778,14 @@ struct kernfs_node *kernfs_find_and_get_node_by_id(struct kernfs_root *root, */ int kernfs_add_one(struct kernfs_node *kn) { - struct kernfs_node *parent = kn->parent; - struct kernfs_root *root = kernfs_root(parent); + struct kernfs_root *root = kernfs_root(kn); struct kernfs_iattrs *ps_iattr; + struct kernfs_node *parent; bool has_ns; int ret; down_write(&root->kernfs_rwsem); + parent = kernfs_parent(kn); ret = -EINVAL; has_ns = kernfs_ns_enabled(parent); @@ -949,6 +959,11 @@ struct kernfs_node *kernfs_walk_and_get_ns(struct kernfs_node *parent, return kn; } +unsigned int kernfs_root_flags(struct kernfs_node *kn) +{ + return kernfs_root(kn)->flags; +} + /** * kernfs_create_root - create a new kernfs hierarchy * @scops: optional syscall operations for the hierarchy @@ -1111,7 +1126,7 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent, static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) { - struct kernfs_node *kn; + struct kernfs_node *kn, *parent; struct kernfs_root *root; if (flags & LOOKUP_RCU) @@ -1162,8 +1177,9 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) if (!kernfs_active(kn)) goto out_bad; + parent = kernfs_parent(kn); /* The kernfs node has been moved? */ - if (kernfs_dentry_node(dentry->d_parent) != kn->parent) + if (kernfs_dentry_node(dentry->d_parent) != parent) goto out_bad; /* The kernfs node has been renamed */ @@ -1171,7 +1187,7 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) goto out_bad; /* The kernfs node has been moved to a different namespace */ - if (kn->parent && kernfs_ns_enabled(kn->parent) && + if (parent && kernfs_ns_enabled(parent) && kernfs_info(dentry->d_sb)->ns != kn->ns) goto out_bad; @@ -1364,7 +1380,7 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos, return kernfs_leftmost_descendant(rb_to_kn(rbn)); /* no sibling left, visit parent */ - return pos->parent; + return kernfs_parent(pos); } static void kernfs_activate_one(struct kernfs_node *kn) @@ -1376,7 +1392,7 @@ static void kernfs_activate_one(struct kernfs_node *kn) if (kernfs_active(kn) || (kn->flags & (KERNFS_HIDDEN | KERNFS_REMOVING))) return; - WARN_ON_ONCE(kn->parent && RB_EMPTY_NODE(&kn->rb)); + WARN_ON_ONCE(rcu_access_pointer(kn->__parent) && RB_EMPTY_NODE(&kn->rb)); WARN_ON_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS); atomic_sub(KN_DEACTIVATED_BIAS, &kn->active); @@ -1446,7 +1462,7 @@ void kernfs_show(struct kernfs_node *kn, bool show) static void __kernfs_remove(struct kernfs_node *kn) { - struct kernfs_node *pos; + struct kernfs_node *pos, *parent; /* Short-circuit if non-root @kn has already finished removal. */ if (!kn) @@ -1458,7 +1474,7 @@ static void __kernfs_remove(struct kernfs_node *kn) * This is for kernfs_remove_self() which plays with active ref * after removal. */ - if (kn->parent && RB_EMPTY_NODE(&kn->rb)) + if (kernfs_parent(kn) && RB_EMPTY_NODE(&kn->rb)) return; pr_debug("kernfs %s: removing\n", kn->name); @@ -1484,14 +1500,14 @@ static void __kernfs_remove(struct kernfs_node *kn) kernfs_get(pos); kernfs_drain(pos); - + parent = kernfs_parent(pos); /* * kernfs_unlink_sibling() succeeds once per node. Use it * to decide who's responsible for cleanups. */ - if (!pos->parent || kernfs_unlink_sibling(pos)) { + if (!parent || kernfs_unlink_sibling(pos)) { struct kernfs_iattrs *ps_iattr = - pos->parent ? pos->parent->iattr : NULL; + parent ? parent->iattr : NULL; /* update timestamps on the parent */ down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); @@ -1721,7 +1737,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, int error; /* can't move or rename root */ - if (!kn->parent) + if (!rcu_access_pointer(kn->__parent)) return -EINVAL; root = kernfs_root(kn); @@ -1732,8 +1748,15 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, (new_parent->flags & KERNFS_EMPTY_DIR)) goto out; + old_parent = kernfs_parent(kn); + if (root->flags & KERNFS_ROOT_INVARIANT_PARENT) { + error = -EINVAL; + if (WARN_ON_ONCE(old_parent != new_parent)) + goto out; + } + error = 0; - if ((kn->parent == new_parent) && (kn->ns == new_ns) && + if ((old_parent == new_parent) && (kn->ns == new_ns) && (strcmp(kn->name, new_name) == 0)) goto out; /* nothing to rename */ @@ -1760,8 +1783,8 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, /* rename_lock protects ->parent and ->name accessors */ write_lock_irq(&kernfs_rename_lock); - old_parent = kn->parent; - kn->parent = new_parent; + old_parent = kernfs_parent(kn); + rcu_assign_pointer(kn->__parent, new_parent); kn->ns = new_ns; if (new_name) { @@ -1794,7 +1817,8 @@ static struct kernfs_node *kernfs_dir_pos(const void *ns, { if (pos) { int valid = kernfs_active(pos) && - pos->parent == parent && hash == pos->hash; + rcu_access_pointer(pos->__parent) == parent && + hash == pos->hash; kernfs_put(pos); if (!valid) pos = NULL; diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h index b42ee6547cdc1..c43bee18b79f7 100644 --- a/fs/kernfs/kernfs-internal.h +++ b/fs/kernfs/kernfs-internal.h @@ -19,6 +19,8 @@ #include <linux/kernfs.h> #include <linux/fs_context.h> +extern rwlock_t kernfs_rename_lock; + struct kernfs_iattrs { kuid_t ia_uid; kgid_t ia_gid; @@ -64,11 +66,14 @@ struct kernfs_root { * * Return: the kernfs_root @kn belongs to. */ -static inline struct kernfs_root *kernfs_root(struct kernfs_node *kn) +static inline struct kernfs_root *kernfs_root(const struct kernfs_node *kn) { + const struct kernfs_node *knp; /* if parent exists, it's always a dir; otherwise, @sd is a dir */ - if (kn->parent) - kn = kn->parent; + guard(rcu)(); + knp = rcu_dereference(kn->__parent); + if (knp) + kn = knp; return kn->dir.root; } @@ -97,6 +102,27 @@ struct kernfs_super_info { }; #define kernfs_info(SB) ((struct kernfs_super_info *)(SB->s_fs_info)) +static inline bool kernfs_root_is_locked(const struct kernfs_node *kn) +{ + return lockdep_is_held(&kernfs_root(kn)->kernfs_rwsem); +} + +static inline struct kernfs_node *kernfs_parent(const struct kernfs_node *kn) +{ + /* + * The kernfs_node::__parent remains valid within a RCU section. The kn + * can be reparented (and renamed) which changes the entry. This can be + * avoided by locking kernfs_root::kernfs_rwsem or kernfs_rename_lock. + * Both locks can be used to obtain a reference on __parent. Once the + * reference count reaches 0 then the node is about to be freed + * and can not be renamed (or become a different parent) anymore. + */ + return rcu_dereference_check(kn->__parent, + kernfs_root_is_locked(kn) || + lockdep_is_held(&kernfs_rename_lock) || + !atomic_read(&kn->count)); +} + static inline struct kernfs_node *kernfs_dentry_node(struct dentry *dentry) { if (d_really_is_negative(dentry)) diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c index 4a0ff08d589ca..2252b16e6ef0b 100644 --- a/fs/kernfs/mount.c +++ b/fs/kernfs/mount.c @@ -148,7 +148,7 @@ static struct dentry *kernfs_get_parent_dentry(struct dentry *child) struct kernfs_root *root = kernfs_root(kn); guard(rwsem_read)(&root->kernfs_rwsem); - return d_obtain_alias(kernfs_get_inode(child->d_sb, kn->parent)); + return d_obtain_alias(kernfs_get_inode(child->d_sb, kernfs_parent(kn))); } static const struct export_operations kernfs_export_ops = { @@ -188,10 +188,10 @@ static struct kernfs_node *find_next_ancestor(struct kernfs_node *child, return NULL; } - while (child->parent != parent) { - if (!child->parent) + while (kernfs_parent(child) != parent) { + child = kernfs_parent(child); + if (!child) return NULL; - child = child->parent; } return child; @@ -216,7 +216,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn, dentry = dget(sb->s_root); /* Check if this is the root kernfs_node */ - if (!kn->parent) + if (!rcu_access_pointer(kn->__parent)) return dentry; root = kernfs_root(kn); diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c index 45371a70caa71..05c62ca93c53d 100644 --- a/fs/kernfs/symlink.c +++ b/fs/kernfs/symlink.c @@ -62,10 +62,10 @@ static int kernfs_get_target_path(struct kernfs_node *parent, /* go up to the root, stop at the base */ base = parent; - while (base->parent) { - kn = target->parent; - while (kn->parent && base != kn) - kn = kn->parent; + while (kernfs_parent(base)) { + kn = kernfs_parent(target); + while (kernfs_parent(kn) && base != kn) + kn = kernfs_parent(kn); if (base == kn) break; @@ -75,14 +75,14 @@ static int kernfs_get_target_path(struct kernfs_node *parent, strcpy(s, "../"); s += 3; - base = base->parent; + base = kernfs_parent(base); } /* determine end of target string for reverse fillup */ kn = target; - while (kn->parent && kn != base) { + while (kernfs_parent(kn) && kn != base) { len += strlen(kn->name) + 1; - kn = kn->parent; + kn = kernfs_parent(kn); } /* check limits */ @@ -94,7 +94,7 @@ static int kernfs_get_target_path(struct kernfs_node *parent, /* reverse fillup of target string from target to base */ kn = target; - while (kn->parent && kn != base) { + while (kernfs_parent(kn) && kn != base) { int slen = strlen(kn->name); len -= slen; @@ -102,7 +102,7 @@ static int kernfs_get_target_path(struct kernfs_node *parent, if (len) s[--len] = '/'; - kn = kn->parent; + kn = kernfs_parent(kn); } return 0; @@ -111,12 +111,13 @@ static int kernfs_get_target_path(struct kernfs_node *parent, static int kernfs_getlink(struct inode *inode, char *path) { struct kernfs_node *kn = inode->i_private; - struct kernfs_node *parent = kn->parent; + struct kernfs_node *parent; struct kernfs_node *target = kn->symlink.target_kn; - struct kernfs_root *root = kernfs_root(parent); + struct kernfs_root *root = kernfs_root(kn); int error; down_read(&root->kernfs_rwsem); + parent = kernfs_parent(kn); error = kernfs_get_target_path(parent, target, path); up_read(&root->kernfs_rwsem); diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index d1995e2d6c943..3671a3fd60198 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -19,13 +19,19 @@ #include "sysfs.h" +static struct kobject *sysfs_file_kobj(struct kernfs_node *kn) +{ + guard(rcu)(); + return rcu_dereference(kn->__parent)->priv; +} + /* * Determine ktype->sysfs_ops for the given kernfs_node. This function * must be called while holding an active reference. */ static const struct sysfs_ops *sysfs_file_ops(struct kernfs_node *kn) { - struct kobject *kobj = kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(kn); if (kn->flags & KERNFS_LOCKDEP) lockdep_assert_held(kn); @@ -40,7 +46,7 @@ static const struct sysfs_ops *sysfs_file_ops(struct kernfs_node *kn) static int sysfs_kf_seq_show(struct seq_file *sf, void *v) { struct kernfs_open_file *of = sf->private; - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); const struct sysfs_ops *ops = sysfs_file_ops(of->kn); ssize_t count; char *buf; @@ -78,7 +84,7 @@ static ssize_t sysfs_kf_bin_read(struct kernfs_open_file *of, char *buf, size_t count, loff_t pos) { struct bin_attribute *battr = of->kn->priv; - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); loff_t size = file_inode(of->file)->i_size; if (!count) @@ -102,7 +108,7 @@ static ssize_t sysfs_kf_read(struct kernfs_open_file *of, char *buf, size_t count, loff_t pos) { const struct sysfs_ops *ops = sysfs_file_ops(of->kn); - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); ssize_t len; /* @@ -128,7 +134,7 @@ static ssize_t sysfs_kf_write(struct kernfs_open_file *of, char *buf, size_t count, loff_t pos) { const struct sysfs_ops *ops = sysfs_file_ops(of->kn); - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); if (!count) return 0; @@ -141,7 +147,7 @@ static ssize_t sysfs_kf_bin_write(struct kernfs_open_file *of, char *buf, size_t count, loff_t pos) { struct bin_attribute *battr = of->kn->priv; - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); loff_t size = file_inode(of->file)->i_size; if (size) { @@ -162,7 +168,7 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of, struct vm_area_struct *vma) { struct bin_attribute *battr = of->kn->priv; - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); return battr->mmap(of->file, kobj, battr, vma); } @@ -171,7 +177,7 @@ static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset, int whence) { struct bin_attribute *battr = of->kn->priv; - struct kobject *kobj = of->kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(of->kn); if (battr->llseek) return battr->llseek(of->file, kobj, battr, offset, whence); @@ -482,7 +488,7 @@ EXPORT_SYMBOL_GPL(sysfs_break_active_protection); */ void sysfs_unbreak_active_protection(struct kernfs_node *kn) { - struct kobject *kobj = kn->parent->priv; + struct kobject *kobj = sysfs_file_kobj(kn); kernfs_unbreak_active_protection(kn); kernfs_put(kn); diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index 87c79d076d6d7..5dda9a268e44c 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -147,6 +147,11 @@ enum kernfs_root_flag { * Support user xattrs to be written to nodes rooted at this root. */ KERNFS_ROOT_SUPPORT_USER_XATTR = 0x0008, + + /* + * Renames must not change the parent node. + */ + KERNFS_ROOT_INVARIANT_PARENT = 0x0010, }; /* type-specific structures for kernfs_node union members */ @@ -199,8 +204,8 @@ struct kernfs_node { * never moved to a different parent, it is safe to access the * parent directly. */ - struct kernfs_node *parent; const char *name; + struct kernfs_node __rcu *__parent; struct rb_node rb; @@ -416,6 +421,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn, struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops, unsigned int flags, void *priv); void kernfs_destroy_root(struct kernfs_root *root); +unsigned int kernfs_root_flags(struct kernfs_node *kn); struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent, const char *name, umode_t mode, @@ -514,6 +520,8 @@ kernfs_create_root(struct kernfs_syscall_ops *scops, unsigned int flags, { return ERR_PTR(-ENOSYS); } static inline void kernfs_destroy_root(struct kernfs_root *root) { } +static inline unsigned int kernfs_root_flags(struct kernfs_node *kn) +{ return 0; } static inline struct kernfs_node * kernfs_create_dir_ns(struct kernfs_node *parent, const char *name, diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c index e28d5f0d20ed0..c9752eb607ec9 100644 --- a/kernel/cgroup/cgroup-v1.c +++ b/kernel/cgroup/cgroup-v1.c @@ -844,7 +844,7 @@ static int cgroup1_rename(struct kernfs_node *kn, struct kernfs_node *new_parent if (kernfs_type(kn) != KERNFS_DIR) return -ENOTDIR; - if (kn->parent != new_parent) + if (rcu_access_pointer(kn->__parent) != new_parent) return -EIO; /* diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index e63d6f3b00470..fc152691a6b42 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -633,9 +633,22 @@ int cgroup_task_count(const struct cgroup *cgrp) return count; } +static struct cgroup *kn_priv(struct kernfs_node *kn) +{ + struct kernfs_node *parent; + /* + * The parent can not be replaced due to KERNFS_ROOT_INVARIANT_PARENT. + * Therefore it is always safe to dereference this pointer outside of a + * RCU section. + */ + parent = rcu_dereference_check(kn->__parent, + kernfs_root_flags(kn) & KERNFS_ROOT_INVARIANT_PARENT); + return parent->priv; +} + struct cgroup_subsys_state *of_css(struct kernfs_open_file *of) { - struct cgroup *cgrp = of->kn->parent->priv; + struct cgroup *cgrp = kn_priv(of->kn); struct cftype *cft = of_cft(of); /* @@ -1612,7 +1625,7 @@ void cgroup_kn_unlock(struct kernfs_node *kn) if (kernfs_type(kn) == KERNFS_DIR) cgrp = kn->priv; else - cgrp = kn->parent->priv; + cgrp = kn_priv(kn); cgroup_unlock(); @@ -1644,7 +1657,7 @@ struct cgroup *cgroup_kn_lock_live(struct kernfs_node *kn, bool drain_offline) if (kernfs_type(kn) == KERNFS_DIR) cgrp = kn->priv; else - cgrp = kn->parent->priv; + cgrp = kn_priv(kn); /* * We're gonna grab cgroup_mutex which nests outside kernfs @@ -2118,7 +2131,8 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask) root->kf_root = kernfs_create_root(kf_sops, KERNFS_ROOT_CREATE_DEACTIVATED | KERNFS_ROOT_SUPPORT_EXPORTOP | - KERNFS_ROOT_SUPPORT_USER_XATTR, + KERNFS_ROOT_SUPPORT_USER_XATTR | + KERNFS_ROOT_INVARIANT_PARENT, root_cgrp); if (IS_ERR(root->kf_root)) { ret = PTR_ERR(root->kf_root); @@ -4144,7 +4158,7 @@ static ssize_t cgroup_file_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { struct cgroup_file_ctx *ctx = of->priv; - struct cgroup *cgrp = of->kn->parent->priv; + struct cgroup *cgrp = kn_priv(of->kn); struct cftype *cft = of_cft(of); struct cgroup_subsys_state *css; int ret; diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h index 8bd1ebd7d6afd..813143b4985dc 100644 --- a/tools/testing/selftests/bpf/progs/profiler.inc.h +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h @@ -223,7 +223,7 @@ static INLINE void* read_full_cgroup_path(struct kernfs_node* cgroup_node, if (bpf_cmp_likely(filepart_length, <=, MAX_PATH)) { payload += filepart_length; } - cgroup_node = BPF_CORE_READ(cgroup_node, parent); + cgroup_node = BPF_CORE_READ(cgroup_node, __parent); } return payload; } -- 2.39.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent. 2025-05-05 22:39 ` [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent Sasha Levin @ 2026-11-13 12:30 ` Christian Loehle 2026-11-13 19:10 ` Sasha Levin 0 siblings, 1 reply; 6+ messages in thread From: Christian Loehle @ 2026-11-13 12:30 UTC (permalink / raw) To: Sasha Levin, linux-kernel, stable Cc: Sebastian Andrzej Siewior, Tejun Heo, Yonghong Song, Greg Kroah-Hartman, reinette.chatre, tglx, mingo, bp, dave.hansen, x86, rafael, dakr, hannes, mkoutny, ast, daniel, andrii, eddyz87, shuah, jolsa, alan.maguire, cgroups, bpf, linux-kselftest On 5/5/25 23:39, Sasha Levin wrote: > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > [ Upstream commit 633488947ef66b194377411322dc9e12aab79b65 ] > > kernfs_rename_lock is used to obtain stable kernfs_node::{name|parent} > pointer. This is a preparation to access kernfs_node::parent under RCU > and ensure that the pointer remains stable under the RCU lifetime > guarantees. > > For a complete path, as it is done in kernfs_path_from_node(), the > kernfs_rename_lock is still required in order to obtain a stable parent > relationship while computing the relevant node depth. This must not > change while the nodes are inspected in order to build the path. > If the kernfs user never moves the nodes (changes the parent) then the > kernfs_rename_lock is not required and the RCU guarantees are > sufficient. This "restriction" can be set with > KERNFS_ROOT_INVARIANT_PARENT. Otherwise the lock is required. > > Rename kernfs_node::parent to kernfs_node::__parent to denote the RCU > access and use RCU accessor while accessing the node. > Make cgroup use KERNFS_ROOT_INVARIANT_PARENT since the parent here can > not change. > > Acked-by: Tejun Heo <tj@kernel.org> > Cc: Yonghong Song <yonghong.song@linux.dev> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Link: https://lore.kernel.org/r/20250213145023.2820193-6-bigeasy@linutronix.de > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 65 +++++++++---- > fs/kernfs/dir.c | 96 ++++++++++++------- > fs/kernfs/kernfs-internal.h | 32 ++++++- > fs/kernfs/mount.c | 10 +- > fs/kernfs/symlink.c | 23 ++--- > fs/sysfs/file.c | 24 +++-- > include/linux/kernfs.h | 10 +- > kernel/cgroup/cgroup-v1.c | 2 +- > kernel/cgroup/cgroup.c | 24 ++++- > .../selftests/bpf/progs/profiler.inc.h | 2 +- > 10 files changed, 195 insertions(+), 93 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index 2d48db66fca85..d4d7ccf1253b8 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -948,10 +948,20 @@ static int rdt_last_cmd_status_show(struct kernfs_open_file *of, > return 0; > } > > +static void *rdt_kn_parent_priv(struct kernfs_node *kn) > +{ > + /* > + * The parent pointer is only valid within RCU section since it can be > + * replaced. > + */ > + guard(rcu)(); > + return rcu_dereference(kn->__parent)->priv; > +} > + > static int rdt_num_closids_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > > seq_printf(seq, "%u\n", s->num_closid); > return 0; > @@ -960,7 +970,7 @@ static int rdt_num_closids_show(struct kernfs_open_file *of, > static int rdt_default_ctrl_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%x\n", r->default_ctrl); > @@ -970,7 +980,7 @@ static int rdt_default_ctrl_show(struct kernfs_open_file *of, > static int rdt_min_cbm_bits_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%u\n", r->cache.min_cbm_bits); > @@ -980,7 +990,7 @@ static int rdt_min_cbm_bits_show(struct kernfs_open_file *of, > static int rdt_shareable_bits_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%x\n", r->cache.shareable_bits); > @@ -1004,7 +1014,7 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of, > static int rdt_bit_usage_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > /* > * Use unsigned long even though only 32 bits are used to ensure > * test_bit() is used safely. > @@ -1086,7 +1096,7 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of, > static int rdt_min_bw_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%u\n", r->membw.min_bw); > @@ -1096,7 +1106,7 @@ static int rdt_min_bw_show(struct kernfs_open_file *of, > static int rdt_num_rmids_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > > seq_printf(seq, "%d\n", r->num_rmid); > > @@ -1106,7 +1116,7 @@ static int rdt_num_rmids_show(struct kernfs_open_file *of, > static int rdt_mon_features_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > struct mon_evt *mevt; > > list_for_each_entry(mevt, &r->evt_list, list) { > @@ -1121,7 +1131,7 @@ static int rdt_mon_features_show(struct kernfs_open_file *of, > static int rdt_bw_gran_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%u\n", r->membw.bw_gran); > @@ -1131,7 +1141,7 @@ static int rdt_bw_gran_show(struct kernfs_open_file *of, > static int rdt_delay_linear_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%u\n", r->membw.delay_linear); > @@ -1149,7 +1159,7 @@ static int max_threshold_occ_show(struct kernfs_open_file *of, > static int rdt_thread_throttle_mode_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > if (r->membw.throttle_mode == THREAD_THROTTLE_PER_THREAD) > @@ -1214,7 +1224,7 @@ static enum resctrl_conf_type resctrl_peer_type(enum resctrl_conf_type my_type) > static int rdt_has_sparse_bitmasks_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct resctrl_schema *s = of->kn->parent->priv; > + struct resctrl_schema *s = rdt_kn_parent_priv(of->kn); > struct rdt_resource *r = s->res; > > seq_printf(seq, "%u\n", r->cache.arch_has_sparse_bitmasks); > @@ -1626,7 +1636,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid > static int mbm_total_bytes_config_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > > mbm_config_show(seq, r, QOS_L3_MBM_TOTAL_EVENT_ID); > > @@ -1636,7 +1646,7 @@ static int mbm_total_bytes_config_show(struct kernfs_open_file *of, > static int mbm_local_bytes_config_show(struct kernfs_open_file *of, > struct seq_file *seq, void *v) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > > mbm_config_show(seq, r, QOS_L3_MBM_LOCAL_EVENT_ID); > > @@ -1742,7 +1752,7 @@ static ssize_t mbm_total_bytes_config_write(struct kernfs_open_file *of, > char *buf, size_t nbytes, > loff_t off) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > int ret; > > /* Valid input requires a trailing newline */ > @@ -1768,7 +1778,7 @@ static ssize_t mbm_local_bytes_config_write(struct kernfs_open_file *of, > char *buf, size_t nbytes, > loff_t off) > { > - struct rdt_resource *r = of->kn->parent->priv; > + struct rdt_resource *r = rdt_kn_parent_priv(of->kn); > int ret; > > /* Valid input requires a trailing newline */ > @@ -2430,12 +2440,13 @@ static struct rdtgroup *kernfs_to_rdtgroup(struct kernfs_node *kn) > * resource. "info" and its subdirectories don't > * have rdtgroup structures, so return NULL here. > */ > - if (kn == kn_info || kn->parent == kn_info) > + if (kn == kn_info || > + rcu_access_pointer(kn->__parent) == kn_info) > return NULL; > else > return kn->priv; > } else { > - return kn->parent->priv; > + return rdt_kn_parent_priv(kn); > } > } > > @@ -3759,9 +3770,18 @@ static int rdtgroup_rmdir_ctrl(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask) > return 0; > } > > +static struct kernfs_node *rdt_kn_parent(struct kernfs_node *kn) > +{ > + /* > + * Valid within the RCU section it was obtained or while rdtgroup_mutex > + * is held. > + */ > + return rcu_dereference_check(kn->__parent, lockdep_is_held(&rdtgroup_mutex)); > +} > + > static int rdtgroup_rmdir(struct kernfs_node *kn) > { > - struct kernfs_node *parent_kn = kn->parent; > + struct kernfs_node *parent_kn; > struct rdtgroup *rdtgrp; > cpumask_var_t tmpmask; > int ret = 0; > @@ -3774,6 +3794,7 @@ static int rdtgroup_rmdir(struct kernfs_node *kn) > ret = -EPERM; > goto out; > } > + parent_kn = rdt_kn_parent(kn); > > /* > * If the rdtgroup is a ctrl_mon group and parent directory > @@ -3842,6 +3863,7 @@ static void mongrp_reparent(struct rdtgroup *rdtgrp, > static int rdtgroup_rename(struct kernfs_node *kn, > struct kernfs_node *new_parent, const char *new_name) > { > + struct kernfs_node *kn_parent; > struct rdtgroup *new_prdtgrp; > struct rdtgroup *rdtgrp; > cpumask_var_t tmpmask; > @@ -3876,8 +3898,9 @@ static int rdtgroup_rename(struct kernfs_node *kn, > goto out; > } > > - if (rdtgrp->type != RDTMON_GROUP || !kn->parent || > - !is_mon_groups(kn->parent, kn->name)) { > + kn_parent = rdt_kn_parent(kn); > + if (rdtgrp->type != RDTMON_GROUP || !kn_parent || > + !is_mon_groups(kn_parent, kn->name)) { > rdt_last_cmd_puts("Source must be a MON group\n"); > ret = -EPERM; > goto out; > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > index 5a1fea414996e..d2306641b569c 100644 > --- a/fs/kernfs/dir.c > +++ b/fs/kernfs/dir.c > @@ -17,7 +17,7 @@ > > #include "kernfs-internal.h" > > -static DEFINE_RWLOCK(kernfs_rename_lock); /* kn->parent and ->name */ > +DEFINE_RWLOCK(kernfs_rename_lock); /* kn->parent and ->name */ > /* > * Don't use rename_lock to piggy back on pr_cont_buf. We don't want to > * call pr_cont() while holding rename_lock. Because sometimes pr_cont() > @@ -56,7 +56,7 @@ static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t buflen) > if (!kn) > return strscpy(buf, "(null)", buflen); > > - return strscpy(buf, kn->parent ? kn->name : "/", buflen); > + return strscpy(buf, rcu_access_pointer(kn->__parent) ? kn->name : "/", buflen); > } > > /* kernfs_node_depth - compute depth from @from to @to */ > @@ -64,9 +64,9 @@ static size_t kernfs_depth(struct kernfs_node *from, struct kernfs_node *to) > { > size_t depth = 0; > > - while (to->parent && to != from) { > + while (rcu_dereference(to->__parent) && to != from) { > depth++; > - to = to->parent; > + to = rcu_dereference(to->__parent); > } > return depth; > } > @@ -84,18 +84,18 @@ static struct kernfs_node *kernfs_common_ancestor(struct kernfs_node *a, > db = kernfs_depth(rb->kn, b); > > while (da > db) { > - a = a->parent; > + a = rcu_dereference(a->__parent); > da--; > } > while (db > da) { > - b = b->parent; > + b = rcu_dereference(b->__parent); > db--; > } > > /* worst case b and a will be the same at root */ > while (b != a) { > - b = b->parent; > - a = a->parent; > + b = rcu_dereference(b->__parent); > + a = rcu_dereference(a->__parent); > } > > return a; > @@ -168,8 +168,9 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to, > > /* Calculate how many bytes we need for the rest */ > for (i = depth_to - 1; i >= 0; i--) { > + > for (kn = kn_to, j = 0; j < i; j++) > - kn = kn->parent; > + kn = rcu_dereference(kn->__parent); > > len += scnprintf(buf + len, buflen - len, "/%s", kn->name); > } > @@ -226,6 +227,7 @@ int kernfs_path_from_node(struct kernfs_node *to, struct kernfs_node *from, > unsigned long flags; > int ret; > > + guard(rcu)(); > read_lock_irqsave(&kernfs_rename_lock, flags); > ret = kernfs_path_from_node_locked(to, from, buf, buflen); > read_unlock_irqrestore(&kernfs_rename_lock, flags); > @@ -295,7 +297,7 @@ struct kernfs_node *kernfs_get_parent(struct kernfs_node *kn) > unsigned long flags; > > read_lock_irqsave(&kernfs_rename_lock, flags); > - parent = kn->parent; > + parent = kernfs_parent(kn); > kernfs_get(parent); > read_unlock_irqrestore(&kernfs_rename_lock, flags); > > @@ -360,8 +362,12 @@ static int kernfs_sd_compare(const struct kernfs_node *left, > */ > static int kernfs_link_sibling(struct kernfs_node *kn) > { > - struct rb_node **node = &kn->parent->dir.children.rb_node; > struct rb_node *parent = NULL; > + struct kernfs_node *kn_parent; > + struct rb_node **node; > + > + kn_parent = kernfs_parent(kn); > + node = &kn_parent->dir.children.rb_node; > > while (*node) { > struct kernfs_node *pos; > @@ -380,13 +386,13 @@ static int kernfs_link_sibling(struct kernfs_node *kn) > > /* add new node and rebalance the tree */ > rb_link_node(&kn->rb, parent, node); > - rb_insert_color(&kn->rb, &kn->parent->dir.children); > + rb_insert_color(&kn->rb, &kn_parent->dir.children); > > /* successfully added, account subdir number */ > down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); > if (kernfs_type(kn) == KERNFS_DIR) > - kn->parent->dir.subdirs++; > - kernfs_inc_rev(kn->parent); > + kn_parent->dir.subdirs++; > + kernfs_inc_rev(kn_parent); > up_write(&kernfs_root(kn)->kernfs_iattr_rwsem); > > return 0; > @@ -407,16 +413,19 @@ static int kernfs_link_sibling(struct kernfs_node *kn) > */ > static bool kernfs_unlink_sibling(struct kernfs_node *kn) > { > + struct kernfs_node *kn_parent; > + > if (RB_EMPTY_NODE(&kn->rb)) > return false; > > + kn_parent = kernfs_parent(kn); > down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); > if (kernfs_type(kn) == KERNFS_DIR) > - kn->parent->dir.subdirs--; > - kernfs_inc_rev(kn->parent); > + kn_parent->dir.subdirs--; > + kernfs_inc_rev(kn_parent); > up_write(&kernfs_root(kn)->kernfs_iattr_rwsem); > > - rb_erase(&kn->rb, &kn->parent->dir.children); > + rb_erase(&kn->rb, &kn_parent->dir.children); > RB_CLEAR_NODE(&kn->rb); > return true; > } > @@ -562,7 +571,7 @@ void kernfs_put(struct kernfs_node *kn) > * Moving/renaming is always done while holding reference. > * kn->parent won't change beneath us. > */ > - parent = kn->parent; > + parent = kernfs_parent(kn); > > WARN_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS, > "kernfs_put: %s/%s: released with incorrect active_ref %d\n", > @@ -701,7 +710,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent, > name, mode, uid, gid, flags); > if (kn) { > kernfs_get(parent); > - kn->parent = parent; > + rcu_assign_pointer(kn->__parent, parent); > } > return kn; > } > @@ -769,13 +778,14 @@ struct kernfs_node *kernfs_find_and_get_node_by_id(struct kernfs_root *root, > */ > int kernfs_add_one(struct kernfs_node *kn) > { > - struct kernfs_node *parent = kn->parent; > - struct kernfs_root *root = kernfs_root(parent); > + struct kernfs_root *root = kernfs_root(kn); > struct kernfs_iattrs *ps_iattr; > + struct kernfs_node *parent; > bool has_ns; > int ret; > > down_write(&root->kernfs_rwsem); > + parent = kernfs_parent(kn); > > ret = -EINVAL; > has_ns = kernfs_ns_enabled(parent); > @@ -949,6 +959,11 @@ struct kernfs_node *kernfs_walk_and_get_ns(struct kernfs_node *parent, > return kn; > } > > +unsigned int kernfs_root_flags(struct kernfs_node *kn) > +{ > + return kernfs_root(kn)->flags; > +} > + > /** > * kernfs_create_root - create a new kernfs hierarchy > * @scops: optional syscall operations for the hierarchy > @@ -1111,7 +1126,7 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent, > > static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) > { > - struct kernfs_node *kn; > + struct kernfs_node *kn, *parent; > struct kernfs_root *root; > > if (flags & LOOKUP_RCU) > @@ -1162,8 +1177,9 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) > if (!kernfs_active(kn)) > goto out_bad; > > + parent = kernfs_parent(kn); > /* The kernfs node has been moved? */ > - if (kernfs_dentry_node(dentry->d_parent) != kn->parent) > + if (kernfs_dentry_node(dentry->d_parent) != parent) > goto out_bad; > > /* The kernfs node has been renamed */ > @@ -1171,7 +1187,7 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) > goto out_bad; > > /* The kernfs node has been moved to a different namespace */ > - if (kn->parent && kernfs_ns_enabled(kn->parent) && > + if (parent && kernfs_ns_enabled(parent) && > kernfs_info(dentry->d_sb)->ns != kn->ns) > goto out_bad; > > @@ -1364,7 +1380,7 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos, > return kernfs_leftmost_descendant(rb_to_kn(rbn)); > > /* no sibling left, visit parent */ > - return pos->parent; > + return kernfs_parent(pos); > } > > static void kernfs_activate_one(struct kernfs_node *kn) > @@ -1376,7 +1392,7 @@ static void kernfs_activate_one(struct kernfs_node *kn) > if (kernfs_active(kn) || (kn->flags & (KERNFS_HIDDEN | KERNFS_REMOVING))) > return; > > - WARN_ON_ONCE(kn->parent && RB_EMPTY_NODE(&kn->rb)); > + WARN_ON_ONCE(rcu_access_pointer(kn->__parent) && RB_EMPTY_NODE(&kn->rb)); > WARN_ON_ONCE(atomic_read(&kn->active) != KN_DEACTIVATED_BIAS); > > atomic_sub(KN_DEACTIVATED_BIAS, &kn->active); > @@ -1446,7 +1462,7 @@ void kernfs_show(struct kernfs_node *kn, bool show) > > static void __kernfs_remove(struct kernfs_node *kn) > { > - struct kernfs_node *pos; > + struct kernfs_node *pos, *parent; > > /* Short-circuit if non-root @kn has already finished removal. */ > if (!kn) > @@ -1458,7 +1474,7 @@ static void __kernfs_remove(struct kernfs_node *kn) > * This is for kernfs_remove_self() which plays with active ref > * after removal. > */ > - if (kn->parent && RB_EMPTY_NODE(&kn->rb)) > + if (kernfs_parent(kn) && RB_EMPTY_NODE(&kn->rb)) > return; > > pr_debug("kernfs %s: removing\n", kn->name); > @@ -1484,14 +1500,14 @@ static void __kernfs_remove(struct kernfs_node *kn) > kernfs_get(pos); > > kernfs_drain(pos); > - > + parent = kernfs_parent(pos); > /* > * kernfs_unlink_sibling() succeeds once per node. Use it > * to decide who's responsible for cleanups. > */ > - if (!pos->parent || kernfs_unlink_sibling(pos)) { > + if (!parent || kernfs_unlink_sibling(pos)) { > struct kernfs_iattrs *ps_iattr = > - pos->parent ? pos->parent->iattr : NULL; > + parent ? parent->iattr : NULL; > > /* update timestamps on the parent */ > down_write(&kernfs_root(kn)->kernfs_iattr_rwsem); > @@ -1721,7 +1737,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, > int error; > > /* can't move or rename root */ > - if (!kn->parent) > + if (!rcu_access_pointer(kn->__parent)) > return -EINVAL; > > root = kernfs_root(kn); > @@ -1732,8 +1748,15 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, > (new_parent->flags & KERNFS_EMPTY_DIR)) > goto out; > > + old_parent = kernfs_parent(kn); > + if (root->flags & KERNFS_ROOT_INVARIANT_PARENT) { > + error = -EINVAL; > + if (WARN_ON_ONCE(old_parent != new_parent)) > + goto out; > + } > + > error = 0; > - if ((kn->parent == new_parent) && (kn->ns == new_ns) && > + if ((old_parent == new_parent) && (kn->ns == new_ns) && > (strcmp(kn->name, new_name) == 0)) > goto out; /* nothing to rename */ > > @@ -1760,8 +1783,8 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, > /* rename_lock protects ->parent and ->name accessors */ > write_lock_irq(&kernfs_rename_lock); > > - old_parent = kn->parent; > - kn->parent = new_parent; > + old_parent = kernfs_parent(kn); > + rcu_assign_pointer(kn->__parent, new_parent); > > kn->ns = new_ns; > if (new_name) { > @@ -1794,7 +1817,8 @@ static struct kernfs_node *kernfs_dir_pos(const void *ns, > { > if (pos) { > int valid = kernfs_active(pos) && > - pos->parent == parent && hash == pos->hash; > + rcu_access_pointer(pos->__parent) == parent && > + hash == pos->hash; > kernfs_put(pos); > if (!valid) > pos = NULL; > diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h > index b42ee6547cdc1..c43bee18b79f7 100644 > --- a/fs/kernfs/kernfs-internal.h > +++ b/fs/kernfs/kernfs-internal.h > @@ -19,6 +19,8 @@ > #include <linux/kernfs.h> > #include <linux/fs_context.h> > > +extern rwlock_t kernfs_rename_lock; > + > struct kernfs_iattrs { > kuid_t ia_uid; > kgid_t ia_gid; > @@ -64,11 +66,14 @@ struct kernfs_root { > * > * Return: the kernfs_root @kn belongs to. > */ > -static inline struct kernfs_root *kernfs_root(struct kernfs_node *kn) > +static inline struct kernfs_root *kernfs_root(const struct kernfs_node *kn) > { > + const struct kernfs_node *knp; > /* if parent exists, it's always a dir; otherwise, @sd is a dir */ > - if (kn->parent) > - kn = kn->parent; > + guard(rcu)(); > + knp = rcu_dereference(kn->__parent); > + if (knp) > + kn = knp; > return kn->dir.root; > } > > @@ -97,6 +102,27 @@ struct kernfs_super_info { > }; > #define kernfs_info(SB) ((struct kernfs_super_info *)(SB->s_fs_info)) > > +static inline bool kernfs_root_is_locked(const struct kernfs_node *kn) > +{ > + return lockdep_is_held(&kernfs_root(kn)->kernfs_rwsem); > +} > + > +static inline struct kernfs_node *kernfs_parent(const struct kernfs_node *kn) > +{ > + /* > + * The kernfs_node::__parent remains valid within a RCU section. The kn > + * can be reparented (and renamed) which changes the entry. This can be > + * avoided by locking kernfs_root::kernfs_rwsem or kernfs_rename_lock. > + * Both locks can be used to obtain a reference on __parent. Once the > + * reference count reaches 0 then the node is about to be freed > + * and can not be renamed (or become a different parent) anymore. > + */ > + return rcu_dereference_check(kn->__parent, > + kernfs_root_is_locked(kn) || > + lockdep_is_held(&kernfs_rename_lock) || > + !atomic_read(&kn->count)); > +} > + > static inline struct kernfs_node *kernfs_dentry_node(struct dentry *dentry) > { > if (d_really_is_negative(dentry)) > diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c > index 4a0ff08d589ca..2252b16e6ef0b 100644 > --- a/fs/kernfs/mount.c > +++ b/fs/kernfs/mount.c > @@ -148,7 +148,7 @@ static struct dentry *kernfs_get_parent_dentry(struct dentry *child) > struct kernfs_root *root = kernfs_root(kn); > > guard(rwsem_read)(&root->kernfs_rwsem); > - return d_obtain_alias(kernfs_get_inode(child->d_sb, kn->parent)); > + return d_obtain_alias(kernfs_get_inode(child->d_sb, kernfs_parent(kn))); > } > > static const struct export_operations kernfs_export_ops = { > @@ -188,10 +188,10 @@ static struct kernfs_node *find_next_ancestor(struct kernfs_node *child, > return NULL; > } > > - while (child->parent != parent) { > - if (!child->parent) > + while (kernfs_parent(child) != parent) { > + child = kernfs_parent(child); > + if (!child) > return NULL; > - child = child->parent; > } > > return child; > @@ -216,7 +216,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn, > dentry = dget(sb->s_root); > > /* Check if this is the root kernfs_node */ > - if (!kn->parent) > + if (!rcu_access_pointer(kn->__parent)) > return dentry; > > root = kernfs_root(kn); > diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c > index 45371a70caa71..05c62ca93c53d 100644 > --- a/fs/kernfs/symlink.c > +++ b/fs/kernfs/symlink.c > @@ -62,10 +62,10 @@ static int kernfs_get_target_path(struct kernfs_node *parent, > > /* go up to the root, stop at the base */ > base = parent; > - while (base->parent) { > - kn = target->parent; > - while (kn->parent && base != kn) > - kn = kn->parent; > + while (kernfs_parent(base)) { > + kn = kernfs_parent(target); > + while (kernfs_parent(kn) && base != kn) > + kn = kernfs_parent(kn); > > if (base == kn) > break; > @@ -75,14 +75,14 @@ static int kernfs_get_target_path(struct kernfs_node *parent, > > strcpy(s, "../"); > s += 3; > - base = base->parent; > + base = kernfs_parent(base); > } > > /* determine end of target string for reverse fillup */ > kn = target; > - while (kn->parent && kn != base) { > + while (kernfs_parent(kn) && kn != base) { > len += strlen(kn->name) + 1; > - kn = kn->parent; > + kn = kernfs_parent(kn); > } > > /* check limits */ > @@ -94,7 +94,7 @@ static int kernfs_get_target_path(struct kernfs_node *parent, > > /* reverse fillup of target string from target to base */ > kn = target; > - while (kn->parent && kn != base) { > + while (kernfs_parent(kn) && kn != base) { > int slen = strlen(kn->name); > > len -= slen; > @@ -102,7 +102,7 @@ static int kernfs_get_target_path(struct kernfs_node *parent, > if (len) > s[--len] = '/'; > > - kn = kn->parent; > + kn = kernfs_parent(kn); > } > > return 0; > @@ -111,12 +111,13 @@ static int kernfs_get_target_path(struct kernfs_node *parent, > static int kernfs_getlink(struct inode *inode, char *path) > { > struct kernfs_node *kn = inode->i_private; > - struct kernfs_node *parent = kn->parent; > + struct kernfs_node *parent; > struct kernfs_node *target = kn->symlink.target_kn; > - struct kernfs_root *root = kernfs_root(parent); > + struct kernfs_root *root = kernfs_root(kn); > int error; > > down_read(&root->kernfs_rwsem); > + parent = kernfs_parent(kn); > error = kernfs_get_target_path(parent, target, path); > up_read(&root->kernfs_rwsem); > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index d1995e2d6c943..3671a3fd60198 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -19,13 +19,19 @@ > > #include "sysfs.h" > > +static struct kobject *sysfs_file_kobj(struct kernfs_node *kn) > +{ > + guard(rcu)(); > + return rcu_dereference(kn->__parent)->priv; > +} > + > /* > * Determine ktype->sysfs_ops for the given kernfs_node. This function > * must be called while holding an active reference. > */ > static const struct sysfs_ops *sysfs_file_ops(struct kernfs_node *kn) > { > - struct kobject *kobj = kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(kn); > > if (kn->flags & KERNFS_LOCKDEP) > lockdep_assert_held(kn); > @@ -40,7 +46,7 @@ static const struct sysfs_ops *sysfs_file_ops(struct kernfs_node *kn) > static int sysfs_kf_seq_show(struct seq_file *sf, void *v) > { > struct kernfs_open_file *of = sf->private; > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > const struct sysfs_ops *ops = sysfs_file_ops(of->kn); > ssize_t count; > char *buf; > @@ -78,7 +84,7 @@ static ssize_t sysfs_kf_bin_read(struct kernfs_open_file *of, char *buf, > size_t count, loff_t pos) > { > struct bin_attribute *battr = of->kn->priv; > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > loff_t size = file_inode(of->file)->i_size; > > if (!count) > @@ -102,7 +108,7 @@ static ssize_t sysfs_kf_read(struct kernfs_open_file *of, char *buf, > size_t count, loff_t pos) > { > const struct sysfs_ops *ops = sysfs_file_ops(of->kn); > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > ssize_t len; > > /* > @@ -128,7 +134,7 @@ static ssize_t sysfs_kf_write(struct kernfs_open_file *of, char *buf, > size_t count, loff_t pos) > { > const struct sysfs_ops *ops = sysfs_file_ops(of->kn); > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > > if (!count) > return 0; > @@ -141,7 +147,7 @@ static ssize_t sysfs_kf_bin_write(struct kernfs_open_file *of, char *buf, > size_t count, loff_t pos) > { > struct bin_attribute *battr = of->kn->priv; > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > loff_t size = file_inode(of->file)->i_size; > > if (size) { > @@ -162,7 +168,7 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of, > struct vm_area_struct *vma) > { > struct bin_attribute *battr = of->kn->priv; > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > > return battr->mmap(of->file, kobj, battr, vma); > } > @@ -171,7 +177,7 @@ static loff_t sysfs_kf_bin_llseek(struct kernfs_open_file *of, loff_t offset, > int whence) > { > struct bin_attribute *battr = of->kn->priv; > - struct kobject *kobj = of->kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(of->kn); > > if (battr->llseek) > return battr->llseek(of->file, kobj, battr, offset, whence); > @@ -482,7 +488,7 @@ EXPORT_SYMBOL_GPL(sysfs_break_active_protection); > */ > void sysfs_unbreak_active_protection(struct kernfs_node *kn) > { > - struct kobject *kobj = kn->parent->priv; > + struct kobject *kobj = sysfs_file_kobj(kn); > > kernfs_unbreak_active_protection(kn); > kernfs_put(kn); > diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h > index 87c79d076d6d7..5dda9a268e44c 100644 > --- a/include/linux/kernfs.h > +++ b/include/linux/kernfs.h > @@ -147,6 +147,11 @@ enum kernfs_root_flag { > * Support user xattrs to be written to nodes rooted at this root. > */ > KERNFS_ROOT_SUPPORT_USER_XATTR = 0x0008, > + > + /* > + * Renames must not change the parent node. > + */ > + KERNFS_ROOT_INVARIANT_PARENT = 0x0010, > }; > > /* type-specific structures for kernfs_node union members */ > @@ -199,8 +204,8 @@ struct kernfs_node { > * never moved to a different parent, it is safe to access the > * parent directly. > */ > - struct kernfs_node *parent; > const char *name; > + struct kernfs_node __rcu *__parent; > > struct rb_node rb; > > @@ -416,6 +421,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn, > struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops, > unsigned int flags, void *priv); > void kernfs_destroy_root(struct kernfs_root *root); > +unsigned int kernfs_root_flags(struct kernfs_node *kn); > > struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent, > const char *name, umode_t mode, > @@ -514,6 +520,8 @@ kernfs_create_root(struct kernfs_syscall_ops *scops, unsigned int flags, > { return ERR_PTR(-ENOSYS); } > > static inline void kernfs_destroy_root(struct kernfs_root *root) { } > +static inline unsigned int kernfs_root_flags(struct kernfs_node *kn) > +{ return 0; } > > static inline struct kernfs_node * > kernfs_create_dir_ns(struct kernfs_node *parent, const char *name, > diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c > index e28d5f0d20ed0..c9752eb607ec9 100644 > --- a/kernel/cgroup/cgroup-v1.c > +++ b/kernel/cgroup/cgroup-v1.c > @@ -844,7 +844,7 @@ static int cgroup1_rename(struct kernfs_node *kn, struct kernfs_node *new_parent > > if (kernfs_type(kn) != KERNFS_DIR) > return -ENOTDIR; > - if (kn->parent != new_parent) > + if (rcu_access_pointer(kn->__parent) != new_parent) > return -EIO; > > /* > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > index e63d6f3b00470..fc152691a6b42 100644 > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -633,9 +633,22 @@ int cgroup_task_count(const struct cgroup *cgrp) > return count; > } > > +static struct cgroup *kn_priv(struct kernfs_node *kn) > +{ > + struct kernfs_node *parent; > + /* > + * The parent can not be replaced due to KERNFS_ROOT_INVARIANT_PARENT. > + * Therefore it is always safe to dereference this pointer outside of a > + * RCU section. > + */ > + parent = rcu_dereference_check(kn->__parent, > + kernfs_root_flags(kn) & KERNFS_ROOT_INVARIANT_PARENT); > + return parent->priv; > +} > + > struct cgroup_subsys_state *of_css(struct kernfs_open_file *of) > { > - struct cgroup *cgrp = of->kn->parent->priv; > + struct cgroup *cgrp = kn_priv(of->kn); > struct cftype *cft = of_cft(of); > > /* > @@ -1612,7 +1625,7 @@ void cgroup_kn_unlock(struct kernfs_node *kn) > if (kernfs_type(kn) == KERNFS_DIR) > cgrp = kn->priv; > else > - cgrp = kn->parent->priv; > + cgrp = kn_priv(kn); > > cgroup_unlock(); > > @@ -1644,7 +1657,7 @@ struct cgroup *cgroup_kn_lock_live(struct kernfs_node *kn, bool drain_offline) > if (kernfs_type(kn) == KERNFS_DIR) > cgrp = kn->priv; > else > - cgrp = kn->parent->priv; > + cgrp = kn_priv(kn); > > /* > * We're gonna grab cgroup_mutex which nests outside kernfs > @@ -2118,7 +2131,8 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask) > root->kf_root = kernfs_create_root(kf_sops, > KERNFS_ROOT_CREATE_DEACTIVATED | > KERNFS_ROOT_SUPPORT_EXPORTOP | > - KERNFS_ROOT_SUPPORT_USER_XATTR, > + KERNFS_ROOT_SUPPORT_USER_XATTR | > + KERNFS_ROOT_INVARIANT_PARENT, > root_cgrp); > if (IS_ERR(root->kf_root)) { > ret = PTR_ERR(root->kf_root); > @@ -4144,7 +4158,7 @@ static ssize_t cgroup_file_write(struct kernfs_open_file *of, char *buf, > size_t nbytes, loff_t off) > { > struct cgroup_file_ctx *ctx = of->priv; > - struct cgroup *cgrp = of->kn->parent->priv; > + struct cgroup *cgrp = kn_priv(of->kn); > struct cftype *cft = of_cft(of); > struct cgroup_subsys_state *css; > int ret; > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h > index 8bd1ebd7d6afd..813143b4985dc 100644 > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h > @@ -223,7 +223,7 @@ static INLINE void* read_full_cgroup_path(struct kernfs_node* cgroup_node, > if (bpf_cmp_likely(filepart_length, <=, MAX_PATH)) { > payload += filepart_length; > } > - cgroup_node = BPF_CORE_READ(cgroup_node, parent); > + cgroup_node = BPF_CORE_READ(cgroup_node, __parent); > } > return payload; > } Hi Sasha, I just stumbled over this issue on a 6.12 stable kernel, any reason this patch wasn't applied back then? Can we apply the backport now? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent. 2026-11-13 12:30 ` Christian Loehle @ 2026-11-13 19:10 ` Sasha Levin 0 siblings, 0 replies; 6+ messages in thread From: Sasha Levin @ 2026-11-13 19:10 UTC (permalink / raw) To: linux-kernel, stable Cc: Sasha Levin, Sebastian Andrzej Siewior, Tejun Heo, Yonghong Song, Greg Kroah-Hartman, reinette.chatre, tglx, mingo, bp, dave.hansen, x86, rafael, dakr, hannes, mkoutny, ast, daniel, andrii, eddyz87, shuah, jolsa, alan.maguire, cgroups, bpf, linux-kselftest, Christian Loehle On 5/5/25 23:39, Sasha Levin wrote: > Subject: [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent. On Fri, Nov 13, 2026, Christian Loehle wrote: > I just stumbled over this issue on a 6.12 stable kernel, > any reason this patch wasn't applied back then? > Can we apply the backport now? 633488947ef6 ("kernfs: Use RCU to access kernfs_node::parent.") is a preparatory refactor with no Fixes: or Cc: stable. It is part of a series whose other patches are not in 6.12 either, and it has fixups upstream, so it is not a clean backport. What failure are you seeing on 6.12 (oops, lockdep splat, trace)? If you can send it, we can look for the fix that addresses it directly. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-13 19:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250505223922.2682012-1-sashal@kernel.org>
2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 059/486] cgroup/rstat: avoid disabling irqs for O(num_cpu) Sasha Levin
2025-05-05 22:32 ` [PATCH AUTOSEL 6.12 081/486] blk-cgroup: improve policy registration error handling Sasha Levin
2025-05-05 22:34 ` [PATCH AUTOSEL 6.12 194/486] blk-throttle: don't take carryover for prioritized processing of metadata Sasha Levin
2025-05-05 22:39 ` [PATCH AUTOSEL 6.12 486/486] kernfs: Use RCU to access kernfs_node::parent Sasha Levin
2026-11-13 12:30 ` Christian Loehle
2026-11-13 19:10 ` Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox