Skip to content

Commit 144760f

Browse files
sjp38torvalds
authored andcommitted
mm/damon/dbgfs/init_regions: use target index instead of target id
Patch series "Remove the type-unclear 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 patchset removes the concept and uses clear type definition. [1] https://lore.kernel.org/linux-mm/20211013154535.4aaeaaf9d0182922e405dd1e@linux-foundation.org/ This patch (of 4): Target id is a 'unsigned long' data, which can be interpreted differently by each monitoring primitives. For example, it means 'struct pid *' for the virtual address spaces monitoring, while it means nothing but an integer to be displayed to debugfs interface users for the physical address space monitoring. It's flexible but makes code ugly and type-unsafe[1]. To be prepared for eventual removal of the concept, this commit removes a use case of the concept in 'init_regions' debugfs file handling. In detail, this commit replaces use of the id with the index of each target in the context's targets list. [1] https://lore.kernel.org/linux-mm/20211013154535.4aaeaaf9d0182922e405dd1e@linux-foundation.org/ Link: https://lkml.kernel.org/r/20211230100723.2238-1-sj@kernel.org Link: https://lkml.kernel.org/r/20211230100723.2238-2-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 d0977ef commit 144760f

2 files changed

Lines changed: 22 additions & 23 deletions

File tree

mm/damon/dbgfs-test.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,19 @@ static void damon_dbgfs_test_set_init_regions(struct kunit *test)
113113
{
114114
struct damon_ctx *ctx = damon_new_ctx();
115115
unsigned long ids[] = {1, 2, 3};
116-
/* Each line represents one region in ``<target id> <start> <end>`` */
117-
char * const valid_inputs[] = {"2 10 20\n 2 20 30\n2 35 45",
118-
"2 10 20\n",
119-
"2 10 20\n1 39 59\n1 70 134\n 2 20 25\n",
116+
/* Each line represents one region in ``<target idx> <start> <end>`` */
117+
char * const valid_inputs[] = {"1 10 20\n 1 20 30\n1 35 45",
118+
"1 10 20\n",
119+
"1 10 20\n0 39 59\n0 70 134\n 1 20 25\n",
120120
""};
121121
/* Reading the file again will show sorted, clean output */
122-
char * const valid_expects[] = {"2 10 20\n2 20 30\n2 35 45\n",
123-
"2 10 20\n",
124-
"1 39 59\n1 70 134\n2 10 20\n2 20 25\n",
122+
char * const valid_expects[] = {"1 10 20\n1 20 30\n1 35 45\n",
123+
"1 10 20\n",
124+
"0 39 59\n0 70 134\n1 10 20\n1 20 25\n",
125125
""};
126-
char * const invalid_inputs[] = {"4 10 20\n", /* target not exists */
127-
"2 10 20\n 2 14 26\n", /* regions overlap */
128-
"1 10 20\n2 30 40\n 1 5 8"}; /* not sorted by address */
126+
char * const invalid_inputs[] = {"3 10 20\n", /* target not exists */
127+
"1 10 20\n 1 14 26\n", /* regions overlap */
128+
"0 10 20\n1 30 40\n 0 5 8"}; /* not sorted by address */
129129
char *input, *expect;
130130
int i, rc;
131131
char buf[256];

mm/damon/dbgfs.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -440,18 +440,20 @@ static ssize_t sprint_init_regions(struct damon_ctx *c, char *buf, ssize_t len)
440440
{
441441
struct damon_target *t;
442442
struct damon_region *r;
443+
int target_idx = 0;
443444
int written = 0;
444445
int rc;
445446

446447
damon_for_each_target(t, c) {
447448
damon_for_each_region(r, t) {
448449
rc = scnprintf(&buf[written], len - written,
449-
"%lu %lu %lu\n",
450-
t->id, r->ar.start, r->ar.end);
450+
"%d %lu %lu\n",
451+
target_idx, r->ar.start, r->ar.end);
451452
if (!rc)
452453
return -ENOMEM;
453454
written += rc;
454455
}
456+
target_idx++;
455457
}
456458
return written;
457459
}
@@ -485,22 +487,19 @@ static ssize_t dbgfs_init_regions_read(struct file *file, char __user *buf,
485487
return len;
486488
}
487489

488-
static int add_init_region(struct damon_ctx *c,
489-
unsigned long target_id, struct damon_addr_range *ar)
490+
static int add_init_region(struct damon_ctx *c, int target_idx,
491+
struct damon_addr_range *ar)
490492
{
491493
struct damon_target *t;
492494
struct damon_region *r, *prev;
493-
unsigned long id;
495+
unsigned long idx = 0;
494496
int rc = -EINVAL;
495497

496498
if (ar->start >= ar->end)
497499
return -EINVAL;
498500

499501
damon_for_each_target(t, c) {
500-
id = t->id;
501-
if (targetid_is_pid(c))
502-
id = (unsigned long)pid_vnr((struct pid *)id);
503-
if (id == target_id) {
502+
if (idx++ == target_idx) {
504503
r = damon_new_region(ar->start, ar->end);
505504
if (!r)
506505
return -ENOMEM;
@@ -523,7 +522,7 @@ static int set_init_regions(struct damon_ctx *c, const char *str, ssize_t len)
523522
struct damon_target *t;
524523
struct damon_region *r, *next;
525524
int pos = 0, parsed, ret;
526-
unsigned long target_id;
525+
int target_idx;
527526
struct damon_addr_range ar;
528527
int err;
529528

@@ -533,11 +532,11 @@ static int set_init_regions(struct damon_ctx *c, const char *str, ssize_t len)
533532
}
534533

535534
while (pos < len) {
536-
ret = sscanf(&str[pos], "%lu %lu %lu%n",
537-
&target_id, &ar.start, &ar.end, &parsed);
535+
ret = sscanf(&str[pos], "%d %lu %lu%n",
536+
&target_idx, &ar.start, &ar.end, &parsed);
538537
if (ret != 3)
539538
break;
540-
err = add_init_region(c, target_id, &ar);
539+
err = add_init_region(c, target_idx, &ar);
541540
if (err)
542541
goto fail;
543542
pos += parsed;

0 commit comments

Comments
 (0)