DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test
@ 2026-07-21 19:04 Hari Mishal
  2026-07-21 23:51 ` SJ Park
  2026-07-22  1:23 ` [PATCH v2] " Hari Mishal
  0 siblings, 2 replies; 5+ messages in thread
From: Hari Mishal @ 2026-07-21 19:04 UTC (permalink / raw)
  To: SJ Park, Shuah Khan
  Cc: Greg Kroah-Hartman, Hari Mishal, damon, linux-mm, linux-kselftest,
	linux-kernel

The obsolete_target test spawns three sh processes and uses their pids
as DAMON monitoring targets.  These processes are created without their
own stdin, so they inherit the test program's stdin, and they are never
waited on or told to exit.  As a result, they are left running (or
become zombies) as orphaned children after the test program exits.

Give each sh process its own dedicated stdin pipe instead of inheriting
the test program's, and after the targets are no longer needed,
communicate() with each process to close its stdin.  This sends EOF to
the shell, which makes it exit, and communicate() then reaps the
process.

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
 tools/testing/selftests/damon/sysfs.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index aa03a1187489..d3d50a5a1d56 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -328,11 +328,14 @@ def main():
     kdamonds.stop()
 
     # test obsolete_target.
-    proc1 = subprocess.Popen(['sh'], stdout=subprocess.PIPE,
+    proc1 = subprocess.Popen(['sh'], stdin=subprocess.PIPE,
+                             stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE)
-    proc2 = subprocess.Popen(['sh'], stdout=subprocess.PIPE,
+    proc2 = subprocess.Popen(['sh'], stdin=subprocess.PIPE,
+                             stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE)
-    proc3 = subprocess.Popen(['sh'], stdout=subprocess.PIPE,
+    proc3 = subprocess.Popen(['sh'], stdin=subprocess.PIPE,
+                             stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE)
     kdamonds = _damon_sysfs.Kdamonds(
             [_damon_sysfs.Kdamond(
@@ -356,5 +359,8 @@ def main():
     assert_ctxs_committed(kdamonds)
     kdamonds.stop()
 
+    for proc in (proc1, proc2, proc3):
+        proc.communicate()
+
 if __name__ == '__main__':
     main()
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test
  2026-07-21 19:04 [PATCH] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test Hari Mishal
@ 2026-07-21 23:51 ` SJ Park
  2026-07-22  1:23 ` [PATCH v2] " Hari Mishal
  1 sibling, 0 replies; 5+ messages in thread
From: SJ Park @ 2026-07-21 23:51 UTC (permalink / raw)
  To: Hari Mishal
  Cc: SJ Park, Shuah Khan, Greg Kroah-Hartman, damon, linux-mm,
	linux-kselftest, linux-kernel

Hello Hari,

On Tue, 21 Jul 2026 21:04:04 +0200 Hari Mishal <harimishal1@gmail.com> wrote:

> The obsolete_target test spawns three sh processes and uses their pids
> as DAMON monitoring targets.  These processes are created without their
> own stdin, so they inherit the test program's stdin, and they are never
> waited on or told to exit.  As a result, they are left running (or
> become zombies) as orphaned children after the test program exits.

Thank you for finding this problem and sharing this great patch!

> 
> Give each sh process its own dedicated stdin pipe instead of inheriting
> the test program's, and after the targets are no longer needed,
> communicate() with each process to close its stdin.  This sends EOF to
> the shell, which makes it exit, and communicate() then reaps the
> process.

Can't we use terminate() or kill() instead of communicate()?


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test
  2026-07-21 19:04 [PATCH] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test Hari Mishal
  2026-07-21 23:51 ` SJ Park
@ 2026-07-22  1:23 ` Hari Mishal
  2026-07-22  3:38   ` SJ Park
  1 sibling, 1 reply; 5+ messages in thread
From: Hari Mishal @ 2026-07-22  1:23 UTC (permalink / raw)
  To: SJ Park, Shuah Khan
  Cc: Greg Kroah-Hartman, damon, linux-mm, linux-kselftest,
	linux-kernel, Hari Mishal

The obsolete_target test spawns three sh processes and uses their pids
as DAMON monitoring targets.  These processes are never terminated or
waited on, so they are left running (or become zombies) as orphaned
children after the test program exits.

Terminate each process and communicate() with it after the targets are
no longer needed, so it exits and gets reaped instead of being leaked.

Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
v2: Terminate each sh process directly instead of giving it its own
    stdin pipe to close, dropping the stdin=PIPE changes and shrinking
    the diff.

 tools/testing/selftests/damon/sysfs.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
index aa03a1187489..bdb37eb1d26f 100755
--- a/tools/testing/selftests/damon/sysfs.py
+++ b/tools/testing/selftests/damon/sysfs.py
@@ -356,5 +356,9 @@ def main():
     assert_ctxs_committed(kdamonds)
     kdamonds.stop()
 
+    for proc in (proc1, proc2, proc3):
+        proc.terminate()
+        proc.communicate()
+
 if __name__ == '__main__':
     main()
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test
  2026-07-22  1:23 ` [PATCH v2] " Hari Mishal
@ 2026-07-22  3:38   ` SJ Park
  2026-07-22 13:48     ` SJ Park
  0 siblings, 1 reply; 5+ messages in thread
From: SJ Park @ 2026-07-22  3:38 UTC (permalink / raw)
  To: Hari Mishal
  Cc: SJ Park, Shuah Khan, Greg Kroah-Hartman, damon, linux-mm,
	linux-kselftest, linux-kernel

Hello Hari,


From the next time, please don't post a new version of a patch as a reply to
the previous version.  Post a new version as a new thread, with changelogs [1]
including links to previous revisions.

Also, give others sufficient time, say, one day, to add comments before posting
a new version.  If someone publicly asked questions to your patch, please
answer the questions and keep the discussion in public until the discussion is
clearly completed, before posting a new version.

On Wed, 22 Jul 2026 03:23:49 +0200 Hari Mishal <harimishal1@gmail.com> wrote:

> The obsolete_target test spawns three sh processes and uses their pids
> as DAMON monitoring targets.  These processes are never terminated or
> waited on, so they are left running (or become zombies) as orphaned
> children after the test program exits.
> 
> Terminate each process and communicate() with it after the targets are
> no longer needed, so it exits and gets reaped instead of being leaked.

Makes sense to me.  Thank you for this patch.

> 
> Signed-off-by: Hari Mishal <harimishal1@gmail.com>

Reviewed-by: SJ Park <sj@kernel.org>

> ---
> v2: Terminate each sh process directly instead of giving it its own
>     stdin pipe to close, dropping the stdin=PIPE changes and shrinking
>     the diff.

When you add changelog, please add links to the previous revisions.

Finally, please use mm-new [2] as the baseline of DAMON patches from the next
time.

This patch is applied to damon/next [2] tree.  If this patch is not added to
mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton)
to pick this.  So, no action from your side is needed for now.  If it seems I
also forgot doing that or you cannot wait for my action, please feel free to
directly ask that to Andrew.

[1] https://docs.kernel.org/process/submitting-patches.html#commentary
[2] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test
  2026-07-22  3:38   ` SJ Park
@ 2026-07-22 13:48     ` SJ Park
  0 siblings, 0 replies; 5+ messages in thread
From: SJ Park @ 2026-07-22 13:48 UTC (permalink / raw)
  To: SJ Park
  Cc: Hari Mishal, Shuah Khan, Greg Kroah-Hartman, damon, linux-mm,
	linux-kselftest, linux-kernel

On Tue, 21 Jul 2026 20:38:43 -0700 SJ Park <sj@kernel.org> wrote:
[...]
> This patch is applied to damon/next [2] tree.  If this patch is not added to
> mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton)
> to pick this.  So, no action from your side is needed for now.  If it seems I
> also forgot doing that or you cannot wait for my action, please feel free to
> directly ask that to Andrew.


We (mm community) now want to focus on making mm.git more stabilized and
therefore ready for the next merge window, rather than adding more changes that
are not really urgent.  I agree and want to help [1] that.  For the reason,
unless Andre pick this first, I will request adding this to mm.git only after
next -rc1 release.  Let me know if you think this is really urgent.

[1] https://lore.kernel.org/20260722133829.87607-1-sj@kernel.org


Thanks,
SJ

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-22 13:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-21 19:04 [PATCH] selftests/damon/sysfs.py: clean up sh processes used for obsolete_target test Hari Mishal
2026-07-21 23:51 ` SJ Park
2026-07-22  1:23 ` [PATCH v2] " Hari Mishal
2026-07-22  3:38   ` SJ Park
2026-07-22 13:48     ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox