Messages in this thread | | | From | Ian Rogers <> | Date | Sun, 5 Nov 2023 13:35:42 -0800 | Subject | Re: [PATCH v4 01/53] perf comm: Use regular mutex |
| |
On Sun, Nov 5, 2023 at 9:32 AM Namhyung Kim <namhyung@kernel.org> wrote: > > Hi Ian, > > On Thu, Nov 2, 2023 at 10:58 AM Ian Rogers <irogers@google.com> wrote: > > > > The rwsem is only after used for writing so switch to a mutex that has > > better error checking. > > Hmm.. ok. It doesn't make sense to use rwsem without readers. > > > > > Fixes: 7a8f349e9d14 ("perf rwsem: Add debug mode that uses a mutex") > > But I'm not sure this is a fix. Other than that,
Thanks Namhyung, it fixes the case that you enable RWS_ERRORCHECK in rwsem.h as the rwsem static initialization is wrong for a mutex.
Ian
> > Signed-off-by: Ian Rogers <irogers@google.com> > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > Thanks, > Namhyung > > > > --- > > tools/perf/util/comm.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c > > index afb8d4fd2644..4ae7bc2aa9a6 100644 > > --- a/tools/perf/util/comm.c > > +++ b/tools/perf/util/comm.c > > @@ -17,7 +17,7 @@ struct comm_str { > > > > /* Should perhaps be moved to struct machine */ > > static struct rb_root comm_str_root; > > -static struct rw_semaphore comm_str_lock = {.lock = PTHREAD_RWLOCK_INITIALIZER,}; > > +static struct mutex comm_str_lock = {.lock = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP,}; > > > > static struct comm_str *comm_str__get(struct comm_str *cs) > > { > > @@ -30,9 +30,9 @@ static struct comm_str *comm_str__get(struct comm_str *cs) > > static void comm_str__put(struct comm_str *cs) > > { > > if (cs && refcount_dec_and_test(&cs->refcnt)) { > > - down_write(&comm_str_lock); > > + mutex_lock(&comm_str_lock); > > rb_erase(&cs->rb_node, &comm_str_root); > > - up_write(&comm_str_lock); > > + mutex_unlock(&comm_str_lock); > > zfree(&cs->str); > > free(cs); > > } > > @@ -98,9 +98,9 @@ static struct comm_str *comm_str__findnew(const char *str, struct rb_root *root) > > { > > struct comm_str *cs; > > > > - down_write(&comm_str_lock); > > + mutex_lock(&comm_str_lock); > > cs = __comm_str__findnew(str, root); > > - up_write(&comm_str_lock); > > + mutex_unlock(&comm_str_lock); > > > > return cs; > > } > > -- > > 2.42.0.869.gea05f2083d-goog > >
| |