Skip to content

Commit 1971bd6

Browse files
sjp38torvalds
sjp38
authored and
torvalds
committed
mm/damon: remove the target id concept
DAMON asks each monitoring target ('struct damon_target') to have one 'unsigned long' integer called 'id', which should be unique among the targets of same monitoring context. Meaning of it is, however, totally up to the monitoring primitives that registered to the monitoring context. For example, the virtual address spaces monitoring primitives treats the id as a 'struct pid' pointer. This makes the code flexible, but ugly, not well-documented, and type-unsafe[1]. Also, identification of each target can be done via its index. For the reason, this commit removes the concept and uses clear type definition. For now, only 'struct pid' pointer is used for the virtual address spaces monitoring. If DAMON is extended in future so that we need to put another identifier field in the struct, we will use a union for such primitives-dependent fields and document which primitives are using which type. [1] https://lore.kernel.org/linux-mm/20211013154535.4aaeaaf9d0182922e405dd1e@linux-foundation.org/ Link: https://lkml.kernel.org/r/20211230100723.2238-5-sj@kernel.org Signed-off-by: SeongJae Park <sj@kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 4364282 commit 1971bd6

8 files changed

Lines changed: 133 additions & 128 deletions

File tree

‎include/linux/damon.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,18 @@ struct damon_region {
6060

6161
/**
6262
* struct damon_target - Represents a monitoring target.
63-
* @id: Unique identifier for this target.
63+
* @pid: The PID of the virtual address space to monitor.
6464
* @nr_regions: Number of monitoring target regions of this target.
6565
* @regions_list: Head of the monitoring target regions of this target.
6666
* @list: List head for siblings.
6767
*
6868
* Each monitoring context could have multiple targets. For example, a context
6969
* for virtual memory address spaces could have multiple target processes. The
70-
* @id of each target should be unique among the targets of the context. For
71-
* example, in the virtual address monitoring context, it could be a pidfd or
72-
* an address of an mm_struct.
70+
* @pid should be set for appropriate address space monitoring primitives
71+
* including the virtual address spaces monitoring primitives.
7372
*/
7473
struct damon_target {
75-
unsigned long id;
74+
struct pid *pid;
7675
unsigned int nr_regions;
7776
struct list_head regions_list;
7877
struct list_head list;
@@ -475,7 +474,7 @@ struct damos *damon_new_scheme(
475474
void damon_add_scheme(struct damon_ctx *ctx, struct damos *s);
476475
void damon_destroy_scheme(struct damos *s);
477476

478-
struct damon_target *damon_new_target(unsigned long id);
477+
struct damon_target *damon_new_target(void);
479478
void damon_add_target(struct damon_ctx *ctx, struct damon_target *t);
480479
bool damon_targets_empty(struct damon_ctx *ctx);
481480
void damon_free_target(struct damon_target *t);

‎mm/damon/core-test.h‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ static void damon_test_regions(struct kunit *test)
2424
KUNIT_EXPECT_EQ(test, 2ul, r->ar.end);
2525
KUNIT_EXPECT_EQ(test, 0u, r->nr_accesses);
2626

27-
t = damon_new_target(42);
27+
t = damon_new_target();
2828
KUNIT_EXPECT_EQ(test, 0u, damon_nr_regions(t));
2929

3030
damon_add_region(r, t);
@@ -52,8 +52,7 @@ static void damon_test_target(struct kunit *test)
5252
struct damon_ctx *c = damon_new_ctx();
5353
struct damon_target *t;
5454

55-
t = damon_new_target(42);
56-
KUNIT_EXPECT_EQ(test, 42ul, t->id);
55+
t = damon_new_target();
5756
KUNIT_EXPECT_EQ(test, 0u, nr_damon_targets(c));
5857

5958
damon_add_target(c, t);
@@ -78,7 +77,6 @@ static void damon_test_target(struct kunit *test)
7877
static void damon_test_aggregate(struct kunit *test)
7978
{
8079
struct damon_ctx *ctx = damon_new_ctx();
81-
unsigned long target_ids[] = {1, 2, 3};
8280
unsigned long saddr[][3] = {{10, 20, 30}, {5, 42, 49}, {13, 33, 55} };
8381
unsigned long eaddr[][3] = {{15, 27, 40}, {31, 45, 55}, {23, 44, 66} };
8482
unsigned long accesses[][3] = {{42, 95, 84}, {10, 20, 30}, {0, 1, 2} };
@@ -87,7 +85,7 @@ static void damon_test_aggregate(struct kunit *test)
8785
int it, ir;
8886

8987
for (it = 0; it < 3; it++) {
90-
t = damon_new_target(target_ids[it]);
88+
t = damon_new_target();
9189
damon_add_target(ctx, t);
9290
}
9391

@@ -125,7 +123,7 @@ static void damon_test_split_at(struct kunit *test)
125123
struct damon_target *t;
126124
struct damon_region *r;
127125

128-
t = damon_new_target(42);
126+
t = damon_new_target();
129127
r = damon_new_region(0, 100);
130128
damon_add_region(r, t);
131129
damon_split_region_at(c, t, r, 25);
@@ -146,7 +144,7 @@ static void damon_test_merge_two(struct kunit *test)
146144
struct damon_region *r, *r2, *r3;
147145
int i;
148146

149-
t = damon_new_target(42);
147+
t = damon_new_target();
150148
r = damon_new_region(0, 100);
151149
r->nr_accesses = 10;
152150
damon_add_region(r, t);
@@ -194,7 +192,7 @@ static void damon_test_merge_regions_of(struct kunit *test)
194192
unsigned long eaddrs[] = {112, 130, 156, 170, 230};
195193
int i;
196194

197-
t = damon_new_target(42);
195+
t = damon_new_target();
198196
for (i = 0; i < ARRAY_SIZE(sa); i++) {
199197
r = damon_new_region(sa[i], ea[i]);
200198
r->nr_accesses = nrs[i];
@@ -218,14 +216,14 @@ static void damon_test_split_regions_of(struct kunit *test)
218216
struct damon_target *t;
219217
struct damon_region *r;
220218

221-
t = damon_new_target(42);
219+
t = damon_new_target();
222220
r = damon_new_region(0, 22);
223221
damon_add_region(r, t);
224222
damon_split_regions_of(c, t, 2);
225223
KUNIT_EXPECT_LE(test, damon_nr_regions(t), 2u);
226224
damon_free_target(t);
227225

228-
t = damon_new_target(42);
226+
t = damon_new_target();
229227
r = damon_new_region(0, 220);
230228
damon_add_region(r, t);
231229
damon_split_regions_of(c, t, 4);

‎mm/damon/core.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,15 @@ void damon_destroy_scheme(struct damos *s)
144144
*
145145
* Returns the pointer to the new struct if success, or NULL otherwise
146146
*/
147-
struct damon_target *damon_new_target(unsigned long id)
147+
struct damon_target *damon_new_target(void)
148148
{
149149
struct damon_target *t;
150150

151151
t = kmalloc(sizeof(*t), GFP_KERNEL);
152152
if (!t)
153153
return NULL;
154154

155-
t->id = id;
155+
t->pid = NULL;
156156
t->nr_regions = 0;
157157
INIT_LIST_HEAD(&t->regions_list);
158158

‎mm/damon/dbgfs-test.h‎

Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,97 +12,79 @@
1212

1313
#include <kunit/test.h>
1414

15-
static void damon_dbgfs_test_str_to_target_ids(struct kunit *test)
15+
static void damon_dbgfs_test_str_to_ints(struct kunit *test)
1616
{
1717
char *question;
18-
unsigned long *answers;
19-
unsigned long expected[] = {12, 35, 46};
18+
int *answers;
19+
int expected[] = {12, 35, 46};
2020
ssize_t nr_integers = 0, i;
2121

2222
question = "123";
23-
answers = str_to_target_ids(question, strlen(question),
24-
&nr_integers);
23+
answers = str_to_ints(question, strlen(question), &nr_integers);
2524
KUNIT_EXPECT_EQ(test, (ssize_t)1, nr_integers);
26-
KUNIT_EXPECT_EQ(test, 123ul, answers[0]);
25+
KUNIT_EXPECT_EQ(test, 123, answers[0]);
2726
kfree(answers);
2827

2928
question = "123abc";
30-
answers = str_to_target_ids(question, strlen(question),
31-
&nr_integers);
29+
answers = str_to_ints(question, strlen(question), &nr_integers);
3230
KUNIT_EXPECT_EQ(test, (ssize_t)1, nr_integers);
33-
KUNIT_EXPECT_EQ(test, 123ul, answers[0]);
31+
KUNIT_EXPECT_EQ(test, 123, answers[0]);
3432
kfree(answers);
3533

3634
question = "a123";
37-
answers = str_to_target_ids(question, strlen(question),
38-
&nr_integers);
35+
answers = str_to_ints(question, strlen(question), &nr_integers);
3936
KUNIT_EXPECT_EQ(test, (ssize_t)0, nr_integers);
4037
kfree(answers);
4138

4239
question = "12 35";
43-
answers = str_to_target_ids(question, strlen(question),
44-
&nr_integers);
40+
answers = str_to_ints(question, strlen(question), &nr_integers);
4541
KUNIT_EXPECT_EQ(test, (ssize_t)2, nr_integers);
4642
for (i = 0; i < nr_integers; i++)
4743
KUNIT_EXPECT_EQ(test, expected[i], answers[i]);
4844
kfree(answers);
4945

5046
question = "12 35 46";
51-
answers = str_to_target_ids(question, strlen(question),
52-
&nr_integers);
47+
answers = str_to_ints(question, strlen(question), &nr_integers);
5348
KUNIT_EXPECT_EQ(test, (ssize_t)3, nr_integers);
5449
for (i = 0; i < nr_integers; i++)
5550
KUNIT_EXPECT_EQ(test, expected[i], answers[i]);
5651
kfree(answers);
5752

5853
question = "12 35 abc 46";
59-
answers = str_to_target_ids(question, strlen(question),
60-
&nr_integers);
54+
answers = str_to_ints(question, strlen(question), &nr_integers);
6155
KUNIT_EXPECT_EQ(test, (ssize_t)2, nr_integers);
6256
for (i = 0; i < 2; i++)
6357
KUNIT_EXPECT_EQ(test, expected[i], answers[i]);
6458
kfree(answers);
6559

6660
question = "";
67-
answers = str_to_target_ids(question, strlen(question),
68-
&nr_integers);
61+
answers = str_to_ints(question, strlen(question), &nr_integers);
6962
KUNIT_EXPECT_EQ(test, (ssize_t)0, nr_integers);
7063
kfree(answers);
7164

7265
question = "\n";
73-
answers = str_to_target_ids(question, strlen(question),
74-
&nr_integers);
66+
answers = str_to_ints(question, strlen(question), &nr_integers);
7567
KUNIT_EXPECT_EQ(test, (ssize_t)0, nr_integers);
7668
kfree(answers);
7769
}
7870

7971
static void damon_dbgfs_test_set_targets(struct kunit *test)
8072
{
8173
struct damon_ctx *ctx = dbgfs_new_ctx();
82-
unsigned long ids[] = {1, 2, 3};
8374
char buf[64];
8475

85-
/* Make DAMON consider target id as plain number */
86-
ctx->primitive.target_valid = NULL;
87-
ctx->primitive.cleanup = NULL;
76+
/* Make DAMON consider target has no pid */
77+
ctx->primitive = (struct damon_primitive){};
8878

89-
dbgfs_set_targets(ctx, ids, 3);
90-
sprint_target_ids(ctx, buf, 64);
91-
KUNIT_EXPECT_STREQ(test, (char *)buf, "1 2 3\n");
92-
93-
dbgfs_set_targets(ctx, NULL, 0);
79+
dbgfs_set_targets(ctx, 0, NULL);
9480
sprint_target_ids(ctx, buf, 64);
9581
KUNIT_EXPECT_STREQ(test, (char *)buf, "\n");
9682

97-
dbgfs_set_targets(ctx, (unsigned long []){1, 2}, 2);
98-
sprint_target_ids(ctx, buf, 64);
99-
KUNIT_EXPECT_STREQ(test, (char *)buf, "1 2\n");
100-
101-
dbgfs_set_targets(ctx, (unsigned long []){2}, 1);
83+
dbgfs_set_targets(ctx, 1, NULL);
10284
sprint_target_ids(ctx, buf, 64);
103-
KUNIT_EXPECT_STREQ(test, (char *)buf, "2\n");
85+
KUNIT_EXPECT_STREQ(test, (char *)buf, "42\n");
10486

105-
dbgfs_set_targets(ctx, NULL, 0);
87+
dbgfs_set_targets(ctx, 0, NULL);
10688
sprint_target_ids(ctx, buf, 64);
10789
KUNIT_EXPECT_STREQ(test, (char *)buf, "\n");
10890

@@ -112,7 +94,6 @@ static void damon_dbgfs_test_set_targets(struct kunit *test)
11294
static void damon_dbgfs_test_set_init_regions(struct kunit *test)
11395
{
11496
struct damon_ctx *ctx = damon_new_ctx();
115-
unsigned long ids[] = {1, 2, 3};
11697
/* Each line represents one region in ``<target idx> <start> <end>`` */
11798
char * const valid_inputs[] = {"1 10 20\n 1 20 30\n1 35 45",
11899
"1 10 20\n",
@@ -130,7 +111,7 @@ static void damon_dbgfs_test_set_init_regions(struct kunit *test)
130111
int i, rc;
131112
char buf[256];
132113

133-
dbgfs_set_targets(ctx, ids, 3);
114+
dbgfs_set_targets(ctx, 3, NULL);
134115

135116
/* Put valid inputs and check the results */
136117
for (i = 0; i < ARRAY_SIZE(valid_inputs); i++) {
@@ -158,12 +139,12 @@ static void damon_dbgfs_test_set_init_regions(struct kunit *test)
158139
KUNIT_EXPECT_STREQ(test, (char *)buf, "");
159140
}
160141

161-
dbgfs_set_targets(ctx, NULL, 0);
142+
dbgfs_set_targets(ctx, 0, NULL);
162143
damon_destroy_ctx(ctx);
163144
}
164145

165146
static struct kunit_case damon_test_cases[] = {
166-
KUNIT_CASE(damon_dbgfs_test_str_to_target_ids),
147+
KUNIT_CASE(damon_dbgfs_test_str_to_ints),
167148
KUNIT_CASE(damon_dbgfs_test_set_targets),
168149
KUNIT_CASE(damon_dbgfs_test_set_init_regions),
169150
{},

0 commit comments

Comments
 (0)