Em Thu, Dec 10, 2015 at 08:52:46AM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
> From: Arnaldo Carvalho de Melo [mailto:a...@kernel.org]
> >
> >Em Wed, Dec 09, 2015 at 11:11:18AM +0900, Masami Hiramatsu escreveu:
> >> +++ b/tools/perf/util/symbol-elf.c
> >> @@ -1045,6 +1045,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
> >>                            /* kmaps already got it */
> >>                            map__put(curr_map);
> >>                            dsos__add(&map->groups->machine->dsos, 
> >> curr_dso);
> >> +                          /* curr_map and machine->dsos already got it */
> >> +                          dso__put(curr_dso);
> >>                            dso__set_loaded(curr_dso, map->type);
> >>                    } else
> >>                            curr_dso = curr_map->dso;
> >
> >Right, to make the code smaller, how about doing it this way, i.e. drop
> >the reference once we have that curr_dso object with a ref held by
> >curr_map, if curr_map doesn't get it, then we don't need and will drop
> >it anyway:
> 
> But as above code, curr_dso is passed to dsos__add after curr_map is put.
> Even if the curr_map is hold by kmaps, isn't the kmaps controlled by other 
> pthreads?
> If no, I'm OK if we move above dsos__add() before map__put() for safety, 
> because
> curr_dso is held by curr_map at that point.

Good catch, so I'll do as you suggest and move dsos__add() to before we
drop that map reference, as then we're sure that we hold it and it holds
the dso.
 
> Thank you,
> 
> >
> >diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> >index 53f19968bfa2..84d787074152 100644
> >--- a/tools/perf/util/symbol-elf.c
> >+++ b/tools/perf/util/symbol-elf.c
> >@@ -1026,8 +1026,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
> >                             curr_dso->long_name_len = dso->long_name_len;
> >                             curr_map = map__new2(start, curr_dso,
> >                                                  map->type);
> >+                            dso__put(curr_dso);
> >                             if (curr_map == NULL) {
> >-                                    dso__put(curr_dso);
> >                                     goto out_elf_end;
> >                             }
> >                             if (adjust_kernel_syms) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to