Re: [PATCH v3 04/17] lib min_heap: Add type safe interface

Peter Zijlstra
2024-04-12T07:30:27Z

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?