Re: [PATCH] selftests/damon: add regression test for damos_walk() vs kdamond exit race

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The script hardcodes the sysfs state path and suppresses exceptions with a bare `except` block, yielding a false pass if path resolution fails.
- [Medium] The script's custom timeout logic is incapable of detecting the kernel deadlock because it blocks synchronously on the system call.
- [Low] The new test script `sysfs_damos_walk_kdamond_exit_race.py` lacks execute permissions.
--

commit c1a389c100306b9715b021c992e2707ecc7ffc39
Author: Sailesh Nandanavanam <saileshnandanavanam@gmail.com>

selftests/damon: add regression test for damos_walk() vs kdamond exit race
    
This commit adds a regression test to verify that damos_walk() does not
deadlock when racing with the kdamond_fn() exit path. It starts kdamond
monitoring a short-lived process, waits for termination, and then rapidly
calls update_schemes_tried_regions to hit the race window.

> diff --git a/tools/testing/selftests/damon/sysfs_damos_walk_kdamond_exit_race.py b/tools/testing/selftests/damon/sysfs_damos_walk_kdamond_exit_race.py
> new file mode 100644
> index 0000000000000..9b9d66a23f512
> --- /dev/null
> +++ b/tools/testing/selftests/damon/sysfs_damos_walk_kdamond_exit_race.py

[Severity: Low]
Should this file be created with executable permissions (100755)? Since the
script is added to TEST_PROGS in the Makefile, the kselftest framework expects
it to be directly executable. Without the execute bit, it can cause warnings
during test execution.

> @@ -0,0 +1,47 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0

[ ... ]

> +    proc.wait()
> +    completed = False
> +    timeout = time.time() + 5
> +    while time.time() < timeout:
> +        err = kdamonds.kdamonds[0].update_schemes_tried_regions()

[Severity: Medium]
If the kernel deadlock bug occurs, update_schemes_tried_regions() will
trigger an uninterruptible wait in the kernel. Doesn't this mean the Python
script will block indefinitely on the system call in the D state?

Since the script is single-threaded, the while loop condition would never be
re-evaluated, making the internal timeout and failure message unreachable.
Instead, the test would just hang until the global kselftest timeout kills it.

> +        if err is not None:
> +            completed = True
> +            break
> +        try:
> +            with open('/sys/kernel/mm/damon/admin/kdamonds/0/state') as f:
> +                state = f.read().strip()
> +            if state == 'off':
> +                completed = True
> +                break
> +        except:
> +            completed = True
> +            break

[Severity: Medium]
Is it safe to hardcode the sysfs state path and use a bare except block here?

If sysfs is mounted at a non-standard location or another file error occurs,
open() will raise an exception. The bare except block will catch this,
incorrectly set completed to True, and exit the loop immediately. This results
in a false pass on the very first iteration, completely missing the race
window it is intended to test. 

Could the script dynamically resolve the path using the kdamonds object
instead of suppressing all exceptions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524091812.35283-1-saileshnandanavanam@gmail.com?part=1