Re: [PATCH RFC 1/8] perf/core: Add *group_leader to perf_event_create_kernel_counter()

Like Xu
2022-12-15T13:36:33Z

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 ?