perf:util:dso: Using O_CLOEXEC in do_open

PATCH In review Wang YanQing 3 participants 5 messages Jan 15, 2018 – Jan 24, 2018 perf events

Share & tools

Wang YanQing

author

2018-01-15 07:01 UTC #0 lore raw

PATCH 1/1 perf:util:dso: Using O_CLOEXEC in do_open

I meet strange behavior with below commands on my gentoo box:
1:perf kmem record
2:CTRL-C to stop 1
3:perf report
4:"Enter", "Enter", "Run scripts for all samples",
"event_analyzing_sample".

Then perf report:
"
No kallsyms or vmlinux with build-id xxxx was found /lib/modules/4.10.0+/build/vmlinux with build id xxxx not found, continuing without symbols
".

It is strange because I am sure /lib/modules/4.10.0+/build/vmlinux is right for perf.data.

After digging, I find out the reason is "perf report" generates many open fds, then "script_browse" uses popen to run "perf script" which run out of open files. The gentoo box has a small default value for "max open files", 1024. Yes, "ulimit -n " with a bigger number could fix it, but I think that using O_CLOEXEC in do_open is a better way.

Signed-off-by Wang YanQing <[email protected]>

---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/dso.c +1 -1

index d5b6f7f..36ef45b 100644
@@ -446,7 +446,7 @@ static int do_open(char *name)
 	char sbuf[STRERR_BUFSIZE];
 
 	do {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY|O_CLOEXEC);
 		if (fd >= 0)
 			return fd;

Signature · 1 line

1.8.5.6.2.g3d8a54e.dirty

Jiri Olsa

2018-01-15 09:45 UTC #1 lore raw

Quoted 14 lines · Wang YanQing

I meet strange behavior with below commands on my gentoo box:
1:perf kmem record
2:CTRL-C to stop 1
3:perf report
4:"Enter", "Enter", "Run scripts for all samples",
"event_analyzing_sample".
Then perf report:
"
No kallsyms or vmlinux with build-id xxxx was found /lib/modules/4.10.0+/build/vmlinux with build id xxxx not found, continuing without symbols
".

Quoted 8 lines

It is strange because I am sure /lib/modules/4.10.0+/build/vmlinux is right for perf.data.
After digging, I find out the reason is "perf report" generates many open fds, then "script_browse" uses popen to run "perf script" which run out of open files. The gentoo box has a small default value for "max open files", 1024. Yes, "ulimit -n " with a bigger number could fix it, but I think that using O_CLOEXEC in do_open is a better way.

that seems right.. so why does kmem record open those files before calling cmd_record? should that be fixed?

thanks,
jirka

Quoted 21 lines

Signed-off-by Wang YanQing <[email protected]>

---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/dso.c +1 -1

index d5b6f7f..36ef45b 100644
@@ -446,7 +446,7 @@ static int do_open(char *name)
 	char sbuf[STRERR_BUFSIZE];
 
 	do {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY|O_CLOEXEC);
 		if (fd >= 0)
 			return fd;

Signature · 1 line

1.8.5.6.2.g3d8a54e.dirty

Wang YanQing

author

2018-01-16 17:03 UTC #2 lore raw

Quoted 18 lines · Jiri Olsa

Quoted 17 lines · Wang YanQing
I meet strange behavior with below commands on my gentoo box:
1:perf kmem record
2:CTRL-C to stop 1
3:perf report
4:"Enter", "Enter", "Run scripts for all samples",
"event_analyzing_sample".
Then perf report:
"
No kallsyms or vmlinux with build-id xxxx was found /lib/modules/4.10.0+/build/vmlinux with build id xxxx not found, continuing without symbols
".
It is strange because I am sure /lib/modules/4.10.0+/build/vmlinux is right for perf.data.

Quoted 8 lines

Quoted 5 lines
After digging, I find out the reason is "perf report" generates many open fds, then "script_browse" uses popen to run "perf script" which run out of open files. The gentoo box has a small default value for "max open files", 1024. Yes, "ulimit -n " with a bigger number could fix it, but I think that using O_CLOEXEC in do_open is a better way.
that seems right.. so why does kmem record open those files before calling cmd_record? should that be fixed?

The problem is "perf report", isn't "perf kmem record", see the above reproduction steps again.

