Modules installed outside of the kernel's build system should go into
"%s/lib/modules/%s/extra", but at present, perf will only look at them
when they are in "%s/lib/modules/%s/kernel". Lets encourage good
citizenship by relaxing this requirement to "%s/lib/modules/%s". This
way open source modules that are out-of-tree have no incentive to start
populating a directory reserved for in-kernle modules and I can stop hex
editing my system's perf binary when profiling OSS out-of-tree modules.
Signed-off-by: Richard Yao <[email protected]>
---
tools/perf/util/machine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index a53cd0b..116842e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -786,7 +786,7 @@ static int machine__set_modules_path(struct machine *machine)
if (!version)
return -1;
- snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s/kernel",
+ snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s",
machine->root_dir, version);
free(version);
--
1.8.3.2
On 4/10/14, 9:52 AM, Richard Yao wrote:
> Modules installed outside of the kernel's build system should go into
> "%s/lib/modules/%s/extra", but at present, perf will only look at them
> when they are in "%s/lib/modules/%s/kernel". Lets encourage good
> citizenship by relaxing this requirement to "%s/lib/modules/%s". This
> way open source modules that are out-of-tree have no incentive to start
> populating a directory reserved for in-kernle modules and I can stop hex
> editing my system's perf binary when profiling OSS out-of-tree modules.
>
> Signed-off-by: Richard Yao <[email protected]>
> ---
> tools/perf/util/machine.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index a53cd0b..116842e 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -786,7 +786,7 @@ static int machine__set_modules_path(struct machine *machine)
> if (!version)
> return -1;
>
> - snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s/kernel",
> + snprintf(modules_path, sizeof(modules_path), "%s/lib/modules/%s",
> machine->root_dir, version);
> free(version);
>
>
Acked-by: David Ahern <[email protected]>
Hi Richard,
On Thu, 10 Apr 2014 12:52:59 -0400, Richard Yao wrote:
> Modules installed outside of the kernel's build system should go into
> "%s/lib/modules/%s/extra", but at present, perf will only look at them
> when they are in "%s/lib/modules/%s/kernel". Lets encourage good
> citizenship by relaxing this requirement to "%s/lib/modules/%s". This
> way open source modules that are out-of-tree have no incentive to start
> populating a directory reserved for in-kernle modules and I can stop hex
> editing my system's perf binary when profiling OSS out-of-tree modules.
But it'll make the perf traverses all the source and build directories
too, right? I don't think it's a right thing to do.
Maybe we can also change stat() in map_groups__set_modules_path_dir() to
lstat() so that it cannot go to unwanted directories in that case. Or
else, just checking "kernel" and "extra" directories will work.
Thanks,
Namhyung
On Tue, Apr 15, 2014 at 02:44:52PM +0900, Namhyung Kim wrote:
> Hi Richard,
>
> On Thu, 10 Apr 2014 12:52:59 -0400, Richard Yao wrote:
> > Modules installed outside of the kernel's build system should go into
> > "%s/lib/modules/%s/extra", but at present, perf will only look at them
> > when they are in "%s/lib/modules/%s/kernel". Lets encourage good
> > citizenship by relaxing this requirement to "%s/lib/modules/%s". This
> > way open source modules that are out-of-tree have no incentive to start
> > populating a directory reserved for in-kernle modules and I can stop hex
> > editing my system's perf binary when profiling OSS out-of-tree modules.
>
> But it'll make the perf traverses all the source and build directories
> too, right? I don't think it's a right thing to do.
>
> Maybe we can also change stat() in map_groups__set_modules_path_dir() to
> lstat() so that it cannot go to unwanted directories in that case. Or
> else, just checking "kernel" and "extra" directories will work.
yay, forgot about source directory.. :-\ looks like lstat should
help, but hardcoding kernel and extra sounds better to me.
Richard, please send updated patch
thanks,
jirka
Dear Jirka,
I have sent an updated patch, but instead of using your white list idea,
I went with a black list. That way things like the historical addon
directory are included and anyone who decides to use a custom directory
for their own in-development modules is free to do so.
Yours truly,
Richard Yao
On 04/15/2014 07:56 AM, Jiri Olsa wrote:
> On Tue, Apr 15, 2014 at 02:44:52PM +0900, Namhyung Kim wrote:
>> Hi Richard,
>>
>> On Thu, 10 Apr 2014 12:52:59 -0400, Richard Yao wrote:
>>> Modules installed outside of the kernel's build system should go into
>>> "%s/lib/modules/%s/extra", but at present, perf will only look at them
>>> when they are in "%s/lib/modules/%s/kernel". Lets encourage good
>>> citizenship by relaxing this requirement to "%s/lib/modules/%s". This
>>> way open source modules that are out-of-tree have no incentive to start
>>> populating a directory reserved for in-kernle modules and I can stop hex
>>> editing my system's perf binary when profiling OSS out-of-tree modules.
>>
>> But it'll make the perf traverses all the source and build directories
>> too, right? I don't think it's a right thing to do.
>>
>> Maybe we can also change stat() in map_groups__set_modules_path_dir() to
>> lstat() so that it cannot go to unwanted directories in that case. Or
>> else, just checking "kernel" and "extra" directories will work.
>
> yay, forgot about source directory.. :-\ looks like lstat should
> help, but hardcoding kernel and extra sounds better to me.
>
> Richard, please send updated patch
>
> thanks,
> jirka
>