Skip to content

Commit 13ca628

Browse files
captain5050acmel
captain5050
authored and
acmel
committed
perf comm: Add reference count checking to 'struct comm_str'
Reference count checking of an rbtree is troublesome as each pointer should have a reference, switch to using a sorted array. Remove an indirection by embedding the reference count with the string. Use pthread_once to safely initialize the comm_strs and reader writer mutex. Signed-off-by: Ian Rogers <irogers@google.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com> Cc: Ben Gainey <ben.gainey@arm.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: James Clark <james.clark@arm.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: K Prateek Nayak <kprateek.nayak@amd.com> Cc: Kajol Jain <kjain@linux.ibm.com> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Li Dong <lidong@vivo.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Oliver Upton <oliver.upton@linux.dev> Cc: Paran Lee <p4ranlee@gmail.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ravi Bangoria <ravi.bangoria@amd.com> Cc: Sun Haiyong <sunhaiyong@loongson.cn> Cc: Tim Chen <tim.c.chen@linux.intel.com> Cc: Yanteng Si <siyanteng@loongson.cn> Cc: Yicong Yang <yangyicong@hisilicon.com> Link: https://lore.kernel.org/r/20240507183545.1236093-6-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
1 parent a8cd476 commit 13ca628

1 file changed

Lines changed: 126 additions & 70 deletions

File tree

‎tools/perf/util/comm.c‎

Lines changed: 126 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,164 @@
11
// SPDX-License-Identifier: GPL-2.0
22
#include "comm.h"
33
#include <errno.h>
4-
#include <stdlib.h>
5-
#include <stdio.h>
64
#include <string.h>
5+
#include <internal/rc_check.h>
76
#include <linux/refcount.h>
8-
#include <linux/rbtree.h>
97
#include <linux/zalloc.h>
108
#include "rwsem.h"
119

