Skip to contentSkip to conversation

perf,tools: Remove test_attr_* muck from perf.h

RFC PATCH In review Peter Zijlstra 2 participants 4 messages May 1, 2014 – May 4, 2014 perf events

lore
Share & tools
lore

Peter Zijlstra

author maintainer
#0 lore raw

RFC PATCH 1/1 perf,tools: Remove test_attr_* muck from perf.h

Hi,

I occasionally build the odd program against raw perf and use perf.h for this.

Now I find that no longer works because of:

52502bf201a85 ("perf tests: Add framework for automated perf_event_attr tests")

So revert the hunk touching on perf.h.

Also, it seems to be for snake shit only, and snakes stink, except on a plane :-)

---
 tools/perf/perf.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)
tools/perf/perf.h +1 -13
index ebdad3376c67..4a089338b60c 100644
@@ -212,25 +212,13 @@ static inline unsigned long long rdclock(void)
 	(void) (&_min1 == &_min2);		\
 	_min1 < _min2 ? _min1 : _min2; })
 
-extern bool test_attr__enabled;
-void test_attr__init(void);
-void test_attr__open(struct perf_event_attr *attr, pid_t pid, int cpu,
-		     int fd, int group_fd, unsigned long flags);
-
 static inline int
 sys_perf_event_open(struct perf_event_attr *attr,
 		      pid_t pid, int cpu, int group_fd,
 		      unsigned long flags)
 {
-	int fd;
-
-	fd = syscall(__NR_perf_event_open, attr, pid, cpu,
+	return syscall(__NR_perf_event_open, attr, pid, cpu,
 		     group_fd, flags);
-
-	if (unlikely(test_attr__enabled))
-		test_attr__open(attr, pid, cpu, fd, group_fd, flags);
-
-	return fd;
 }
 
 #define MAX_COUNTERS			256
#1 lore raw
Quoted 10 lines · Peter Zijlstra

Hi,

I occasionally build the odd program against raw perf and use perf.h for this.

Now I find that no longer works because of:

52502bf201a85 ("perf tests: Add framework for automated perf_event_attr tests")

So revert the hunk touching on perf.h.

But but but, then you'll brake attr tests

Quoted 3 lines

Also, it seems to be for snake shit only, and snakes stink, except on a plane :-)

wanna get to lwn quotes page, huh? ;)

any details about the breakage? maybe we could find some way to make you happy and keep the snake shit in place

jirka

author maintainer
#2 lore raw
Quoted 15 lines · Jiri Olsa
Quoted 10 lines · Peter Zijlstra

Hi,

I occasionally build the odd program against raw perf and use perf.h for this.

Now I find that no longer works because of:

52502bf201a85 ("perf tests: Add framework for automated perf_event_attr tests")

So revert the hunk touching on perf.h.

But but but, then you'll brake attr tests

Quoted 1 line
Quoted 7 lines
Quoted 2 lines

Also, it seems to be for snake shit only, and snakes stink, except on a plane :-)

wanna get to lwn quotes page, huh? ;)

any details about the breakage? maybe we could find some way to make you happy and keep the snake shit in place

linux-2.6/tools/perf# gcc -o perf_bug perf_bug.c /tmp/cclObJsz.o: In function `sys_perf_event_open':
perf_bug.c:(.text+0x48): undefined reference to test_attr__enabled' perf_bug.c:(.text+0x72): undefined reference to test_attr__open'
collect2: error: ld returned 1 exit status

Initially perf.h was meant to have just the syscall wrapper and few system level defines (like the membars etc..)

But now its grown all kinds of gunk outside of that.

So ideally we'd split the thing into two files, one which is a stand-alone syscall wrapper + system utils things and one which includes the former and adds all the perf (tool, not kernel) gunk on.

I see all sorts of extern perf_{host,guest} things and util/target.h includes.. that all should definitely move elsewhere.

---
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/mman.h>
#include <signal.h>
#include <unistd.h>
#include <stdio.h>
#include "perf.h"

struct perf_event_attr perf_attr = {
	.type = PERF_TYPE_SOFTWARE,
	.config = PERF_COUNT_SW_EMULATION_FAULTS,
	.sample_period = 0,
	.inherit = 1,
};

int main(void)

{
	void *page;
	pid_t pid;
	int fd[16];

	fd[0] = sys_perf_event_open(&perf_attr, 0, -1, -1, 0);
	if (fd[0] < 0) {
		perror("sys_perf_event_open 0");
		return 0;
	}

	perf_attr.inherit = 0;

	fd[1] = sys_perf_event_open(&perf_attr, 0, -1, fd[0], 0);
	if (fd[1] < 0) {
		perror("sys_perf_event_open 1");
		return 0;
	}

	page = mmap(NULL, 3*4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd[1], 0);
	if (page == ((void *)-1)) {
		perror("mmap");
		return 0;
	}

	pid = fork();
	if (!pid) {
		/* child */
		while (1) ;
		return 0;
	}
	usleep(500000);

	munmap(page, 3*4096);

	close(fd[1]);
	kill(pid, SIGTERM);
	waitpid(pid, NULL, 0);

	return 0;
}
#3 lore raw
Quoted 33 lines · Peter Zijlstra
Quoted 15 lines · Jiri Olsa
Quoted 10 lines · Peter Zijlstra

Hi,

I occasionally build the odd program against raw perf and use perf.h for this.

Now I find that no longer works because of:

52502bf201a85 ("perf tests: Add framework for automated perf_event_attr tests")

So revert the hunk touching on perf.h.

But but but, then you'll brake attr tests

Quoted 1 line
Quoted 7 lines
Quoted 2 lines

Also, it seems to be for snake shit only, and snakes stink, except on a plane :-)

wanna get to lwn quotes page, huh? ;)

any details about the breakage? maybe we could find some way to make you happy and keep the snake shit in place

linux-2.6/tools/perf# gcc -o perf_bug perf_bug.c /tmp/cclObJsz.o: In function `sys_perf_event_open':
perf_bug.c:(.text+0x48): undefined reference to test_attr__enabled' perf_bug.c:(.text+0x72): undefined reference to test_attr__open'
collect2: error: ld returned 1 exit status

Initially perf.h was meant to have just the syscall wrapper and few system level defines (like the membars etc..)

Quoted 8 lines

But now its grown all kinds of gunk outside of that.

So ideally we'd split the thing into two files, one which is a stand-alone syscall wrapper + system utils things and one which includes the former and adds all the perf (tool, not kernel) gunk on.

I see all sorts of extern perf_{host,guest} things and util/target.h includes.. that all should definitely move elsewhere.

sounds good, I'll prepare something along those lines

jirka