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 v5 07/10] selftests/damon/_damon_sysfs: support pause file staging
Date: Mon, 23 Mar 2026 18:28:24 -0700	[thread overview]
Message-ID: <20260324012825.46899-1-sj@kernel.org> (raw)
In-Reply-To: <20260323231538.84452-8-sj@kernel.org>

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260323231538.84452-8-sj@kernel.org
# start of sashiko.dev inline review
commit d9ca6c4a33fc4d613982730edda6f4b67ae30a67
Author: SeongJae Park <sj@kernel.org>
selftests/damon/_damon_sysfs: support pause file staging

This commit adds support for the newly added pause file to the DAMON
test-purpose sysfs interface control Python module.

> diff --git a/tools/testing/selftests/damon/_damon_sysfs.py b/tools/testing/selftests/damon/_damon_sysfs.py
> --- a/tools/testing/selftests/damon/_damon_sysfs.py
> +++ b/tools/testing/selftests/damon/_damon_sysfs.py
[ ... ]
>  class DamonCtx:
>      targets = None
>      schemes = None
>      kdamond = None
> +    pause = None
>      idx = None
>  
>      def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), targets=[],
> -            schemes=[]):
> +            schemes=[], pause=False):

This isn't a bug introduced by this patch, but since the function signature
is being modified here, is it a good time to address the mutable default
arguments?

In Python, default arguments are evaluated exactly once at function
definition time. This means any instances of DamonCtx created without
explicitly providing targets, schemes, or monitoring_attrs will share
the same list structures and DamonAttrs instance in memory.

For example, self.monitoring_attrs.context = self overwrites the context
attribute of the shared DamonAttrs object for all default contexts.

Would it be better to change these defaults to None and instantiate them
within the method body to avoid unexpected state sharing across instances?

>          self.ops = ops
>          self.monitoring_attrs = monitoring_attrs
>          self.monitoring_attrs.context = self

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

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


  reply	other threads:[~2026-03-24  1:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-23 23:15 [RFC PATCH v5 00/10] mm/damon: let DAMON be paused and resumed SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 01/10] mm/damon/core: introduce damon_ctx->paused SeongJae Park
2026-03-24  1:28   ` (sashiko review) " SeongJae Park
2026-03-24  4:07     ` SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 02/10] mm/damon/sysfs: add pause file under context dir SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 03/10] Docs/mm/damon/design: update for context pause/resume feature SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 04/10] Docs/admin-guide/mm/damon/usage: update for pause file SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 05/10] Docs/ABI/damon: update for pause sysfs file SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 06/10] mm/damon/tests/core-kunit: test pause commitment SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging SeongJae Park
2026-03-24  1:28   ` SeongJae Park [this message]
2026-03-24  4:08     ` (sashiko review) " SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 08/10] selftests/damon/drgn_dump_damon_status: dump pause SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 09/10] selftests/damon/sysfs.py: check pause on assert_ctx_committed() SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 10/10] selftests/damon/sysfs.py: pause DAMON before dumping status SeongJae Park
2026-03-24  1:27 ` (sashiko status) [RFC PATCH v5 00/10] mm/damon: let DAMON be paused and resumed 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=20260324012825.46899-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