RE: [PATCH RFC V2 07/10] perf tools: change machine comm_exec type to atomic

[Liang, Kan]

> Em Sun, Sep 10, 2017 at 07:23:20PM -0700, kan.liang@xxxxxxxxx escreveu:
> > From: Kan Liang <kan.liang@xxxxxxxxx>
> >
> > In case there are two or more threads want to change it.
> 
> Please describe the scenario that made you want to have this in place, to
> help in reviewing and to have it documented in this patch commit log.
> 

Thinking more about it. For synthesizing event, we don't need to change the
comm_exec. PERF_RECORD_MISC_COMM_EXEC is not set in this stage.
But we should consider it if  we want to multithread the event processing stage.

Since it's not an issue for now, I will drop this patch and add some comments
in the code for V3.

Thanks,
Kan

> - Arnaldo
> 
> > Signed-off-by: Kan Liang <kan.liang@xxxxxxxxx>
> > ---
> >  tools/perf/util/machine.c | 11 ++++++-----  tools/perf/util/machine.h
> > |  2 +-
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 2f1ad29..bbfb9e0 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -64,8 +64,8 @@ int machine__init(struct machine *machine, const
> > char *root_dir, pid_t pid)
> >
> >  	machine->id_hdr_size = 0;
> >  	machine->kptr_restrict_warned = false;
> > -	machine->comm_exec = false;
> >  	machine->kernel_start = 0;
> > +	atomic_set(&machine->comm_exec, 0);
> >
> >  	memset(machine->vmlinux_maps, 0, sizeof(machine-
> >vmlinux_maps));
> >
> > @@ -238,14 +238,15 @@ struct machine *machines__add(struct machines
> > *machines, pid_t pid,
> >
> >  void machines__set_comm_exec(struct machines *machines, bool
> > comm_exec)  {
> > +	int exec = comm_exec ? 1 : 0;
> >  	struct rb_node *nd;
> >
> > -	machines->host.comm_exec = comm_exec;
> > +	atomic_set(&machines->host.comm_exec, exec);
> >
> >  	for (nd = rb_first(&machines->guests); nd; nd = rb_next(nd)) {
> >  		struct machine *machine = rb_entry(nd, struct machine,
> rb_node);
> >
> > -		machine->comm_exec = comm_exec;
> > +		atomic_set(&machine->comm_exec, exec);
> >  	}
> >  }
> >
> > @@ -505,7 +506,7 @@ struct thread *machine__find_thread(struct
> machine
> > *machine, pid_t pid,  struct comm *machine__thread_exec_comm(struct
> machine *machine,
> >  				       struct thread *thread)
> >  {
> > -	if (machine->comm_exec)
> > +	if (atomic_read(&machine->comm_exec))
> >  		return thread__exec_comm(thread);
> >  	else
> >  		return thread__comm(thread);
> > @@ -521,7 +522,7 @@ int machine__process_comm_event(struct machine
> *machine, union perf_event *event
> >  	int err = 0;
> >
> >  	if (exec)
> > -		machine->comm_exec = true;
> > +		atomic_set(&machine->comm_exec, 1);
> >
> >  	if (dump_trace)
> >  		perf_event__fprintf_comm(event, stdout); diff --git
> > a/tools/perf/util/machine.h b/tools/perf/util/machine.h index
> > 64663eb..fb3c2a2 100644
> > --- a/tools/perf/util/machine.h
> > +++ b/tools/perf/util/machine.h
> > @@ -38,7 +38,7 @@ struct machine {
> >  	struct rb_node	  rb_node;
> >  	pid_t		  pid;
> >  	u16		  id_hdr_size;
> > -	bool		  comm_exec;
> > +	atomic_t	  comm_exec;
> >  	bool		  kptr_restrict_warned;
> >  	char		  *root_dir;
> >  	struct threads    threads[THREADS__TABLE_SIZE];
> > --
> > 2.5.5