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

[Peter Zijlstra]

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?