Em Thu, Dec 19, 2013 at 05:54:51PM -0500, Dongsheng Yang escreveu: > We are using XXX__synthesize_threads() function to synthesize the > symbols of user space for host. This patch add support of guest > for these functions.
I think this patch should be split in at least two, please read below. And please write here an outline of what will happen so that it can find the right /proc files using machine->root_dir, etc, so that casual (and experienced too) hackers can figure out what your patch does more quickly. - Arnaldo > Signed-off-by: Dongsheng Yang <[email protected]> > --- > tools/perf/util/event.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c > index 6948768..6e36bbb 100644 > --- a/tools/perf/util/event.c > +++ b/tools/perf/util/event.c > @@ -106,8 +106,12 @@ static pid_t perf_event__synthesize_comm(struct > perf_tool *tool, > > memset(&event->comm, 0, sizeof(event->comm)); > > - tgid = perf_event__get_comm_tgid(pid, event->comm.comm, > - sizeof(event->comm.comm)); > + if (machine__is_host(machine)) > + tgid = perf_event__get_comm_tgid(pid, event->comm.comm, > + sizeof(event->comm.comm)); > + else > + tgid = machine->pid; > + > if (tgid < 0) > goto out; > > @@ -129,7 +133,11 @@ static pid_t perf_event__synthesize_comm(struct > perf_tool *tool, > goto out; > } > > - snprintf(filename, sizeof(filename), "/proc/%d/task", pid); > + if (machine__is_default_guest(machine)) > + return 0; So the hunk above, the one that makes it bail out if it is a default guest, I think it should be a separate patch with an explanation that since pid is equal to 0: #define DEFAULT_GUEST_KERNEL_ID (0) opendir("/proc/0/task") will fail, so its better to check if it is the default guest instead of trying to opendir a directory that doesn't exists and print a meaningless message in debug mode: pr_debug("couldn't open %s\n", filename); Then, the rest, which is to use machine->root_dir to find the right proc dir, probably via sshfs (right?), will work as expected _and_ is a separate, unrelated to the above (bailing out on default guest pid)) patch, ok? > + snprintf(filename, sizeof(filename), "%s/proc/%d/task", > + machine->root_dir, pid); > > tasks = opendir(filename); > if (tasks == NULL) { > @@ -178,7 +186,11 @@ static int perf_event__synthesize_mmap_events(struct > perf_tool *tool, > FILE *fp; > int rc = 0; > > - snprintf(filename, sizeof(filename), "/proc/%d/maps", pid); > + if (machine__is_default_guest(machine)) This one should be on the first patch, together with the other machine__is_default_guest() test, please look for any other such test and group them in a separate patch, as outlined above. > + return 0; > + > + snprintf(filename, sizeof(filename), "%s/proc/%d/maps", > + machine->root_dir, pid); > > fp = fopen(filename, "r"); > if (fp == NULL) { > @@ -218,7 +230,10 @@ static int perf_event__synthesize_mmap_events(struct > perf_tool *tool, > /* > * Just like the kernel, see __perf_event_mmap in > kernel/perf_event.c > */ > - event->header.misc = PERF_RECORD_MISC_USER; > + if (machine__is_host(machine)) > + event->header.misc = PERF_RECORD_MISC_USER; > + else > + event->header.misc = PERF_RECORD_MISC_GUEST_USER; > > if (prot[2] != 'x') { > if (!mmap_data || prot[0] != 'r') > @@ -387,6 +402,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool, > struct machine *machine, bool mmap_data) > { > DIR *proc; > + char proc_path[PATH_MAX]; > struct dirent dirent, *next; > union perf_event *comm_event, *mmap_event; > int err = -1; > @@ -399,7 +415,12 @@ int perf_event__synthesize_threads(struct perf_tool > *tool, > if (mmap_event == NULL) > goto out_free_comm; > > - proc = opendir("/proc"); > + if (machine__is_default_guest(machine)) > + return 0; One more :-) > + snprintf(proc_path, sizeof(proc_path), "%s/proc", machine->root_dir); > + proc = opendir(proc_path); > + > if (proc == NULL) > goto out_free_mmap; > > -- > 1.8.2.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
