Re: [External] Re: [PATCH v2 2/6] perf/core: Introduce percpu perf_cgroup

From: Chengming Zhou <zhouchengming@bytedance.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, acme@kernel.org, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	namhyung@kernel.org, eranian@google.com,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	duanxiongchun@bytedance.com, songmuchun@bytedance.com
Subject: Re: [External] Re: [PATCH v2 2/6] perf/core: Introduce percpu perf_cgroup
Date: Wed, 23 Mar 2022 00:33:51 +0800	[thread overview]
Message-ID: <0a7ebd49-cfdd-911e-3982-44f594320b0f@bytedance.com> (raw)
In-Reply-To: <YjnIpn/PCM6wKpOC@hirez.programming.kicks-ass.net>

On 2022/3/22 9:01 下午, Peter Zijlstra wrote:
> On Tue, Mar 22, 2022 at 08:08:30PM +0800, Chengming Zhou wrote:
>> Although we don't have incosistency problem any more, we can
>> have other problem like:
>>
>> CPU1					CPU2
>> (in context_switch)			(attach running task)
>> 					prev->cgroups = cgrp2
>> perf_cgroup_sched_switch(prev, next)
>> 	cgrp2 == cgrp2 is True
>>
> 
> Again, I'm not following, how can you attach to a running task from
> another CPU ?

Hi Peter, I make a little testcase which can reproduce the race
problem, on system with PSI disabled. Because when PSI enabled,
cgroup_move_task() will hold rq lock to assign task->cgroups.

```
#!/bin/bash

cd /sys/fs/cgroup/perf_event

mkdir cg1
mkdir cg2

perf stat -e cycles --cgroup /cg1 &

cg_run()
{
        cg=$1
        shift
        echo $BASHPID > $cg/cgroup.procs
        $@
}

cg_run cg1 schbench -r 100 &
cg_run cg2 schbench -r 100 &

while true; do
        for i in $(cat cg1/cgroup.procs); do
                echo $i > cg2/cgroup.procs
        done
        for i in $(cat cg2/cgroup.procs); do
                echo $i > cg1/cgroup.procs
        done
done
```

Some seconds later, dmesg will show the WARNING message:

[   51.777830] WARNING: CPU: 2 PID: 1849 at kernel/events/core.c:869 perf_cgroup_switch+0x246/0x290
[   51.779167] Modules linked in:
[   51.779696] CPU: 2 PID: 1849 Comm: schbench Not tainted 5.17.0-rc8 #28
[   51.780691] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
[   51.782353] RIP: 0010:perf_cgroup_switch+0x246/0x290
[   51.783145] Code: 0f 0b e9 0b ff ff ff 48 83 7c 24 08 00 74 0c e8 00 7e f7 ff fb 66 0f 1f 44 00 00 48 83 c4 10 5b 5d 41 5c 41 5d 41 5e 41 5f c3 <0f> 0b e9 4f fe ff ff e8 be 7e f7 ff e8 a9 1f 93 00 89 c0 49 c7 c5
[   51.785804] RSP: 0018:ffffba4440fcbd80 EFLAGS: 00010086
[   51.786617] RAX: 0000000000000002 RBX: ffff8d78eb8b7200 RCX: 0000000000000000
[   51.787696] RDX: 0000000000000000 RSI: ffffffffae1c83db RDI: ffffffffae185a69
[   51.788777] RBP: ffff8d78eb8aad40 R08: 0000000000000001 R09: 0000000000000001
[   51.789854] R10: 0000000000000000 R11: ffff8d78eb8b7220 R12: ffff8d78eb8b7208
[   51.790929] R13: ffff8d78eb8aafa0 R14: ffff8d74cd6bb600 R15: 0000000000000000
[   51.792006] FS:  00007fedaaffd700(0000) GS:ffff8d78eb880000(0000) knlGS:0000000000000000
[   51.793223] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   51.794122] CR2: 000055e4bf2b696c CR3: 00000001128a2003 CR4: 0000000000370ee0
[   51.795209] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   51.796292] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   51.797375] Call Trace:
[   51.797828]  <TASK>
[   51.798229]  __perf_event_task_sched_in+0x151/0x350
[   51.799009]  ? lock_release+0x1ed/0x2e0
[   51.799640]  finish_task_switch+0x1d3/0x2e0
[   51.800328]  ? __switch_to+0x136/0x4b0
[   51.800953]  __schedule+0x33e/0xae0
[   51.801535]  schedule+0x4e/0xc0
[   51.802080]  exit_to_user_mode_prepare+0x172/0x2a0
[   51.802850]  ? asm_sysvec_apic_timer_interrupt+0xa/0x20
[   51.803675]  irqentry_exit_to_user_mode+0x5/0x40
[   51.804413]  sysvec_apic_timer_interrupt+0x5c/0xd0
[   51.805183]  asm_sysvec_apic_timer_interrupt+0x12/0x20

Thanks.