Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <bd9f8db0-64d3-433b-502a-f83845f932fe@bytedance.com>
Date:   Wed, 23 Mar 2022 20:58:03 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [External] Re: [PATCH v2 2/6] perf/core: Introduce percpu
 perf_cgroup
Content-Language: en-US
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
References: <20220322120834.98637-1-zhouchengming@bytedance.com>
 <20220322120834.98637-3-zhouchengming@bytedance.com>
 <YjnIpn/PCM6wKpOC@hirez.programming.kicks-ass.net>
 <0a7ebd49-cfdd-911e-3982-44f594320b0f@bytedance.com>
 <YjrWxnnIZvn3PWoX@hirez.programming.kicks-ass.net>
From:   Chengming Zhou <zhouchengming@bytedance.com>
In-Reply-To: <YjrWxnnIZvn3PWoX@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2022/3/23 4:13 下午, Peter Zijlstra wrote:
> On Wed, Mar 23, 2022 at 12:33:51AM +0800, Chengming Zhou wrote:
>> 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.
> 
> No, the problem is that you're talking about cgroup attach while I'm
> thinking of attaching a event to a task. And your picture has nothing to
> clarify.
> 
> Those pictures of yours could really do with a few more function names
> in them, otherwise it's absolutely unclear what code is running where.

Sorry for the confusion ;-)
I will draw a better picture including more function names in the next version.

Thanks.