The problem is "perf report" generates may open fds with dso:do_open for symbol analysis, and "perf report" uses popen to run another command "perf script", then the cmd "perf script" will run out of "max open files".

Thanks.

Quoted 25 lines

thanks,
jirka

Quoted 21 lines

Signed-off-by Wang YanQing <[email protected]>

---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/dso.c +1 -1

index d5b6f7f..36ef45b 100644
@@ -446,7 +446,7 @@ static int do_open(char *name)
 	char sbuf[STRERR_BUFSIZE];
 
 	do {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY|O_CLOEXEC);
 		if (fd >= 0)
 			return fd;

Signature · 1 line

1.8.5.6.2.g3d8a54e.dirty

Jiri Olsa

Acked-by on v1 1/1

2018-01-18 09:45 UTC #3 lore raw

In reply to Wang YanQing

Quoted 17 lines · Wang YanQing

I meet strange behavior with below commands on my gentoo box:
1:perf kmem record
2:CTRL-C to stop 1
3:perf report
4:"Enter", "Enter", "Run scripts for all samples",
"event_analyzing_sample".
Then perf report:
"
No kallsyms or vmlinux with build-id xxxx was found /lib/modules/4.10.0+/build/vmlinux with build id xxxx not found, continuing without symbols
".
It is strange because I am sure /lib/modules/4.10.0+/build/vmlinux is right for perf.data.

Quoted 7 lines

After digging, I find out the reason is "perf report" generates many open fds, then "script_browse" uses popen to run "perf script" which run out of open files. The gentoo box has a small default value for "max open files", 1024. Yes, "ulimit -n " with a bigger number could fix it, but I think that using O_CLOEXEC in do_open is a better way.
Signed-off-by Wang YanQing <[email protected]>

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 19 lines

---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/dso.c +1 -1

index d5b6f7f..36ef45b 100644
@@ -446,7 +446,7 @@ static int do_open(char *name)
 	char sbuf[STRERR_BUFSIZE];
 
 	do {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY|O_CLOEXEC);
 		if (fd >= 0)
 			return fd;

Signature · 1 line

1.8.5.6.2.g3d8a54e.dirty

tip-bot for Wang YanQing

merge bot

2018-01-24 11:25 UTC #4 lore raw

[tip:perf/core] perf symbols: Using O_CLOEXEC in do_open

In reply to Wang YanQing

perf symbols: Using O_CLOEXEC in do_open

Commit-ID:  4c0d8d27954d9efb2a02ec9fc16f39b02f248bb7
Gitweb:     https://git.kernel.org/tip/4c0d8d27954d9efb2a02ec9fc16f39b02f248bb7
Author:     Wang YanQing <[email protected]>
AuthorDate: Mon, 15 Jan 2018 13:04:48 +0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 23 Jan 2018 09:49:28 -0300

perf symbols: Using O_CLOEXEC in do_open

I've meet a strange behavior with these commands on my gentoo box:

1: perf kmem record
2: CTRL-C to stop 1
3: perf report
4: "Enter", "Enter", "Run scripts for all samples",
"event_analyzing_sample".

Then 'perf report' says:

"
No kallsyms or vmlinux with build-id xxxx was found /lib/modules/4.10.0+/build/vmlinux with build id xxxx not found, continuing without symbols
".

It is strange because I am sure /lib/modules/4.10.0+/build/vmlinux is right for perf.data.

After digging, I found out the reason is that "perf report" generates many open fds, then "script_browse" uses popen to run "perf script" which run out of open files.

The gentoo box has a small default value for "max open files", 1024. Yes, "ulimit -n " with a bigger number could fix it, but I think that using O_CLOEXEC in do_open is a better way.

Signed-off-by Wang YanQing <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/20180115050448.GA20759@udknight open here

[ Make sure O_CLOEXEC is available in old systems by adding a patch
just before this one, to keep this bisectable in such systems ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/dso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/dso.c +1 -1

index d5b6f7f..36ef45b 100644
@@ -446,7 +446,7 @@ static int do_open(char *name)
 	char sbuf[STRERR_BUFSIZE];
 
 	do {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY|O_CLOEXEC);
 		if (fd >= 0)
 			return fd;