linux-kernel - Re: [PATCH 5/9] perf utils: add support for arch standard events

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: <77588a26-e216-b44d-fc9e-cea6c05444df@huawei.com>
Date:   Thu, 8 Feb 2018 14:45:37 +0000
From:   John Garry <john.garry@...wei.com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <alexander.shishkin@...ux.intel.com>, <namhyung@...nel.org>,
        <ak@...ux.intel.com>, <wcohen@...hat.com>, <will.deacon@....com>,
        <ganapatrao.kulkarni@...ium.com>, <linux-kernel@...r.kernel.org>,
        <linuxarm@...wei.com>, <zhangshaokun@...ilicon.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 5/9] perf utils: add support for arch standard events

On 08/02/2018 13:55, Jiri Olsa wrote:
> On Wed, Feb 07, 2018 at 01:45:00AM +0800, John Garry wrote:
>
> SNIP
>
>> +	char *perpkg;
>> +	char *unit;
>> +	char *metric_expr;
>> +	char *metric_name;
>> +	char *metric_group;
>> +	struct list_head list;
>> +	char strings[];
>> +};
>> +
>> +static LIST_HEAD(arch_std_events);
>> +
>> +#define ADD_EVENT_STRING(string) do { if (string) {		\
>> +	es->string = strings;					\
>> +	strings += snprintf(strings, len, "%s", string) + 1;	\
>> +} } while (0)
>> +
>> +static int save_arch_std_events(void *data, char *name, char *event,
>> +				char *desc, char *long_desc, char *pmu,
>> +				char *unit, char *perpkg, char *metric_expr,
>> +				char *metric_name, char *metric_group)
>> +{
>> +	struct event_struct *es;
>> +	struct stat *sb = data;
>> +	int len;
>> +	char *strings;
>> +
>> +	/*
>> +	 * Lazily allocate size of the json file to hold the
>> +	 * strings, which would be more than large enough.
>> +	 */
>> +	len = sb->st_size;
>> +
>> +	es = malloc(sizeof(*es) + len);
>
> hum, so for single event you allocate buffer of the size
> of the entire file this event is defined in?
>
> what do I miss? I assume there're more of those arch-defined
> events defined in the single file..

Hi Jirka,

Yes, allocating the file size per event was just to make the code more 
concise (instead of finding each string length), but obviously it is an 
inefficient practice in terms of memory usage.

But since the JSONs are generally not huge, and in practice we would 
only be accessing a fraction of the buffer's physical memory to save the 
event strings, I thought it ok.

Anyway, I'll see if there is something more efficient I can do.

Thanks,
John

>
> jirka
>
> .
>