linux-kernel - Re: [PATCH v2 6/6] perf cpumap: Add range data encoding

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]

Date:   Wed, 29 Jun 2022 11:31:07 +0200
From:   Jiri Olsa <olsajiri@...il.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        James Clark <james.clark@....com>,
        Kees Cook <keescook@...omium.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Riccardo Mancini <rickyman7@...il.com>,
        German Gomez <german.gomez@....com>,
        Colin Ian King <colin.king@...el.com>,
        Song Liu <songliubraving@...com>,
        Dave Marchevsky <davemarchevsky@...com>,
        Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
        Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>,
        Leo Yan <leo.yan@...aro.org>, linux-perf-users@...r.kernel.org,
        linux-kernel@...r.kernel.org, Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2 6/6] perf cpumap: Add range data encoding

On Tue, Jun 14, 2022 at 07:33:53AM -0700, Ian Rogers wrote:

SNIP

> -static size_t cpus_size(const struct perf_cpu_map *map)
> +static void synthesize_range_cpus(struct synthesize_cpu_map_data *data)
>  {
> -	return sizeof(struct cpu_map_entries) + perf_cpu_map__nr(map) * sizeof(u16);
> +	data->data->type = PERF_CPU_MAP__RANGE_CPUS;
> +	data->data->range_cpu_data.any_cpu = data->has_any_cpu;
> +	data->data->range_cpu_data.start_cpu = data->min_cpu;
> +	data->data->range_cpu_data.end_cpu = data->max_cpu;
>  }
>  
> -static size_t mask_size(const struct perf_cpu_map *map, int *max)
> -{
> -	*max = perf_cpu_map__max(map).cpu;
> -	return sizeof(struct perf_record_mask_cpu_map32) + BITS_TO_U32(*max) * sizeof(__u32);
> -}
> -
> -static void *cpu_map_data__alloc(const struct perf_cpu_map *map, size_t *size,
> -				 u16 *type, int *max)
> +static void *cpu_map_data__alloc(struct synthesize_cpu_map_data *syn_data,
> +				 size_t header_size)
>  {
>  	size_t size_cpus, size_mask;
> -	bool is_dummy = perf_cpu_map__empty(map);
>  
> -	/*
> -	 * Both array and mask data have variable size based
> -	 * on the number of cpus and their actual values.
> -	 * The size of the 'struct perf_record_cpu_map_data' is:
> -	 *
> -	 *   array = size of 'struct cpu_map_entries' +
> -	 *           number of cpus * sizeof(u64)
> -	 *
> -	 *   mask  = size of 'struct perf_record_record_cpu_map' +
> -	 *           maximum cpu bit converted to size of longs
> -	 *
> -	 * and finally + the size of 'struct perf_record_cpu_map_data'.
> -	 */
> -	size_cpus = cpus_size(map);
> -	size_mask = mask_size(map, max);
> +	syn_data->nr = perf_cpu_map__nr(syn_data->map);
> +	syn_data->has_any_cpu = (perf_cpu_map__cpu(syn_data->map, 0).cpu == -1) ? 1 : 0;

I'm bit lost in the logic in here.. should it be '.cpu != -1' ?
has_any_cpu is named as bool but used as index below ;-)

could you please keep/update the comment above and explain
the conditions when each cpu_map type is taken

thanks,
jirka

>  
> -	if (is_dummy || (size_cpus < size_mask)) {
> -		*size += size_cpus;
> -		*type  = PERF_CPU_MAP__CPUS;
> -	} else {
> -		*size += size_mask;
> -		*type  = PERF_CPU_MAP__MASK;
> +	syn_data->min_cpu = perf_cpu_map__cpu(syn_data->map, syn_data->has_any_cpu).cpu;
> +	syn_data->max_cpu = perf_cpu_map__max(syn_data->map).cpu;
> +	if (syn_data->max_cpu - syn_data->min_cpu + 1 == syn_data->nr - syn_data->has_any_cpu) {
> +		/* A consecutive range of CPUs can be encoded using a range. */
> +		assert(sizeof(u16) + sizeof(struct perf_record_range_cpu_map) == sizeof(u64));
> +		syn_data->type = PERF_CPU_MAP__RANGE_CPUS;
> +		syn_data->size = header_size + sizeof(u64);
> +		return zalloc(syn_data->size);
>  	}
>  
> -	*size += sizeof(__u16); /* For perf_record_cpu_map_data.type. */
> -	*size = PERF_ALIGN(*size, sizeof(u64));
> -	return zalloc(*size);
> +	size_cpus = sizeof(u16) + sizeof(struct cpu_map_entries) + syn_data->nr * sizeof(u16);
> +	/* Due to padding, the 4bytes per entry mask variant is always smaller. */
> +	size_mask = sizeof(u16) + sizeof(struct perf_record_mask_cpu_map32) +
> +		BITS_TO_U32(syn_data->max_cpu) * sizeof(__u32);
> +	if (syn_data->has_any_cpu || size_cpus < size_mask) {
> +		/* Follow the CPU map encoding. */
> +		syn_data->type = PERF_CPU_MAP__CPUS;
> +		syn_data->size = header_size + PERF_ALIGN(size_cpus, sizeof(u64));
> +		return zalloc(syn_data->size);
> +	}
> +	/* Encode using a bitmask. */
> +	syn_data->type = PERF_CPU_MAP__MASK;
> +	syn_data->size = header_size + PERF_ALIGN(size_mask, sizeof(u64));
> +	return zalloc(syn_data->size);

SNIP