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

[Like Xu]

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 ?