Messages in this thread Patch in this message | | | Date | Wed, 9 Mar 2022 00:31:06 +0100 | Subject | Re: [PATCH v3 3/3] sched/cpuacct: remove redundant RCU read lock | From | Marek Szyprowski <> |
| |
On 20.02.2022 06:14, Chengming Zhou wrote: > The cpuacct_account_field() and it's cgroup v2 wrapper > cgroup_account_cputime_field() is only called from cputime > in task_group_account_field(), which is already in RCU read-side > critical section. So remove these redundant RCU read lock. > > Suggested-by: Tejun Heo <tj@kernel.org> > Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
This patch landed recently in linux-next as commit 3eba0505d03a ("sched/cpuacct: Remove redundant RCU read lock"). On my test systems I found that it triggers a following warning in the early boot stage:
Calibrating delay loop (skipped), value calculated using timer frequency.. 48.00 BogoMIPS (lpj=240000) pid_max: default: 32768 minimum: 301 Mount-cache hash table entries: 2048 (order: 1, 8192 bytes, linear) Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes, linear) CPU: Testing write buffer coherency: ok CPU0: Spectre v2: using BPIALL workaround CPU0: thread -1, cpu 0, socket 9, mpidr 80000900 cblist_init_generic: Setting adjustable number of callback queues.
============================= WARNING: suspicious RCU usage 5.17.0-rc5-00052-g167ee9b08bed #11459 Not tainted ----------------------------- ./include/linux/cgroup.h:481 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1 no locks held by swapper/0/1.
stack backtrace: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc5-00052-g167ee9b08bed #11459 Hardware name: Samsung Exynos (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x58/0x70 dump_stack_lvl from account_system_index_time+0x13c/0x148 account_system_index_time from account_system_time+0x78/0xa0 account_system_time from update_process_times+0x50/0xc4 update_process_times from tick_handle_periodic+0x1c/0x94 tick_handle_periodic from exynos4_mct_tick_isr+0x30/0x38 exynos4_mct_tick_isr from __handle_irq_event_percpu+0x74/0x3ac __handle_irq_event_percpu from handle_irq_event_percpu+0xc/0x38 handle_irq_event_percpu from handle_irq_event+0x38/0x5c handle_irq_event from handle_fasteoi_irq+0xc4/0x180 handle_fasteoi_irq from generic_handle_domain_irq+0x44/0x88 generic_handle_domain_irq from gic_handle_irq+0x88/0xac gic_handle_irq from generic_handle_arch_irq+0x58/0x78 generic_handle_arch_irq from __irq_svc+0x54/0x88 Exception stack(0xc1cafea0 to 0xc1cafee8) fea0: 00000000 c0f33ac4 00000001 00000129 60000053 c127b458 00000008 2e64e000 fec0: ef7bd5fc c127af84 c1209048 c1208f10 00000000 c1cafef0 c0b7e25c c0b8976c fee0: 20000053 ffffffff __irq_svc from _raw_spin_unlock_irqrestore+0x2c/0x60 _raw_spin_unlock_irqrestore from cblist_init_generic.constprop.14+0x298/0x328 cblist_init_generic.constprop.14 from rcu_init_tasks_generic+0x18/0x110 rcu_init_tasks_generic from kernel_init_freeable+0xa0/0x214 kernel_init_freeable from kernel_init+0x18/0x12c kernel_init from ret_from_fork+0x14/0x2c Exception stack(0xc1caffb0 to 0xc1cafff8) ffa0: 00000000 00000000 00000000 00000000 ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 cblist_init_generic: Setting shift to 1 and lim to 1. Running RCU-tasks wait API self tests Setting up static identity map for 0x40100000 - 0x40100060 rcu: Hierarchical SRCU implementation. smp: Bringing up secondary CPUs ... CPU1: thread -1, cpu 1, socket 9, mpidr 80000901 CPU1: Spectre v2: using BPIALL workaround smp: Brought up 1 node, 2 CPUs SMP: Total of 2 processors activated (96.00 BogoMIPS). CPU: All CPU(s) started in SVC mode.
The above log comes from ARM 32bit Samsung Exnyos4210 based Trats board. To get above log I reverted commit dc6e0818bc9a ("sched/cpuacct: Optimize away RCU read lock"), because it triggered a similar warning.
> --- > include/linux/cgroup.h | 2 -- > kernel/sched/cpuacct.c | 2 -- > 2 files changed, 4 deletions(-) > > diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h > index 9a109c6ac0e0..1e356c222756 100644 > --- a/include/linux/cgroup.h > +++ b/include/linux/cgroup.h > @@ -804,11 +804,9 @@ static inline void cgroup_account_cputime_field(struct task_struct *task, > > cpuacct_account_field(task, index, delta_exec); > > - rcu_read_lock(); > cgrp = task_dfl_cgroup(task); > if (cgroup_parent(cgrp)) > __cgroup_account_cputime_field(cgrp, index, delta_exec); > - rcu_read_unlock(); > } > > #else /* CONFIG_CGROUPS */ > diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c > index f79f88456d72..d269ede84e85 100644 > --- a/kernel/sched/cpuacct.c > +++ b/kernel/sched/cpuacct.c > @@ -352,10 +352,8 @@ void cpuacct_account_field(struct task_struct *tsk, int index, u64 val) > { > struct cpuacct *ca; > > - rcu_read_lock(); > for (ca = task_ca(tsk); ca != &root_cpuacct; ca = parent_ca(ca)) > __this_cpu_add(ca->cpustat->cpustat[index], val); > - rcu_read_unlock(); > } > > struct cgroup_subsys cpuacct_cgrp_subsys = {
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
| |