linux-kernel - Re: [PATCH 3/3] perf: fix synthesizing mmaps for threads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140226143208.GC7552@ghostprotocols.net>
Date:	Wed, 26 Feb 2014 11:32:08 -0300
From:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To:	Don Zickus <dzickus@...hat.com>
Cc:	LKML <linux-kernel@...r.kernel.org>, jolsa@...hat.com,
	eranian@...gle.com
Subject: Re: [PATCH 3/3] perf: fix synthesizing mmaps for threads

Em Wed, Feb 26, 2014 at 11:17:26AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 25, 2014 at 10:43:47PM -0500, Don Zickus escreveu:
> > Currently if a process creates a bunch of threads using pthread_create
> > and then perf is run in system_wide mode, the mmaps for those threads
> > are not captured with a synthesized mmap event.
> > 
> > The reason is those threads are not visible when walking the /proc/
> > directory looking for /proc/<pid>/maps files.  Instead they are discovered
> > using the /proc/<pid>/tasks file (which the synthesized comm event uses).
> > 
> > This causes problems when a program is trying to map a data address to a
> > tid.  Because the tid has no maps, the event is dropped.  Changing the program
> > to look up using the pid instead of the tid, finds the correct maps but creates
> > ugly hacks in the program to carry the correct tid around.
> > 
> > Fix this by synthesizing mmap events for each tid found in the /proc/<pid>/tasks
> > file.
> 
> This seems to cover two problems, the first is for mmap/mmap2 event
> processing to lookup pid/tid instead of pid/pid, the other one is to
> iterate thru /proc/pid/tasks/, so this needes spliting up.
> 
> Now looking at the /tasks/ part...

Agreed we need to synthesize the mmap events for the threads in
/proc/pid/task/, but that need to be done in
perf_event__synthesize_mmap_events, not in
perf_event__synthesize_comm_events, that should remain just for
synthesizing comm events .

I.e. we should move that /tasks/ traversal from synthesize_comm to its
caller and from there synthesize the mmaps too.

I can do that if you don't beat me to it. :-)

- Arnaldo

> > This may not be entirely clean but it seems to work.
> > 
> > Signed-off-by: Don Zickus <dzickus@...hat.com>
> > ---
> >  tools/perf/util/event.c   | 15 +++++++++++----
> >  tools/perf/util/machine.c |  4 ++--
> >  2 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index 086c7c8..09c53bb 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -93,10 +93,13 @@ static pid_t perf_event__get_comm_tgid(pid_t pid, char *comm, size_t len)
> >  }
> >  
> >  static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> > -					 union perf_event *event, pid_t pid,
> > +					 union perf_event *event,
> > +					 union perf_event *mmap_event,
> > +					 pid_t pid,
> >  					 int full,
> >  					 perf_event__handler_t process,
> > -					 struct machine *machine)
> > +					 struct machine *machine,
> > +					 bool mmap_data)
> >  {
> >  	char filename[PATH_MAX];
> >  	size_t size;
> > @@ -168,6 +171,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
> >  			tgid = -1;
> >  			break;
> >  		}
> > +
> > +		/* process the thread's maps too */
> > +		perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > +						  process, machine, mmap_data);
> >  	}
> >  
> >  	closedir(tasks);
> > @@ -331,8 +338,8 @@ static int __event__synthesize_thread(union perf_event *comm_event,
> >  				      struct perf_tool *tool,
> >  				      struct machine *machine, bool mmap_data)
> >  {
> > -	pid_t tgid = perf_event__synthesize_comm(tool, comm_event, pid, full,
> > -						 process, machine);
> > +	pid_t tgid = perf_event__synthesize_comm(tool, comm_event, mmap_event, pid,
> > +						 full, process, machine, mmap_data);
> >  	if (tgid == -1)
> >  		return -1;
> >  	return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 813e94e..eb26544 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1026,7 +1026,7 @@ int machine__process_mmap2_event(struct machine *machine,
> >  	}
> >  
> >  	thread = machine__findnew_thread(machine, event->mmap2.pid,
> > -					event->mmap2.pid);
> > +					event->mmap2.tid);
> >  	if (thread == NULL)
> >  		goto out_problem;
> >  
> > @@ -1074,7 +1074,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
> >  	}
> >  
> >  	thread = machine__findnew_thread(machine, event->mmap.pid,
> > -					 event->mmap.pid);
> > +					 event->mmap.tid);
> >  	if (thread == NULL)
> >  		goto out_problem;
> >  
> > -- 
> > 1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/