[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1be5f15e-a9eb-b6c1-81fa-5b8322ebb229@samsung.com>
Date: Wed, 9 Mar 2022 00:31:06 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Chengming Zhou <zhouchengming@...edance.com>, mingo@...hat.com,
peterz@...radead.org, vincent.guittot@...aro.org,
bristot@...hat.com, zhaolei@...fujitsu.com, tj@...nel.org,
lizefan.x@...edance.com, hannes@...xchg.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] sched/cpuacct: remove redundant RCU read lock
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@...nel.org>
> Signed-off-by: Chengming Zhou <zhouchengming@...edance.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
Powered by blists - more mailing lists