Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <90b640af-1d81-64bf-96f7-24aa44cca2dd@gmail.com>
Date:   Thu, 15 Dec 2022 21:36:19 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.5.1
Subject: Re: [PATCH RFC 1/8] perf/core: Add *group_leader to
 perf_event_create_kernel_counter()
Content-Language: en-US
To:     Ravi Bangoria <ravi.bangoria@amd.com>,
        Peter Zijlstra <peterz@infradead.org>
Cc:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Marc Zyngier <maz@kernel.org>,
        Fenghua Yu <fenghua.yu@intel.com>, kvmarm@lists.linux.dev,
        linux-perf-users@vger.kernel.org
References: <20221212125844.41157-1-likexu@tencent.com>
 <20221212125844.41157-2-likexu@tencent.com>
 <4990ef71-734a-10cf-e7dc-51b33b26fd18@amd.com>
From:   Like Xu <like.xu.linux@gmail.com>
In-Reply-To: <4990ef71-734a-10cf-e7dc-51b33b26fd18@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 14/12/2022 11:52 am, Ravi Bangoria wrote:
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index 7f04f995c975..f671b1a9a691 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -12674,12 +12674,14 @@ SYSCALL_DEFINE5(perf_event_open,
>>    * @attr: attributes of the counter to create
>>    * @cpu: cpu in which the counter is bound
>>    * @task: task to profile (NULL for percpu)
>> + * @group_leader: event group leader
>>    * @overflow_handler: callback to trigger when we hit the event
>>    * @context: context data could be used in overflow_handler callback
>>    */
>>   struct perf_event *
>>   perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,
>>   				 struct task_struct *task,
>> +				 struct perf_event *group_leader,
>>   				 perf_overflow_handler_t overflow_handler,
>>   				 void *context)
>>   {
>> @@ -12694,7 +12696,7 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr, int cpu,
>>   	if (attr->aux_output)
>>   		return ERR_PTR(-EINVAL);
>>   
>> -	event = perf_event_alloc(attr, cpu, task, NULL, NULL,
>> +	event = perf_event_alloc(attr, cpu, task, group_leader, NULL,
>>   				 overflow_handler, context, -1);
> 
> Grouping involves lot of complexities. Setting group_leader won't be sufficient.
> Please see perf_event_open() syscall code for more detail.
> 
> Thanks,
> Ravi

This is the main reason why the RFC tag is added. More detailed professional 
reviews is encouraged.

AFAI, there does exist a number of code gaps here to support grouped events in 
the kernel,
but there are also opportunities, as there may be other new use cases bringing 
innovation.

I have to confirm this idea with maintainers first, the alternative is to create 
yet another special
perf_event similar to PMC_IDX_FIXED_VLBR, which schedules perf_metrics MSR for 
KVM stuff.

PeterZ, any input ?