lkml.org 
[lkml]   [2022]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v1 2/4] perf mutex with nsinfo: Updated pthread_mutex_t usage
On Wed, Jul 27, 2022 at 4:20 AM <gpavithrasha@gmail.com> wrote:
>
> From: pavithra <gpavithrasha@gmail.com>
>
> Updated usage of pthread_mutex_t with nsinfo
> with the new wrapped lock(struct) in mutex.h
> (remove data races).

It doesn't look like you updated the usage with new wrapping.
Instead you introduced new usage. Then I think you need to
describe why it's needed and what problems it solves.

>
> Signed-off-by: pavithra <gpavithrasha@gmail.com>
> ---
> tools/perf/builtin-inject.c | 6 +++---
> tools/perf/util/map.c | 2 ++
> tools/perf/util/probe-event.c | 6 +++++-
> tools/perf/util/symbol.c | 2 +-
> 4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 372ecb3e2c06..81eaed8da207 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -388,10 +388,10 @@ static int perf_event__repipe_id_index(struct perf_session *session,
> static int dso__read_build_id(struct dso *dso)
> {
> if (dso->has_build_id)
> - return 0;
> -
> + return 0;
> +
> if (filename__read_build_id(dso->long_name, dso->build_id,
> - sizeof(dso->build_id)) > 0) {
> + sizeof(dso->build_id)) > 0) {

Unrelated whitespace changes.. is it really needed?

Also I'm not sure you are working on acme/perf/core branch.

> dso->has_build_id = true;
> return 0;
> }
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 5b83ed1ebbd6..2ef5fe0cc53c 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -214,8 +214,10 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
> if (!(prot & PROT_EXEC))
> dso__set_loaded(dso);
> }
> + mutex_lock(&dso->lock);
> dso->nsinfo = nsi;
> dso__put(dso);
> + mutex_unlock(&dso->lock);
> }
> return map;
> out_delete:
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 91cab5f669d2..e527f2612ba4 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -38,6 +38,7 @@
> #include "session.h"
> #include "string2.h"
> #include "strbuf.h"
> +#include "mutex.h"
>
> #include <subcmd/pager.h>
> #include <linux/ctype.h>
> @@ -171,8 +172,11 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user)
> struct map *map;
>
> map = dso__new_map(target);
> - if (map && map->dso)
> + if (map && map->dso) {
> + mutex_lock(&map->dso->lock);
> map->dso->nsinfo = nsinfo__get(nsi);
> + mutex_unlock(&map->dso->lock);
> + }
> return map;
> } else {
> return kernel_get_module_map(target);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 342be12cfa1e..4b711b13f915 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1619,7 +1619,7 @@ int dso__load(struct dso *dso, struct map *map)
> struct nscookie nsc;
> char newmapname[PATH_MAX];
> const char *map_path = dso->long_name;
> -
> + mutex_lock(&dso->lock);

No matching unlock?

Thanks,
Namhyung


> perfmap = strncmp(dso->name, "/tmp/perf-", 10) == 0;
> if (perfmap) {
> if (dso->nsinfo && (dso__find_perf_map(newmapname,
> --
> 2.25.1
>

\
 
 \ /
  Last update: 2022-07-28 00:34    [W:2.138 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site