From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE894D66; Wed, 29 Nov 2023 16:56:03 -0800 (PST) Received: by mail-pg1-f169.google.com with SMTP id 41be03b00d2f7-5c6263fce87so86548a12.0; Wed, 29 Nov 2023 16:56:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701305763; x=1701910563; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cmiAKCI1fSv/7HCnkC6OaMmQ8+p+YKtjiHGASgJDn84=; b=s/EE4uc3SQcyZYfgKlUNJSNAhJMH3sZWMO6uut7RMCmJrwvkFlDDlxaMAgWzENNu4H +AnxguUCeTN48ljQLyQirjiXRYpkbHpjHBFlbBMw46JMQa2vrONajY+BRNtYKmNU6qoG oK74Fdp8FiSNKBFdiQ1DNlkGrFR/em0Yf/jQBxAxJB5pOX4QUXB/Zi6ERWlKIzcqzsUm 6+HBJz7GD9u4RzukzvfnSLC32X8sxuY30edBkLKEF6Wu55HxVRiDUZT98uUocNcadDpb QlcLQo7mWQ+j199iv7KuI0ADqnVVdmQylojc5uUz4MqW33uBpKYczDdVGtmZveFtpyc0 YWCw== X-Gm-Message-State: AOJu0YwVbnMit9cOpcE7PyBtxoO8CkfwqvTg+KpUjpXa3SMnJsJdw/Oy U2c+4WFpy6N1fWo2SDGU8W9zraThMQZqnabuyeQ= X-Google-Smtp-Source: AGHT+IHza8dNTvmjlkCVaw8H+Dj7D7xGV1OOxGXgTsQ18HrQy5bBI0i6eDlyG37Doh8ih/q3/oM43lBtWeKlyhTqIl4= X-Received: by 2002:a17:90b:310d:b0:286:1809:1517 with SMTP id gc13-20020a17090b310d00b0028618091517mr6586512pjb.7.1701305763212; Wed, 29 Nov 2023 16:56:03 -0800 (PST) Precedence: bulk X-Mailing-List: linux-perf-users@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231127220902.1315692-1-irogers@google.com> <20231127220902.1315692-2-irogers@google.com> In-Reply-To: <20231127220902.1315692-2-irogers@google.com> From: Namhyung Kim Date: Wed, 29 Nov 2023 16:55:52 -0800 Message-ID: Subject: Re: [PATCH v5 01/50] perf comm: Use regular mutex To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Nick Terrell , Kan Liang , Andi Kleen , Kajol Jain , Athira Rajeev , Huacai Chen , Masami Hiramatsu , Vincent Whitchurch , "Steinar H. Gunderson" , Liam Howlett , Miguel Ojeda , Colin Ian King , Dmitrii Dolgov <9erthalion6@gmail.com>, Yang Jihong , Ming Wang , James Clark , K Prateek Nayak , Sean Christopherson , Leo Yan , Ravi Bangoria , German Gomez , Changbin Du , Paolo Bonzini , Li Dong , Sandipan Das , liuwenyu , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Guilherme Amadio Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Nov 27, 2023 at 2:09=E2=80=AFPM Ian Rogers wro= te: > > The rwsem is only after used for writing so switch to a mutex that has > better error checking. > > Fixes: 7a8f349e9d14 ("perf rwsem: Add debug mode that uses a mutex") I think we talked about fixing this separately, no? > Signed-off-by: Ian Rogers > --- > 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 =3D {.lock =3D PTHREAD_RWLOCK_I= NITIALIZER,}; > +static struct mutex comm_str_lock =3D {.lock =3D PTHREAD_ERRORCHECK_MUTE= X_INITIALIZER_NP,}; IIUC it has a problem with musl libc. Actually I think it's better to hide the field and the pthread initializer under some macro like MUTEX_INITIALIZER or DEFINE_MUTEX() like in the kernel. Thanks, Namhyung > > 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 *s= tr, struct rb_root *root) > { > struct comm_str *cs; > > - down_write(&comm_str_lock); > + mutex_lock(&comm_str_lock); > cs =3D __comm_str__findnew(str, root); > - up_write(&comm_str_lock); > + mutex_unlock(&comm_str_lock); > > return cs; > } > -- > 2.43.0.rc1.413.gea7ed67945-goog >