12-
struct comm_str {
13-
char *str;
14-
struct rb_node rb_node;
10+
DECLARE_RC_STRUCT(comm_str) {
1511
refcount_t refcnt;
12+
char str[];
1613
};
1714

18-
/* Should perhaps be moved to struct machine */
19-
static struct rb_root comm_str_root;
20-
static struct rw_semaphore comm_str_lock = {.lock = PTHREAD_RWLOCK_INITIALIZER,};
15+
static struct comm_strs {
16+
struct rw_semaphore lock;
17+
struct comm_str **strs;
18+
int num_strs;
19+
int capacity;
20+
} _comm_strs;
21+
22+
static void comm_strs__init(void)
23+
{
24+
init_rwsem(&_comm_strs.lock);
25+
_comm_strs.capacity = 16;
26+
_comm_strs.num_strs = 0;
27+
_comm_strs.strs = calloc(16, sizeof(*_comm_strs.strs));
28+
}
29+
30+
static struct comm_strs *comm_strs__get(void)
31+
{
32+
static pthread_once_t comm_strs_type_once = PTHREAD_ONCE_INIT;
33+
34+
pthread_once(&comm_strs_type_once, comm_strs__init);
35+
36+
return &_comm_strs;
37+
}
38+
39+
static refcount_t *comm_str__refcnt(struct comm_str *cs)
40+
{
41+
return &RC_CHK_ACCESS(cs)->refcnt;
42+
}
43+
44+
static const char *comm_str__str(const struct comm_str *cs)
45+
{
46+
return &RC_CHK_ACCESS(cs)->str[0];
47+
}
2148

2249
static struct comm_str *comm_str__get(struct comm_str *cs)
2350
{
24-
if (cs && refcount_inc_not_zero(&cs->refcnt))
25-
return cs;
51+
struct comm_str *result;
52+
53+
if (RC_CHK_GET(result, cs))
54+
refcount_inc_not_zero(comm_str__refcnt(cs));
2655

27-
return NULL;
56+
return result;
2857
}
2958

3059
static void comm_str__put(struct comm_str *cs)
3160
{
32-
if (cs && refcount_dec_and_test(&cs->refcnt)) {
33-
down_write(&comm_str_lock);
34-
rb_erase(&cs->rb_node, &comm_str_root);
35-
up_write(&comm_str_lock);
36-
zfree(&cs->str);
37-
free(cs);
61+
if (cs && refcount_dec_and_test(comm_str__refcnt(cs))) {
62+
struct comm_strs *comm_strs = comm_strs__get();
63+
int i;
64+
65+
down_write(&comm_strs->lock);
66+
for (i = 0; i < comm_strs->num_strs; i++) {
67+
if (comm_strs->strs[i] == cs)
68+
break;
69+
}
70+
for (; i < comm_strs->num_strs - 1; i++)
71+
comm_strs->strs[i] = comm_strs->strs[i + 1];
72+
73+
comm_strs->num_strs--;
74+
up_write(&comm_strs->lock);
75+
RC_CHK_FREE(cs);
76+
} else {
77+
RC_CHK_PUT(cs);
3878
}
3979
}
4080

41-
static struct comm_str *comm_str__alloc(const char *str)
81+
static struct comm_str *comm_str__new(const char *str)
4282
{
43-
struct comm_str *cs;
83+
struct comm_str *result = NULL;
84+
RC_STRUCT(comm_str) *cs;
4485

45-
cs = zalloc(sizeof(*cs));
46-
if (!cs)
47-
return NULL;
48-
49-
cs->str = strdup(str);
50-
if (!cs->str) {
51-
free(cs);
52-
return NULL;
86+
cs = malloc(sizeof(*cs) + strlen(str) + 1);
87+
if (ADD_RC_CHK(result, cs)) {
88+
refcount_set(comm_str__refcnt(result), 1);
89+
strcpy(&cs->str[0], str);
5390
}
91+
return result;
92+
}
5493

55-
refcount_set(&cs->refcnt, 1);
94+
static int comm_str__cmp(const void *_lhs, const void *_rhs)
95+
{
96+
const struct comm_str *lhs = *(const struct comm_str * const *)_lhs;
97+
const struct comm_str *rhs = *(const struct comm_str * const *)_rhs;
5698

57-
return cs;
99+
return strcmp(comm_str__str(lhs), comm_str__str(rhs));
58100
}
59101

60-
static
61-
struct comm_str *__comm_str__findnew(const char *str, struct rb_root *root)
102+
static int comm_str__search(const void *_key, const void *_member)
62103
{
63-
struct rb_node **p = &root->rb_node;
64-
struct rb_node *parent = NULL;
65-
struct comm_str *iter, *new;
66-
int cmp;
67-
68-
while (*p != NULL) {
69-
parent = *p;
70-
iter = rb_entry(parent, struct comm_str, rb_node);
71-
72-
/*
73-
* If we race with comm_str__put, iter->refcnt is 0
74-
* and it will be removed within comm_str__put call
75-
* shortly, ignore it in this search.
76-
*/
77-
cmp = strcmp(str, iter->str);
78-
if (!cmp && comm_str__get(iter))
79-
return iter;
80-
81-
if (cmp < 0)
82-
p = &(*p)->rb_left;
83-
else
84-
p = &(*p)->rb_right;
85-
}
104+
const char *key = _key;
105+
const struct comm_str *member = *(const struct comm_str * const *)_member;
86106

87-
new = comm_str__alloc(str);
88-
if (!new)
89-
return NULL;
107+
return strcmp(key, comm_str__str(member));
108+
}
109+
110+
static struct comm_str *__comm_strs__find(struct comm_strs *comm_strs, const char *str)
111+
{
112+
struct comm_str **result;
90113

91-
rb_link_node(&new->rb_node, parent, p);
92-
rb_insert_color(&new->rb_node, root);
114+
result = bsearch(str, comm_strs->strs, comm_strs->num_strs, sizeof(struct comm_str *),
115+
comm_str__search);
93116

94-
return new;
117+
if (!result)
118+
return NULL;
119+
120+
return comm_str__get(*result);
95121
}
96122

97-
static struct comm_str *comm_str__findnew(const char *str, struct rb_root *root)
123+
static struct comm_str *comm_strs__findnew(const char *str)
98124
{
99-
struct comm_str *cs;
125+
struct comm_strs *comm_strs = comm_strs__get();
126+
struct comm_str *result;
100127

101-
down_write(&comm_str_lock);
102-
cs = __comm_str__findnew(str, root);
103-
up_write(&comm_str_lock);
128+
if (!comm_strs)
129+
return NULL;
104130

105-
return cs;
131+
down_read(&comm_strs->lock);
132+
result = __comm_strs__find(comm_strs, str);
133+
up_read(&comm_strs->lock);
134+
if (result)
135+
return result;
136+
137+
down_write(&comm_strs->lock);
138+
result = __comm_strs__find(comm_strs, str);
139+
if (!result) {
140+
if (comm_strs->num_strs == comm_strs->capacity) {
141+
struct comm_str **tmp;
142+
143+
tmp = reallocarray(comm_strs->strs,
144+
comm_strs->capacity + 16,
145+
sizeof(*comm_strs->strs));
146+
if (!tmp) {
147+
up_write(&comm_strs->lock);
148+
return NULL;
149+
}
150+
comm_strs->strs = tmp;
151+
comm_strs->capacity += 16;
152+
}
153+
result = comm_str__new(str);
154+
if (result) {
155+
comm_strs->strs[comm_strs->num_strs++] = result;
156+
qsort(comm_strs->strs, comm_strs->num_strs, sizeof(struct comm_str *),
157+
comm_str__cmp);
158+
}
159+
}
160+
up_write(&comm_strs->lock);
161+
return result;
106162
}
107163

108164
struct comm *comm__new(const char *str, u64 timestamp, bool exec)
@@ -115,7 +171,7 @@ struct comm *comm__new(const char *str, u64 timestamp, bool exec)
115171
comm->start = timestamp;
116172
comm->exec = exec;
117173

118-
comm->comm_str = comm_str__findnew(str, &comm_str_root);
174+
comm->comm_str = comm_strs__findnew(str);
119175
if (!comm->comm_str) {
120176
free(comm);
121177
return NULL;
@@ -128,7 +184,7 @@ int comm__override(struct comm *comm, const char *str, u64 timestamp, bool exec)
128184
{
129185
struct comm_str *new, *old = comm->comm_str;
130186

131-
new = comm_str__findnew(str, &comm_str_root);
187+
new = comm_strs__findnew(str);
132188
if (!new)
133189
return -ENOMEM;
134190

@@ -149,5 +205,5 @@ void comm__free(struct comm *comm)
149205

150206
const char *comm__str(const struct comm *comm)
151207
{
152-
return comm->comm_str->str;
208+
return comm_str__str(comm->comm_str);
153209
}

0 commit comments

Comments
 (0)