From nobody Thu Dec 18 04:49:35 2025 Received: from mail-pl1-f201.google.com (mail-pl1-f201.google.com [209.85.214.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A37A1F3BA8 for ; Mon, 24 Mar 2025 22:35:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742855715; cv=none; b=Rl5tN84aPn+5AEkV3gipdMp56GZNHosngMlfHIVGFss7gOB70yVb6D/zx5ZHHKN7g07TEzsPBaihJrAWmYzS1DeJgiBPujWdEkXRze2bRUUMqYKq9G5i+5Sipq+w1RFkop19TvW60n/G1m/stNjFHzepw8XdlI6B1DFxt0RMYQI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742855715; c=relaxed/simple; bh=NOwsl0wEuL5koR3NXHep7k7Xz0LqWmUdBTPVtZ/ePi4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Content-Type; b=NnXr8Tfqg0z70vzX0oc0pwimPaQIE5O3xPDDZzaYfxxfgW5cUjT2Ik7eAs2sMnG/k7B83q5mQqJuKB0r7EML3wRn7fRpzGf7nQ8W5mHYAZ2IA5DsKt5kAt7EmlFujoalPYEiHvTOwQ4Qvr/HoW6wEV/HylDF32ic9Eyia0DZ0Rs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=Q7VTGG4b; arc=none smtp.client-ip=209.85.214.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Q7VTGG4b" Received: by mail-pl1-f201.google.com with SMTP id d9443c01a7336-22647ff3cf5so69297965ad.0 for ; Mon, 24 Mar 2025 15:35:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1742855713; x=1743460513; darn=vger.kernel.org; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :from:to:cc:subject:date:message-id:reply-to; bh=xG9Nmmv36pxRcjjEqaCPf9FJZsaOcmEhpQ4zx8j/rkI=; b=Q7VTGG4bNM1ImOXBVkZvdFX4F4gUmNDX1777FSNDRaAdKHshQSnEf/Y+4RgwgcqWi6 i1xy4LEvaMzYQjOn9YMzZb4VRySx4FmnT5CsOop7z3AhFZEOrhRyY4N9wFRoRT+boi2D Q+uuliqDoFQQstXCY7pQGPb+mFmKTvxq9r3aAgq/Smci6+gwmmzkKeLrMoO99x4w8erk 702yFgJA6vCSGTGWVj3s6PIxUesqaAR2EQty//kZuzUdCShwX6cURcUUHM3pR4wMby3D fLfDPbIpBkbOBLMoH/khBp6xMcn5rLkzc+uT64x1RbTGs/yCvM0ddzrf/D0uV/h9Ma/d OWUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1742855713; x=1743460513; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=xG9Nmmv36pxRcjjEqaCPf9FJZsaOcmEhpQ4zx8j/rkI=; b=PK7VHvIK7jK/WAWor12zxbhKBUDBehRdeY1yasLmRr8lDp8FOdDSO+4uvBM7sEjILI z6pjYcebRWCofBa274Ea/W78gP0x8FU4IdqZku9eL8vXVVDItcJwIBDr+g0m5YKHIDzN DctGz8g6DBXCsU00gnLJlX/Cws4jibwyfMfaA158s/M6LN/nV6LpixRhFVWhxLXFknhs iMPKy89u0yQZupneQ3D/a4W3Ngp4mXbDUdcyUgVLu6Png5LWApFwHKVGqx/G4c3U+49G z37aLVPkQYvMl/G7+fqQYsSP1yT5lGvwSeg7ujKCo1KYNJUZqYgsRFjEDyeAamuhQ0sR P60A== X-Forwarded-Encrypted: i=1; AJvYcCUFo9h2wGy4X6uacxYFdQlahFuzmCmGUAvfGohvNbf8LjxQKU6eaPRio6LH9+5jLGY36qyTb+ZYToYOLIQ=@vger.kernel.org X-Gm-Message-State: AOJu0YxNzq4WFG5+cshI1nHtUYNZfnNVyVOmgl4J8fMnNBmdSQVUHEeE 9NZY/08dH2k/I4Lt3AhuU5GCTMYJJwFMZW+U6FW630R76ouJaztZwsP94Q1IjPlA4qzU4jOWLpl U+P3V2w== X-Google-Smtp-Source: AGHT+IEF9FZXzvHy0V0vOJKqofApvJzXbXsG6N9/WfJN4q/K2ePlAkMljkL6PeMHdQ9rJKW/Yt82ONv5kZRR X-Received: from pffk14.prod.google.com ([2002:aa7:88ce:0:b0:730:8e17:ed13]) (user=irogers job=prod-delivery.src-stubby-dispatcher) by 2002:a17:902:ce82:b0:223:525b:2a7 with SMTP id d9443c01a7336-22780d7c73bmr200774775ad.15.1742855712732; Mon, 24 Mar 2025 15:35:12 -0700 (PDT) Date: Mon, 24 Mar 2025 15:34:46 -0700 In-Reply-To: <20250324223452.208081-1-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250324223452.208081-1-irogers@google.com> X-Mailer: git-send-email 2.49.0.395.g12beb8f557-goog Message-ID: <20250324223452.208081-9-irogers@google.com> Subject: [PATCH v1 08/14] perf intel-tpebs: Refactor tpebs_results list From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Kan Liang , Weilin Wang , James Clark , Xu Yang , John Garry , Howard Chu , Levi Yun , Dominique Martinet , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" evsel names and metric-ids are used for matching but this can be problematic, for example, multiple occurrences of the same retirement latency event become a single event for the record. Change the name of the record events so they are unique and reflect the evsel of the retirement latency event that opens them (the retirement latency event's evsel address is embedded within them). This allows an evsel based close to close the event when the retirement latency event is closed. This is important as perf stat has an evlist and the session listen to the record events has an evlist, knowing which event should remove the tpebs_retire_lat can't be tied to an evlist list as there is more than 1, so closing which evlist should cause the tpebs to stop? Using the evsel and the last one out doing the tpebs_stop is cleaner. Signed-off-by: Ian Rogers --- tools/perf/builtin-stat.c | 2 - tools/perf/util/evlist.c | 1 - tools/perf/util/evsel.c | 2 +- tools/perf/util/intel-tpebs.c | 147 ++++++++++++++++++++-------------- tools/perf/util/intel-tpebs.h | 4 +- 5 files changed, 90 insertions(+), 66 deletions(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 68ea7589c143..80e491bd775b 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -681,8 +681,6 @@ static enum counter_recovery stat_handle_error(struct e= vsel *counter) if (child_pid !=3D -1) kill(child_pid, SIGTERM); =20 - tpebs_delete(); - return COUNTER_FATAL; } =20 diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index c1a04141aed0..0a21da4f990f 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -183,7 +183,6 @@ void evlist__delete(struct evlist *evlist) if (evlist =3D=3D NULL) return; =20 - tpebs_delete(); evlist__free_stats(evlist); evlist__munmap(evlist); evlist__close(evlist); diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 121283f2f382..554252ed1aab 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -2759,7 +2759,7 @@ int evsel__open(struct evsel *evsel, struct perf_cpu_= map *cpus, void evsel__close(struct evsel *evsel) { if (evsel__is_retire_lat(evsel)) - tpebs_delete(); + evsel__tpebs_close(evsel); perf_evsel__close(&evsel->core); perf_evsel__free_id(&evsel->core); } diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c index 9116cdc6981e..500c472f7059 100644 --- a/tools/perf/util/intel-tpebs.c +++ b/tools/perf/util/intel-tpebs.c @@ -34,10 +34,10 @@ static struct child_process tpebs_cmd; =20 struct tpebs_retire_lat { struct list_head nd; - /* Event name */ - char *name; - /* Event name with the TPEBS modifier R */ - const char *tpebs_name; + /** @evsel: The evsel that opened the retire_lat event. */ + struct evsel *evsel; + /** @event: Event passed to perf record. */ + char *event; /* Count of retire_latency values found in sample data */ size_t count; /* Sum of all the retire_latency values in sample data */ @@ -48,6 +48,8 @@ struct tpebs_retire_lat { bool started; }; =20 +static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel= ); + static int evsel__tpebs_start_perf_record(struct evsel *evsel, int control= _fd[], int ack_fd[]) { const char **record_argv; @@ -84,7 +86,7 @@ static int evsel__tpebs_start_perf_record(struct evsel *e= vsel, int control_fd[], =20 list_for_each_entry(t, &tpebs_results, nd) { record_argv[i++] =3D "-e"; - record_argv[i++] =3D t->name; + record_argv[i++] =3D t->event; } record_argv[i++] =3D NULL; assert(i =3D=3D 10 + 2 * tpebs_event_size || i =3D=3D 8 + 2 * tpebs_event= _size); @@ -107,27 +109,20 @@ static int process_sample_event(const struct perf_too= l *tool __maybe_unused, struct evsel *evsel, struct machine *machine __maybe_unused) { - int ret =3D 0; - const char *evname; struct tpebs_retire_lat *t; =20 - evname =3D evsel__name(evsel); - + t =3D tpebs_retire_lat__find(evsel); + if (!t) + return -EINVAL; /* * Need to handle per core results? We are assuming average retire * latency value will be used. Save the number of samples and the sum of * retire latency value for each event. */ - list_for_each_entry(t, &tpebs_results, nd) { - if (!strcmp(evname, t->name)) { - t->count +=3D 1; - t->sum +=3D sample->retire_lat; - t->val =3D (double) t->sum / t->count; - break; - } - } - - return ret; + t->count +=3D 1; + t->sum +=3D sample->retire_lat; + t->val =3D (double) t->sum / t->count; + return 0; } =20 static int process_feature_event(struct perf_session *session, @@ -182,50 +177,93 @@ static int tpebs_stop(void) return ret; } =20 -static char *evsel__tpebs_name(struct evsel *evsel) +/** + * evsel__tpebs_event() - Create string event encoding to pass to `perf re= cord`. + */ +static int evsel__tpebs_event(struct evsel *evsel, char **event) { char *name, *modifier; + int ret; =20 name =3D strdup(evsel->name); - if (!name) - return NULL; + if (!*name) + return -ENOMEM; =20 modifier =3D strrchr(name, 'R'); if (!modifier) { - pr_err("Tpebs event missing modifier '%s'\n", name); - free(name); - return NULL; + ret =3D -EINVAL; + goto out; } - *modifier =3D 'p'; - return name; + modifier =3D strchr(name, ':'); + if (!modifier) + modifier =3D strrchr(name, '/'); + if (!modifier) { + ret =3D -EINVAL; + goto out; + } + *modifier =3D '\0'; + if (asprintf(event, "%s/name=3Dtpebs_event_%p/%s", name, evsel, modifier = + 1) > 0) + ret =3D 0; + else + ret =3D -ENOMEM; +out: + if (ret) + pr_err("Tpebs event modifier broken '%s'\n", evsel->name); + free(name); + return ret; } =20 static struct tpebs_retire_lat *tpebs_retire_lat__new(struct evsel *evsel) { struct tpebs_retire_lat *result =3D zalloc(sizeof(*result)); + int ret; =20 if (!result) return NULL; =20 - result->tpebs_name =3D evsel->name; - result->name =3D evsel__tpebs_name(evsel); - if (!result->name) { + ret =3D evsel__tpebs_event(evsel, &result->event); + if (ret) { free(result); return NULL; } + result->evsel =3D evsel; list_add_tail(&result->nd, &tpebs_results); return result; } =20 +static void tpebs_retire_lat__delete(struct tpebs_retire_lat *r) +{ + zfree(&r->event); + free(r); +} + static struct tpebs_retire_lat *tpebs_retire_lat__find(struct evsel *evsel) { struct tpebs_retire_lat *t; + uint64_t num; =20 + /* + * Evsels will match for evlist with the retirement latency event. The + * name with "tpebs_event_" prefix will be present on events being read + * from `perf record`. + */ + if (evsel__is_retire_lat(evsel)) { + list_for_each_entry(t, &tpebs_results, nd) { + if (t->evsel =3D=3D evsel) + return t; + } + return NULL; + } + assert(!strncmp(evsel->name, "tpebs_event_", 12)); + errno =3D 0; + num =3D strtoull(evsel->name + 12, NULL, 16); + if (errno) { + pr_err("Bad evsel for tpebs find '%s'\n", evsel->name); + return NULL; + } list_for_each_entry(t, &tpebs_results, nd) { - if (t->tpebs_name =3D=3D evsel->name || - !strcmp(t->tpebs_name, evsel->name) || - (evsel->metric_id && !strcmp(t->tpebs_name, evsel->metric_id))) + if ((uint64_t)t->evsel =3D=3D num) return t; } return NULL; @@ -362,8 +400,12 @@ int evsel__tpebs_open(struct evsel *evsel) close(ack_fd[0]); close(ack_fd[1]); } - if (ret) - tpebs_delete(); + if (ret) { + struct tpebs_retire_lat *t =3D tpebs_retire_lat__find(evsel); + + list_del_init(&t->nd); + tpebs_retire_lat__delete(t); + } return ret; } =20 @@ -413,34 +455,19 @@ int tpebs_set_evsel(struct evsel *evsel, int cpu_map_= idx, int thread) return 0; } =20 -static void tpebs_retire_lat__delete(struct tpebs_retire_lat *r) -{ - zfree(&r->name); - free(r); -} - - -/* - * tpebs_delete - delete tpebs related data and stop the created thread and - * process by calling tpebs_stop(). - * - * This function is called from evlist_delete() and also from builtin-stat - * stat_handle_error(). If tpebs_start() is called from places other then = perf - * stat, need to ensure tpebs_delete() is also called to safely free mem a= nd - * close the data read thread and the forked perf record process. +/** + * evsel__tpebs_close() - delete tpebs related data. If the last event, st= op the + * created thread and process by calling tpebs_stop(). * - * This function is also called in evsel__close() to be symmetric with - * tpebs_start() being called in evsel__open(). We will update this call s= ite - * when move tpebs_start() to evlist level. + * This function is called in evsel__close() to be symmetric with + * evsel__tpebs_open() being called in evsel__open(). */ -void tpebs_delete(void) +void evsel__tpebs_close(struct evsel *evsel) { - struct tpebs_retire_lat *r, *rtmp; + struct tpebs_retire_lat *t =3D tpebs_retire_lat__find(evsel); =20 - tpebs_stop(); + tpebs_retire_lat__delete(t); =20 - list_for_each_entry_safe(r, rtmp, &tpebs_results, nd) { - list_del_init(&r->nd); - tpebs_retire_lat__delete(r); - } + if (list_empty(&tpebs_results)) + tpebs_stop(); } diff --git a/tools/perf/util/intel-tpebs.h b/tools/perf/util/intel-tpebs.h index 88d70bb37971..6ff92f5de9b4 100644 --- a/tools/perf/util/intel-tpebs.h +++ b/tools/perf/util/intel-tpebs.h @@ -11,7 +11,7 @@ struct evsel; =20 extern bool tpebs_recording; int evsel__tpebs_open(struct evsel *evsel); -void tpebs_delete(void); +void evsel__tpebs_close(struct evsel *evsel); int tpebs_set_evsel(struct evsel *evsel, int cpu_map_idx, int thread); =20 #else @@ -21,7 +21,7 @@ static inline int evsel__tpebs_open(struct evsel *evsel _= _maybe_unused) return 0; } =20 -static inline void tpebs_delete(void) {}; +static inline void evsel__tpebs_close(struct evsel *evsed __maybe_unused) = {}; =20 static inline int tpebs_set_evsel(struct evsel *evsel __maybe_unused, int cpu_map_idx __maybe_unused, --=20 2.49.0.395.g12beb8f557-goog