public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: SeongJae Park <sj@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>,
	damon@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-mm@kvack.org
Subject: Re: (sashiko review) [RFC PATCH v4 10/10] selftets/damon/sysfs.py: pause DAMON before dumping status
Date: Sun, 22 Mar 2026 10:15:33 -0700	[thread overview]
Message-ID: <20260322171534.83470-1-sj@kernel.org> (raw)
In-Reply-To: <20260322155728.81434-11-sj@kernel.org>

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260322155728.81434-11-sj@kernel.org
# start of sashiko.dev inline review
commit 67adf8e7b8416252faf583e176f0e8424fd4fb95
Author: SeongJae Park <sj@kernel.org>

selftets/damon/sysfs.py: pause DAMON before dumping status

The sysfs.py test commits DAMON parameters, dumps the internal DAMON
state, and verifies the parameters. This commit pauses DAMON execution
during the state dumping to avoid races that can result in false test
results.

> selftets/damon/sysfs.py: pause DAMON before dumping status

This isn't a bug, but there's a typo (selftets) in the subject line.

> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> --- a/tools/testing/selftests/damon/sysfs.py
> +++ b/tools/testing/selftests/damon/sysfs.py
[ ... ]
> @@ -193,18 +193,55 @@ def assert_ctx_committed(ctx, dump):
>  def assert_ctxs_committed(kdamonds):
> +    ctxs_paused_for_dump = []
> +    kdamonds_paused_for_dump = []
> +    # pause for safe state dumping
> +    for kd in kdamonds.kdamonds:
[ ... ]
> +    # resume contexts paused for safe state dumping
> +    for ctx in ctxs_paused_for_dump:
> +        ctx.pause = False
> +    if kd in kdamonds_paused_for_dump:
> +        err = kd.commit()

Is this if statement intended to be a for loop?

Since kd is just retaining the value from the last iteration of the earlier
loop, using an if statement here means that if there are multiple kdamonds,
only the last one might have its state committed. 

Could this leave other kdamonds un-resumed in the kernel?

> +        if err is not None:
> +            print('resume fail (%s)' % err)
> +            kdamonds.stop()
> +            exit(1)
[ ... ]


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260322155728.81434-11-sj@kernel.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260322155728.81434-11-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)


  reply	other threads:[~2026-03-22 17:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-22 15:57 [RFC PATCH v4 00/10] mm/damon: let DAMON be paused and resumed SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 01/10] mm/damon/core: introduce damon_ctx->paused SeongJae Park
2026-03-22 17:06   ` (sashiko review) " SeongJae Park
2026-03-22 17:40     ` SeongJae Park
2026-03-22 20:51       ` SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 02/10] mm/damon/sysfs: add pause file under context dir SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 03/10] Docs/mm/damon/design: update for context pause/resume feature SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 04/10] Docs/admin-guide/mm/damon/usage: update for pause file SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 05/10] Docs/ABI/damon: update for pause sysfs file SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 06/10] mm/damon/tests/core-kunit: test pause commitment SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 07/10] selftests/damon/_damon_sysfs: support pause file staging SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 08/10] selftests/damon/drgn_dump_damon_status: dump pause SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 09/10] selftests/damon/sysfs.py: check pause on assert_ctx_committed() SeongJae Park
2026-03-22 15:57 ` [RFC PATCH v4 10/10] selftets/damon/sysfs.py: pause DAMON before dumping status SeongJae Park
2026-03-22 17:15   ` SeongJae Park [this message]
2026-03-22 17:47     ` (sashiko review) " SeongJae Park
2026-03-22 17:05 ` (sashiko status) [RFC PATCH v4 00/10] mm/damon: let DAMON be paused and resumed SeongJae Park
2026-03-22 17:11   ` SeongJae Park

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=20260322171534.83470-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shuah@kernel.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