From: SeongJae Park <sj@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: SeongJae Park <sj@kernel.org>,
damon@lists.linux.dev, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: [PATCH mm-hotfixes-unstable] mm/damon/sysfs: fix unnecessary monitoring results removal when online-commit inputs
Date: Sat, 28 Oct 2023 21:33:53 +0000 [thread overview]
Message-ID: <20231028213353.45397-1-sj@kernel.org> (raw)
Commit db27869df6ed ("mm/damon/sysfs: remove requested targets when
online-commit inputs") of mm-hotfixes-unstable tree[1] makes all targets
to removed and then added again based on the user input, for
online-commit inputs. The commit says it is inefficient but ok as
below:
This could cause unnecessary memory dealloc and realloc operations,
but this is not a hot code path. Also, note that damon_target is
stateless, and hence no data is lost
But that's not true. 'struct target' is containing the monitoring
results ('->regions_list'). As a result, the patch makes all previous
monitoring results to be removed for every online-commit inputs. Fix it
by removing targets only when really changed or removal is requested.
[1] https://lore.kernel.org/damon/20231022210735.46409-2-sj@kernel.org/
Fixes: db27869df6ed ("mm/damon/sysfs: remove requested targets when online-commit inputs") # mm-hotfixes-unstable
Signed-off-by: SeongJae Park <sj@kernel.org>
---
Andrew, if you squash this into the original patch, please update the last
paragraph as below.
Fix it by doing in place updates and removals of existing targets as user
requested. Note that monitoring results (->regions_list) of targets that no
change has requested are maintained.
mm/damon/sysfs.c | 44 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index d13e353bee52..1a231bde18f9 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1150,23 +1150,57 @@ static int damon_sysfs_add_target(struct damon_sysfs_target *sys_target,
return err;
}
+static int damon_sysfs_update_target(struct damon_target *target,
+ struct damon_ctx *ctx,
+ struct damon_sysfs_target *sys_target)
+{
+ struct pid *pid;
+ struct damon_region *r, *next;
+
+ if (!damon_target_has_pid(ctx))
+ return 0;
+
+ pid = find_get_pid(sys_target->pid);
+ if (!pid)
+ return -EINVAL;
+
+ /* no change to the target */
+ if (pid == target->pid) {
+ put_pid(pid);
+ return 0;
+ }
+
+ /* remove old monitoring results and update the target's pid */
+ damon_for_each_region_safe(r, next, target)
+ damon_destroy_region(r, target);
+ put_pid(target->pid);
+ target->pid = pid;
+ return 0;
+}
+
static int damon_sysfs_set_targets(struct damon_ctx *ctx,
struct damon_sysfs_targets *sysfs_targets)
{
struct damon_target *t, *next;
- int i, err;
+ int i = 0, err;
/* Multiple physical address space monitoring targets makes no sense */
if (ctx->ops.id == DAMON_OPS_PADDR && sysfs_targets->nr > 1)
return -EINVAL;
damon_for_each_target_safe(t, next, ctx) {
- if (damon_target_has_pid(ctx))
- put_pid(t->pid);
- damon_destroy_target(t);
+ if (i < sysfs_targets->nr) {
+ damon_sysfs_update_target(t, ctx,
+ sysfs_targets->targets_arr[i]);
+ } else {
+ if (damon_target_has_pid(ctx))
+ put_pid(t->pid);
+ damon_destroy_target(t);
+ }
+ i++;
}
- for (i = 0; i < sysfs_targets->nr; i++) {
+ for (; i < sysfs_targets->nr; i++) {
struct damon_sysfs_target *st = sysfs_targets->targets_arr[i];
err = damon_sysfs_add_target(st, ctx);
--
2.34.1
reply other threads:[~2023-10-28 21:33 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231028213353.45397-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).