[patch] perf_event_open enable sysfs exported events

[patch] perf_event_open enable sysfs exported events

[Vince Weaver]

perf_event exports a bunch of event info via /sys under
/sys/bus/event_source/devices

By parsing these files it's possible to generate more likely valid
(and almost-valid) events.  It also makes exercising expanded PMUs
(such as uncore events on recent Intel machines) possible.

The included somewhat ugly patch enables generating events based
on the sysfs files (1 time out of 9).  It sets up a table of all
possible event fields and generalized events for all detected event
types the first time such an event is requested, then uses this table
to generate events subsequent times.

The parser is possibly fragile.  The perf tool actually has a full
lexer to handle the parsing of these files.  I've tested on a core2
and sandybridge machine but it might possibly fail on more obscure
machines, especially if any have non-contiguous event fields.

Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>

diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index 5250d1e..32c11cf 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -6,6 +6,9 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <errno.h>
 #include "perf_event.h"
 #include "random.h"
 #include "sanitise.h"
@@ -13,6 +16,442 @@
 #include "maps.h"
 #include "shm.h"
 
+
+struct generic_event_type {
+	char *name;
+	char *value;
+	long long config;
+	long long config1;
+};
+
+struct format_type {
+	char *name;
+	char *value;
+	int field;
+	int bits;
+	unsigned long long  mask;
+	int shift;
+};
+
+struct pmu_type {
+	char *name;
+	int type;
+	int num_formats;
+	int num_generic_events;
+	struct format_type *formats;
+	struct generic_event_type *generic_events;
+};
+
+static int num_pmus=0;
+
+static struct pmu_type *pmus=NULL;
+
+
+#define FIELD_UNKNOWN	0
+#define FIELD_CONFIG	1
+#define FIELD_CONFIG1	2
+
+#define MAX_FIELDS	3
+
+
+static int parse_format(char *string, int *field_type, int *shift, int *bits) {
+
+	int i,firstnum,secondnum;
+	char format_string[BUFSIZ];
+
+	/* get format */
+	i=0;
+	while(1) {
+		format_string[i]=string[i];
+		if (string[i]==':') {
+			format_string[i]=0;
+			break;
+		}
+		if (string[i]==0) break;
+		i++;
+	}
+
+	if (!strcmp(format_string,"config")) {
+		*field_type=FIELD_CONFIG;
+	} else if (!strcmp(format_string,"config1")) {
+		*field_type=FIELD_CONFIG1;
+	} else {
+		*field_type=FIELD_UNKNOWN;
+	}
+
+	/* Read first number */
+	i++;
+	firstnum=0;
+	while(1) {
+		if (string[i]==0) break;
+		if (string[i]=='-') break;
+		if ((string[i]<'0') || (string[i]>'9')) {
+			fprintf(stderr,"Unknown format char %c\n",string[i]);
+			return -1;
+		}
+		firstnum*=10;
+		firstnum+=(string[i])-'0';
+		i++;
+	}
+	*shift=firstnum;
+
+	/* check if no second num */
+	if (string[i]==0) {
+		*bits=1;
+		return 0;
+	}
+
+	/* Read second number */
+	i++;
+	secondnum=0;
+	while(1) {
+		if (string[i]==0) break;
+		if (string[i]=='-') break;
+		if ((string[i]<'0') || (string[i]>'9')) {
+			fprintf(stderr,"Unknown format char %c\n",string[i]);
+			return -1;
+		}
+		secondnum*=10;
+		secondnum+=(string[i])-'0';
+		i++;
+	}
+
+	*bits=(secondnum-firstnum)+1;
+
+	return 0;
+}
+
+static int update_configs(int pmu, char *field,
+			long long value, long long *c, long long *c1) {
+
+	int i;
+
+	for(i=0;i<pmus[pmu].num_formats;i++) {
+		if (!strcmp(field,pmus[pmu].formats[i].name)) {
+			if (pmus[pmu].formats[i].field==FIELD_CONFIG) {
+				*c|=( (value&pmus[pmu].formats[i].mask)
+					<<pmus[pmu].formats[i].shift);
+				return 0;
+			}
+
+			if (pmus[pmu].formats[i].field==FIELD_CONFIG1) {
+				*c1|=( (value&pmus[pmu].formats[i].mask)
+					<<pmus[pmu].formats[i].shift);
+				return 0;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int parse_generic(int pmu,char *value, long long *config, long long *config1) {
+
+	long long c=0,c1=0,temp;
+	char field[BUFSIZ];
+	int i,ptr=0;
+
+	while(1) {
+		i=0;
+		while(1) {
+			field[i]=value[ptr];
+			if (value[ptr]==0) break;
+			if ((value[ptr]=='=') || (value[ptr]==',')) {
+				field[i]=0;
+				break;
+			}
+			i++;
+			ptr++;
+		}
+
+		/* if at end, was parameter w/o value */
+		if ((value[ptr]==',') || (value[ptr]==0)) {
+			temp=0x1;
+		}
+		else {
+			/* get number */
+
+			ptr++;
+
+			if (value[ptr]!='0') fprintf(stderr,"Expected 0x\n");
+			ptr++;
+			if (value[ptr]!='x') fprintf(stderr,"Expected 0x\n");
+			ptr++;
+			temp=0x0;
+			while(1) {
+
+				if (value[ptr]==0) break;
+				if (value[ptr]==',') break;
+				if (! ( ((value[ptr]>='0') && (value[ptr]<='9'))
+                   			|| ((value[ptr]>='a') && (value[ptr]<='f'))) ) {
+					fprintf(stderr,"Unexpected char %c\n",value[ptr]);
+				}
+				temp*=16;
+				if ((value[ptr]>='0') && (value[ptr]<='9')) {
+					temp+=value[ptr]-'0';
+				}
+				else {
+					temp+=(value[ptr]-'a')+10;
+				}
+				i++;
+				ptr++;
+			}
+		}
+		update_configs(pmu,field,temp,&c,&c1);
+		if (value[ptr]==0) break;
+		ptr++;
+	}
+	*config=c;
+	*config1=c1;
+	return 0;
+}
+
+
+static int init_pmus(void) {
+
+	DIR *dir,*event_dir,*format_dir;
+	struct dirent *entry,*event_entry,*format_entry;
+	char dir_name[BUFSIZ],event_name[BUFSIZ],event_value[BUFSIZ],
+		temp_name[BUFSIZ],format_name[BUFSIZ],format_value[BUFSIZ];
+	int type,pmu_num=0,format_num=0,generic_num=0;
+	FILE *fff;
+	int result;
+
+
+	/* Count number of PMUs */
+	/* This may break if PMUs are ever added/removed on the fly? */
+
+	dir=opendir("/sys/bus/event_source/devices");
+	if (dir==NULL) {
+		fprintf(stderr,"Unable to opendir "
+			"/sys/bus/event_source/devices : %s\n",
+			strerror(errno));
+		return -1;
+	}
+
+	while(1) {
+		entry=readdir(dir);
+		if (entry==NULL) break;
+		if (!strcmp(".",entry->d_name)) continue;
+		if (!strcmp("..",entry->d_name)) continue;
+		num_pmus++;
+	}
+
+	if (num_pmus<1) return -1;
+
+	pmus=calloc(num_pmus,sizeof(struct pmu_type));
+	if (pmus==NULL) {
+		return -1;
+	}
+
+	/****************/
+	/* Add each PMU */
+	/****************/
+
+	rewinddir(dir);
+
+	while(1) {
+		entry=readdir(dir);
+		if (entry==NULL) break;
+		if (!strcmp(".",entry->d_name)) continue;
+		if (!strcmp("..",entry->d_name)) continue;
+
+		/* read name */
+		pmus[pmu_num].name=strdup(entry->d_name);
+		sprintf(dir_name,"/sys/bus/event_source/devices/%s",
+			entry->d_name);
+
+		/* read type */
+		sprintf(temp_name,"%s/type",dir_name);
+		fff=fopen(temp_name,"r");
+		if (fff==NULL) {
+		}
+		else {
+			result=fscanf(fff,"%d",&type);
+			pmus[pmu_num].type=type;
+			fclose(fff);
+		}
+
+		/***********************/
+		/* Scan format strings */
+		/***********************/
+		sprintf(format_name,"%s/format",dir_name);
+		format_dir=opendir(format_name);
+		if (format_dir==NULL) {
+			/* Can be normal to have no format strings */
+		}
+		else {
+			/* Count format strings */
+			while(1) {
+				format_entry=readdir(format_dir);
+				if (format_entry==NULL) break;
+				if (!strcmp(".",format_entry->d_name)) continue;
+				if (!strcmp("..",format_entry->d_name)) continue;
+				pmus[pmu_num].num_formats++;
+			}
+
+			/* Allocate format structure */
+			pmus[pmu_num].formats=calloc(pmus[pmu_num].num_formats,
+							sizeof(struct format_type));
+			if (pmus[pmu_num].formats==NULL) {
+				pmus[pmu_num].num_formats=0;
+				return -1;
+			}
+
+			/* Read format string info */
+			rewinddir(format_dir);
+			format_num=0;
+			while(1) {
+				format_entry=readdir(format_dir);
+
+				if (format_entry==NULL) break;
+				if (!strcmp(".",format_entry->d_name)) continue;
+				if (!strcmp("..",format_entry->d_name)) continue;
+
+				pmus[pmu_num].formats[format_num].name=
+					strdup(format_entry->d_name);
+				sprintf(temp_name,"%s/format/%s",
+					dir_name,format_entry->d_name);
+				fff=fopen(temp_name,"r");
+				if (fff!=NULL) {
+					result=fscanf(fff,"%s",format_value);
+					pmus[pmu_num].formats[format_num].value=
+						strdup(format_value);
+					fclose(fff);
+				}
+				parse_format(format_value,
+						&pmus[pmu_num].formats[format_num].field,
+						&pmus[pmu_num].formats[format_num].shift,
+						&pmus[pmu_num].formats[format_num].bits);
+				if (pmus[pmu_num].formats[format_num].bits==64) {
+					pmus[pmu_num].formats[format_num].mask=0xffffffffffffffffULL;
+				} else {
+					pmus[pmu_num].formats[format_num].mask=
+						(1ULL<<pmus[pmu_num].formats[format_num].bits)-1;
+				}
+				format_num++;
+			}
+			closedir(format_dir);
+		}
+
+		/***********************/
+		/* Scan generic events */
+		/***********************/
+		sprintf(event_name,"%s/events",dir_name);
+		event_dir=opendir(event_name);
+		if (event_dir==NULL) {
+			/* It's sometimes normal to have no generic events */
+		}
+		else {
+
+			/* Count generic events */
+			while(1) {
+				event_entry=readdir(event_dir);
+				if (event_entry==NULL) break;
+				if (!strcmp(".",event_entry->d_name)) continue;
+				if (!strcmp("..",event_entry->d_name)) continue;
+				pmus[pmu_num].num_generic_events++;
+			}
+
+			/* Allocate generic events */
+			pmus[pmu_num].generic_events=calloc(
+				pmus[pmu_num].num_generic_events,
+				sizeof(struct generic_event_type));
+			if (pmus[pmu_num].generic_events==NULL) {
+				pmus[pmu_num].num_generic_events=0;
+				return -1;
+			}
+
+			/* Read in generic events */
+			rewinddir(event_dir);
+			generic_num=0;
+			while(1) {
+				event_entry=readdir(event_dir);
+				if (event_entry==NULL) break;
+				if (!strcmp(".",event_entry->d_name)) continue;
+				if (!strcmp("..",event_entry->d_name)) continue;
+
+				pmus[pmu_num].generic_events[generic_num].name=
+					strdup(event_entry->d_name);
+				sprintf(temp_name,"%s/events/%s",
+					dir_name,event_entry->d_name);
+				fff=fopen(temp_name,"r");
+				if (fff!=NULL) {
+					result=fscanf(fff,"%s",event_value);
+					pmus[pmu_num].generic_events[generic_num].value=
+						strdup(event_value);
+					fclose(fff);
+				}
+				parse_generic(pmu_num,event_value,
+						&pmus[pmu_num].generic_events[generic_num].config,
+						&pmus[pmu_num].generic_events[generic_num].config1);
+				generic_num++;
+			}
+			closedir(event_dir);
+		}
+		pmu_num++;
+	}
+
+	closedir(dir);
+
+	(void)result;
+
+	return 0;
+
+}
+
+
+static long long random_sysfs_config(__u32 *type, __u64 *config1) {
+
+	int i,j;
+	long long c=0,c1=0;
+
+	i=rand()%num_pmus;
+
+	*type=pmus[i].type;
+
+	switch(rand()%3) {
+		/* Random by Format */
+		case 0:
+			if (pmus[i].num_formats==0) goto out;
+			for(j=0;j<pmus[i].num_formats;j++) {
+				/* 50% chance of having field set */
+				if (rand_bool()) {
+					if (pmus[i].formats[j].field==FIELD_CONFIG) {
+						c|=(rand64()&pmus[i].formats[j].mask)<<
+							pmus[i].formats[j].shift;
+					} else {
+						c1|=(rand64()&pmus[i].formats[j].mask)<<
+							pmus[i].formats[j].shift;
+					}
+				}
+			}
+			break;
+
+
+		/* Random by generic event */
+		case 1:
+			if (pmus[i].num_generic_events==0) goto out;
+			j=rand()%pmus[i].num_generic_events;
+			c=pmus[i].generic_events[j].config;
+			c1=pmus[i].generic_events[j].config1;
+			break;
+
+		default:
+			goto out;
+			break;
+	}
+	*config1=c1;
+	return c;
+out:
+	*config1=rand()%64;
+	return rand()%64;
+}
+
+/* arbitrary high number unlikely to be used by perf_event */
+#define PERF_TYPE_READ_FROM_SYSFS 1027
+
+
 static long long random_cache_config(void)
 {
 
@@ -80,7 +519,7 @@ static int random_event_type(void)
 
 	int type;
 
-	switch (rand() % 6) {
+	switch (rand() % 8) {
 	case 0:
 		type = PERF_TYPE_HARDWARE;
 		break;
@@ -99,6 +538,9 @@ static int random_event_type(void)
 	case 5:
 		type = PERF_TYPE_BREAKPOINT;
 		break;
+	case 6:
+		type = PERF_TYPE_READ_FROM_SYSFS;
+		break;
 	default:
 		type = rand();
 		break;
@@ -106,11 +548,11 @@ static int random_event_type(void)
 	return type;
 }
 
-static long long random_event_config(long long event_type)
+static long long random_event_config(__u32 *event_type, __u64 *config1)
 {
 	unsigned long long config;
 
-	switch (event_type) {
+	switch (*event_type) {
 	case PERF_TYPE_HARDWARE:
 		switch (rand() % 11) {
 		case 0:
@@ -205,8 +647,10 @@ static long long random_event_config(long long event_type)
 			config = 0;
 		break;
 
-/* FIXME: value can also be one of the ones found in */
-/* /sys/bus/event_source/devices                     */
+	case PERF_TYPE_READ_FROM_SYSFS:
+		if (pmus==NULL) init_pmus();
+		config = random_sysfs_config(event_type,config1);
+		break;
 
 	default:
 		config = rand64();
@@ -329,10 +773,10 @@ static void create_mostly_valid_counting_event(struct perf_event_attr *attr)
 	attr->type = random_event_type();
 
 	attr->size = sizeof(struct perf_event_attr);
-	/* FIXME: can typically be 64,72,80,96 depending on kernel */
+	/* FIXME: can typically be 64,72,80,or 96 depending on kernel */
 	/* Other values will likely not create a valid event       */
 
-	attr->config = random_event_config(attr->type);
+	attr->config = random_event_config(&attr->type,&attr->config1);
 	if (attr->type == PERF_TYPE_BREAKPOINT) {
 		setup_breakpoints(attr);
 	}
@@ -379,7 +823,7 @@ static void create_mostly_valid_sampling_event(struct perf_event_attr *attr)
 
 	attr->type = random_event_type();
 	attr->size = sizeof(struct perf_event_attr);
-	attr->config = random_event_config(attr->type);
+	attr->config = random_event_config(&attr->type,&attr->config1);
 	if (attr->type == PERF_TYPE_BREAKPOINT) {
 		setup_breakpoints(attr);
 	}
@@ -427,7 +871,7 @@ static void create_random_event(struct perf_event_attr *attr)
 {
 
 	attr->type = random_event_type();
-	attr->config = random_event_config(attr->type);
+	attr->config = random_event_config(&attr->type,&attr->config1);
 	setup_breakpoints(attr);
 
 	switch (rand_bool()) {

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Tue, Jun 11, 2013 at 12:33:23AM -0400, Vince Weaver wrote:

 > @@ -80,7 +519,7 @@ static int random_event_type(void)
 >  
 >  	int type;
 >  
 > -	switch (rand() % 6) {
 > +	switch (rand() % 8) {
 >  	case 0:
 >  		type = PERF_TYPE_HARDWARE;
 >  		break;
 > @@ -99,6 +538,9 @@ static int random_event_type(void)
 >  	case 5:
 >  		type = PERF_TYPE_BREAKPOINT;
 >  		break;
 > +	case 6:
 > +		type = PERF_TYPE_READ_FROM_SYSFS;
 > +		break;
 >  	default:
 >  		type = rand();
 >  		break;

is 8 correct here ? not 7 ?

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Tue, 11 Jun 2013, Dave Jones wrote:

> On Tue, Jun 11, 2013 at 12:33:23AM -0400, Vince Weaver wrote:
> 
>  > @@ -80,7 +519,7 @@ static int random_event_type(void)
>  >  
>  >  	int type;
>  >  
>  > -	switch (rand() % 6) {
>  > +	switch (rand() % 8) {
>  >  	case 0:
>  >  		type = PERF_TYPE_HARDWARE;
>  >  		break;
>  > @@ -99,6 +538,9 @@ static int random_event_type(void)
>  >  	case 5:
>  >  		type = PERF_TYPE_BREAKPOINT;
>  >  		break;
>  > +	case 6:
>  > +		type = PERF_TYPE_READ_FROM_SYSFS;
>  > +		break;
>  >  	default:
>  >  		type = rand();
>  >  		break;
> 
> is 8 correct here ? not 7 ?

If you pick 7 then the default case never gets called, correct?  
I think that's a minor bug in the existing implementation, the default
case was never called.

Perhaps proper coding convention would be to have the 
make-the-type-field-completely-random case be an explicit value and use 
the default case only for error handling.

I should also maybe have the completely-random case be 
"completely radom but with preference to values < 256" as that's more
likely to trigger actual valid types.

Vince

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Tue, Jun 11, 2013 at 05:24:37PM -0400, Vince Weaver wrote:
 > On Tue, 11 Jun 2013, Dave Jones wrote:
 > 
 > > On Tue, Jun 11, 2013 at 12:33:23AM -0400, Vince Weaver wrote:
 > > 
 > >  > @@ -80,7 +519,7 @@ static int random_event_type(void)
 > >  >  
 > >  >  	int type;
 > >  >  
 > >  > -	switch (rand() % 6) {
 > >  > +	switch (rand() % 8) {
 > >  >  	case 0:
 > >  >  		type = PERF_TYPE_HARDWARE;
 > >  >  		break;
 > >  > @@ -99,6 +538,9 @@ static int random_event_type(void)
 > >  >  	case 5:
 > >  >  		type = PERF_TYPE_BREAKPOINT;
 > >  >  		break;
 > >  > +	case 6:
 > >  > +		type = PERF_TYPE_READ_FROM_SYSFS;
 > >  > +		break;
 > >  >  	default:
 > >  >  		type = rand();
 > >  >  		break;
 > > 
 > > is 8 correct here ? not 7 ?
 > 
 > If you pick 7 then the default case never gets called, correct?  
 > I think that's a minor bug in the existing implementation, the default
 > case was never called.

ah, yeah, I see.

 > Perhaps proper coding convention would be to have the 
 > make-the-type-field-completely-random case be an explicit value and use 
 > the default case only for error handling.

yeah, that's what I've done in a lot of other places (actually BUG() in those cases).
For the most part they're present just to shut up -Wswitch-default, which
has caught a few cases in the past where I've missed an option.
 
 > I should also maybe have the completely-random case be 
 > "completely radom but with preference to values < 256" as that's more
 > likely to trigger actual valid types.

ok, I'll apply what you sent so far. (and hold off on running it through Lindent for now,
you really don't like whitespace huh? ;)

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Tue, 11 Jun 2013, Dave Jones wrote:

> yeah, that's what I've done in a lot of other places (actually BUG() in those cases).
> For the most part they're present just to shut up -Wswitch-default, which
> has caught a few cases in the past where I've missed an option.

I'll go back through and check out all of the switch statements (there 
are a lot of them) and make sure they handle the default case 
consistently.
  
> ok, I'll apply what you sent so far. (and hold off on running it through Lindent for now,
> you really don't like whitespace huh? ;)

Not enough whitespace?  Or over-use of tabs?

Everyone hates my personal indent style so I try to match whatever project 
I contribute to, to the extent of using nano as an editor to make sure
emacs et al aren't introducing weird whitespace changes behind my back.
My contributions to outside projects are usually only one-liners so it
usually isn't an issue...

Vince

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Tue, Jun 11, 2013 at 06:21:08PM -0400, Vince Weaver wrote:
 > On Tue, 11 Jun 2013, Dave Jones wrote:
 > 
 > > yeah, that's what I've done in a lot of other places (actually BUG() in those cases).
 > > For the most part they're present just to shut up -Wswitch-default, which
 > > has caught a few cases in the past where I've missed an option.
 > 
 > I'll go back through and check out all of the switch statements (there 
 > are a lot of them) and make sure they handle the default case 
 > consistently.
 >   
 > > ok, I'll apply what you sent so far. (and hold off on running it through Lindent for now,
 > > you really don't like whitespace huh? ;)
 > 
 > Not enough whitespace?  Or over-use of tabs?

Mostly stuff like this. ..

	if((foo&bar)==0) i=rand();

instead of

	if ((foo & bar) == 0)
		i = rand();

indent -kr gets it mostly right. I tried bending the kernels scripts/Lindent to fit,
but I'm still not 100% happy.  It's good for a start point though.

	Dave
 

Re: [patch] perf_event_open enable sysfs exported events

[Tommi Rantala]

2013/6/11 Vince Weaver <vincent.weaver@maine.edu>:
>
> perf_event exports a bunch of event info via /sys under
> /sys/bus/event_source/devices
>
> By parsing these files it's possible to generate more likely valid
> (and almost-valid) events.  It also makes exercising expanded PMUs
> (such as uncore events on recent Intel machines) possible.
>
> The included somewhat ugly patch enables generating events based
> on the sysfs files (1 time out of 9).  It sets up a table of all
> possible event fields and generalized events for all detected event
> types the first time such an event is requested, then uses this table
> to generate events subsequent times.
>
> The parser is possibly fragile.  The perf tool actually has a full
> lexer to handle the parsing of these files.  I've tested on a core2
> and sandybridge machine but it might possibly fail on more obscure
> machines, especially if any have non-contiguous event fields.
>
> Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
>
> +static long long random_sysfs_config(__u32 *type, __u64 *config1) {
> +
> +       int i,j;
> +       long long c=0,c1=0;
> +
> +       i=rand()%num_pmus;

Hi, I'm occasionally seeing:

Unable to opendir /sys/bus/event_source/devices : Resource temporarily
unavailable
[221186.493638] traps: trinity-child30[375] trap divide error
ip:40dbae sp:7fff3c1d7ae0 error:0 in trinity[400000+28000]

One of the cores collected with -D gives me:

Core was generated by `./trinity -q -l off -C40 -D'.
Program terminated with signal 8, Arithmetic exception.
#0  0x000000000040dbae in random_sysfs_config (config1=0x1676038,
type=<optimized out>) at syscalls/perf_event_open.c:409
409             i=rand()%num_pmus;
#0  0x000000000040dbae in random_sysfs_config (config1=0x1676038,
type=<optimized out>) at syscalls/perf_event_open.c:409
#1  random_event_config (event_type=event_type@entry=0x1676000,
config1=config1@entry=0x1676038) at syscalls/perf_event_open.c:652
#2  0x000000000040e1e2 in create_mostly_valid_counting_event
(attr=0x1676000) at syscalls/perf_event_open.c:779

Tommi

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Thu, 13 Jun 2013, Tommi Rantala wrote:

> Unable to opendir /sys/bus/event_source/devices : Resource temporarily
> unavailable
> [221186.493638] traps: trinity-child30[375] trap divide error
> ip:40dbae sp:7fff3c1d7ae0 error:0 in trinity[400000+28000]

The below patch should fix this problem.

Though in the case where /sys/bus/event_source/devices isn't available
trinity will try opening it again each time it tries to do a 
PERF_TYPE_READ_FROM_SYSFS type (1 time in 7).  Not sure if it's
worth rate-limiting that.

Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>


diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index 32c11cf..11f49e8 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -406,6 +406,14 @@ static long long random_sysfs_config(__u32 *type, __u64 *config1) {
 	int i,j;
 	long long c=0,c1=0;
 
+	if (num_pmus==0) {
+		/* For some reason we didn't get initialized */
+		/* Fake it so we don't divide by zero        */
+		*type=rand32();
+		*config1=rand64();
+		return rand64();
+	}
+
 	i=rand()%num_pmus;
 
 	*type=pmus[i].type;

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Thu, Jun 13, 2013 at 03:58:37PM -0400, Vince Weaver wrote:
 > On Thu, 13 Jun 2013, Tommi Rantala wrote:
 > 
 > > [221186.493638] traps: trinity-child30[375] trap divide error
 > > ip:40dbae sp:7fff3c1d7ae0 error:0 in trinity[400000+28000]
 > 
 > diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
 > index 32c11cf..11f49e8 100644
 > --- a/syscalls/perf_event_open.c
 > +++ b/syscalls/perf_event_open.c
 > @@ -406,6 +406,14 @@ static long long random_sysfs_config(__u32 *type, __u64 *config1) {
 >  	int i,j;
 >  	long long c=0,c1=0;
 >  
 > +	if (num_pmus==0) {
 > +		/* For some reason we didn't get initialized */
 > +		/* Fake it so we don't divide by zero        */
 > +		*type=rand32();
 > +		*config1=rand64();
 > +		return rand64();
 > +	}

Probably safer ?

+               *type = rand32() | 1;
+               *config1 = rand64() | 1;
+               return rand64() | 1;

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Thu, Jun 13, 2013 at 05:07:59PM -0400, Vince Weaver wrote:
 > On Thu, 13 Jun 2013, Dave Jones wrote:
 > > 
 > > Probably safer ?
 > > 
 > > +               *type = rand32() | 1;
 > > +               *config1 = rand64() | 1;
 > > +               return rand64() | 1;
 > 
 > no, 0 is a perfectly valid value for all of those fields.
 > 
 > The error was coming in to play because if sysfs wasn't found, num_pmus
 > was set to 0, and we were doing a " rand() % num_pmus " to choose
 > a random value for the type field.

ah, gotcha. I'll queue up your patch.

thanks,

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Thu, 13 Jun 2013, Dave Jones wrote:
> 
> Probably safer ?
> 
> +               *type = rand32() | 1;
> +               *config1 = rand64() | 1;
> +               return rand64() | 1;

no, 0 is a perfectly valid value for all of those fields.

The error was coming in to play because if sysfs wasn't found, num_pmus
was set to 0, and we were doing a " rand() % num_pmus " to choose
a random value for the type field.

Vince

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Thu, Jun 13, 2013 at 03:58:37PM -0400, Vince Weaver wrote:
 > On Thu, 13 Jun 2013, Tommi Rantala wrote:
 > 
 > > Unable to opendir /sys/bus/event_source/devices : Resource temporarily
 > > unavailable
 >
 > Though in the case where /sys/bus/event_source/devices isn't available
 > trinity will try opening it again each time it tries to do a 
 > PERF_TYPE_READ_FROM_SYSFS type (1 time in 7).  Not sure if it's
 > worth rate-limiting that.

I'm puzzled why I'm seeing opendir() fail with -ENOMEM when there's gigabytes
of free memory available.  Likewise, getting -EAGAIN seems.. weird.

Is there actually a problem there ? Or can we just remove the output from
the failure paths, and fail silently ?

init_pmus might be simpler if it was converted to use nftw() too.

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Wed, 19 Jun 2013, Dave Jones wrote:

> On Thu, Jun 13, 2013 at 03:58:37PM -0400, Vince Weaver wrote:
>  > On Thu, 13 Jun 2013, Tommi Rantala wrote:
>  > 
>  > > Unable to opendir /sys/bus/event_source/devices : Resource temporarily
>  > > unavailable
>  >
>  > Though in the case where /sys/bus/event_source/devices isn't available
>  > trinity will try opening it again each time it tries to do a 
>  > PERF_TYPE_READ_FROM_SYSFS type (1 time in 7).  Not sure if it's
>  > worth rate-limiting that.
> 
> I'm puzzled why I'm seeing opendir() fail with -ENOMEM when there's gigabytes
> of free memory available.  Likewise, getting -EAGAIN seems.. weird.

Is this easy to reproduce?  I haven't seen it locally but then again
I have only been running trinity with -c perf_event_open
I only left the debug message in because I thought it would be unusual
for a quick sysfs traversal to fail.

> init_pmus might be simpler if it was converted to use nftw() too.

I was unaware of the existence of nftw(), I'll work on updating the code
to use it.

Vince

Re: [patch] perf_event_open enable sysfs exported events

[Dave Jones]

On Wed, Jun 19, 2013 at 12:34:43PM -0400, Vince Weaver wrote:
 > On Wed, 19 Jun 2013, Dave Jones wrote:
 > 
 > > On Thu, Jun 13, 2013 at 03:58:37PM -0400, Vince Weaver wrote:
 > >  > On Thu, 13 Jun 2013, Tommi Rantala wrote:
 > >  > 
 > >  > > Unable to opendir /sys/bus/event_source/devices : Resource temporarily
 > >  > > unavailable
 > >  >
 > >  > Though in the case where /sys/bus/event_source/devices isn't available
 > >  > trinity will try opening it again each time it tries to do a 
 > >  > PERF_TYPE_READ_FROM_SYSFS type (1 time in 7).  Not sure if it's
 > >  > worth rate-limiting that.
 > > 
 > > I'm puzzled why I'm seeing opendir() fail with -ENOMEM when there's gigabytes
 > > of free memory available.  Likewise, getting -EAGAIN seems.. weird.
 > 
 > Is this easy to reproduce?  I haven't seen it locally but then again
 > I have only been running trinity with -c perf_event_open
 > I only left the debug message in because I thought it would be unusual
 > for a quick sysfs traversal to fail.

Hmm, that's weird. When I run with -c perf_event_open it doesn't happen.
So it's an interaction with something else that's causing it. Fun.
 
 > > init_pmus might be simpler if it was converted to use nftw() too.
 > 
 > I was unaware of the existence of nftw(), I'll work on updating the code
 > to use it.

I only recently learned of it myself. the tree walking in files.c got
a lot simpler. See b0d4df18a6785b0c3347f521bb2c09e88b0c0966.

	Dave

Re: [patch] perf_event_open enable sysfs exported events

[Vince Weaver]

On Wed, 19 Jun 2013, Dave Jones wrote:

>  > > I'm puzzled why I'm seeing opendir() fail with -ENOMEM when there's gigabytes
>  > > of free memory available.  Likewise, getting -EAGAIN seems.. weird.

OK, I've been able to reproduce this but having trouble explaining what I 
see.  

(as a side-note, should gdb work with trinity?  I couldn't
get breakpoints to work, in the end I had to add a long sleep in 
init_pmus() and then attach by pid)

So init_pmus() does this:

  dir=opendir("/sys/bus/event_source/devices");

Which glibc converts to something like this:

  fd = openat_not_cancel_3 (dfd, name, flags);
  DIR *dirp = (DIR *) malloc (sizeof (DIR) + allocation);

Where "sizeof (DIR) + allocation" appears to be 32,808 bytes.

When this works, you get an strace output like this:

openat(AT_FDCWD, "/sys/bus/event_source/devices", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 6 entries */, 32768)     = 168
...


But in the failure case you get this, where the malloc() starts trying to 
allocate hundreds of megabytes or RAM and fails?

openat(AT_FDCWD, "/sys/bus/event_source/devices", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 658
brk(0x1a7c000)              = 0x1a54000
mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 134217728, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 67108864, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 134217728, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 67108864, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
brk(0x1a76000)              = 0x1a54000
mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 134217728, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 67108864, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 134217728, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
mmap(NULL, 67108864, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 EAGAIN (Resource temporarily unavailable)
close(658)                  = 0
write(2, "Unable to opendir /sys/bus/event"..., 83Unable to opendir /sys/bus/event_source/devices : Resource temporarily unavailable) = 83


Weird.  Should I try to track this down more, or is this something 
expected to happen when malloc() is involved?

It's tricky to debug because the malloc() runs fine from inside of gdb.

Also I'm noticing that I might need some locking around the init_pmus() 
call as there's a chance the trinity children might try to call into it at 
the same time.

Vince