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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240412073017.GE30852@noisy.programming.kicks-ass.net>
Date: Fri, 12 Apr 2024 09:30:17 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Kuan-Wei Chiu <visitorckw@...il.com>
Cc: colyli@...e.de, kent.overstreet@...ux.dev, msakai@...hat.com,
	mingo@...hat.com, acme@...nel.org, namhyung@...nel.org,
	akpm@...ux-foundation.org, bfoster@...hat.com, mark.rutland@....com,
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
	irogers@...gle.com, adrian.hunter@...el.com, jserv@...s.ncku.edu.tw,
	linux-bcache@...r.kernel.org, linux-kernel@...r.kernel.org,
	dm-devel@...ts.linux.dev, linux-bcachefs@...r.kernel.org,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v3 04/17] lib min_heap: Add type safe interface

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?