On Fri, Apr 12, 2024 at 09:30:17AM +0200, Peter Zijlstra wrote:
> On Sun, Apr 07, 2024 at 12:47:14AM +0800, Kuan-Wei Chiu wrote:
>
> > -struct min_heap {
> > - void *data;
> > - int nr;
> > - int size;
> > -};
> > +#define MIN_HEAP_PREALLOCATED(_type, _name, _nr) \
> > +struct _name { \
> > + int nr; \
> > + int size; \
> > + _type *data; \
> > + _type preallocated[_nr]; \
> > +}
>
>
> > @@ -3738,7 +3739,7 @@ static noinline int visit_groups_merge(struct
> > perf_event_context *ctx,
> > struct perf_cpu_context *cpuctx = NULL;
> > /* Space for per CPU and/or any CPU event iterators. */
> > struct perf_event *itrs[2];
> > - struct min_heap event_heap;
> > + struct perf_event_min_heap event_heap;
> > struct perf_event **evt;
> > int ret;
> >
> > @@ -3747,11 +3748,9 @@ static noinline int visit_groups_merge(struct
> > perf_event_context *ctx,
> >
> > if (!ctx->task) {
> > cpuctx = this_cpu_ptr(&perf_cpu_context);
> > - event_heap = (struct min_heap){
> > - .data = cpuctx->heap,
> > - .nr = 0,
> > - .size = cpuctx->heap_size,
> > - };
> > + event_heap.data = cpuctx->heap;
> > + event_heap.nr = 0;
> > + event_heap.size = cpuctx->heap_size;
> >
> > lockdep_assert_held(&cpuctx->ctx.lock);
> >
> > @@ -3760,11 +3759,9 @@ static noinline int visit_groups_merge(struct
> > perf_event_context *ctx,
> > css = &cpuctx->cgrp->css;
> > #endif
> > } else {
> > - event_heap = (struct min_heap){
> > - .data = itrs,
> > - .nr = 0,
> > - .size = ARRAY_SIZE(itrs),
> > - };
> > + event_heap.data = itrs;
> > + event_heap.nr = 0;
> > + event_heap.size = ARRAY_SIZE(itrs);
> > /* Events not within a CPU context may be on any CPU. */
> > __heap_add(&event_heap, perf_event_groups_first(groups, -1,
> > pmu, NULL));
> > }
>
> Not too happy about these. If you ever add more fields they will go
> uninitialized. Why not keep the existing form and fix the struct name?
Sorry for the late reply. I'm traveling.
I'll change that back in v4.
Regards,
Kuan-Wei
