[PATCH] tracing/hwlat: Bind the sampler kthread instead of setting its affinity

[PATCH] tracing/hwlat: Bind the sampler kthread instead of setting its affinity

[Wenwu Hou]

start_single_kthread() sets the affinity of the kthread it just created
before waking it up, but kthread_affine_node() overrides that mask on the
first wake-up. See commit 60ba9c38b931 ("kthread: Comment on the purpose
and placement of kthread_affine_node() call").

In round-robin mode, move_to_next_cpu() compares save_cpumask against
current->cpus_ptr, sees the replaced mask, and switches the tracer to
MODE_NONE on its very first iteration:

  hwlat_detector: cpumask changed while in round-robin mode,
  switching to mode none

So the tracer no longer migrates across tracing_cpumask. More importantly,
tracing_cpumask is not honoured at all, and with isolcpus=/nohz_full= the
thread cannot run on the isolated CPUs it is meant to sample.

Use kthread_bind_mask() instead. hwlat requires exactly the CPUs in
tracing_cpumask, isolated ones included, so a hard binding is the correct
semantics; the PF_NO_SETAFFINITY it sets is precisely what tells
kthread_affine_node() to leave the thread alone. The in-thread
set_cpus_allowed_ptr() in move_to_next_cpu() keeps working, since
PF_NO_SETAFFINITY only gates the user-facing SCA_CHECK path.

The mask is now computed and validated before kthread_create(), which also
avoids creating a thread when tracing_cpumask has no online CPU in common:
cpumask_first() returned nr_cpu_ids there and cpumask_set_cpu() then wrote
past the end of the mask.

Tested on a 4-vCPU KVM guest, bit 0 of the mask is CPU 0:

  mode        mask    before                    after
  none        0b0101  Cpus_allowed_list: 0-3    0,2
  round-robin 0b1111  mode -> none, no migrate  migrates 1 2 3 0
  round-robin 0b0101  mask 0-3, no migrate      migrates 0 2
  per-cpu     0b0110  2 threads, cpus 1,2      2 threads, cpus 1,2

Fixes: d1a89197589c4 ("kthread: Default affine kthread to its preferred NUMA node")
Cc: stable@vger.kernel.org
Signed-off-by: Wenwu Hou <hwenwur@gmail.com>
---
 kernel/trace/trace_hwlat.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 3fe274b84f1c2..65e184d800ff1 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -423,15 +423,12 @@ static int start_single_kthread(struct trace_array *tr)
 	if (kdata->kthread)
 		goto out_put_cpus;
 
-	kthread = kthread_create(kthread_fn, NULL, "hwlatd");
-	if (IS_ERR(kthread)) {
-		pr_err(BANNER "could not start sampling thread\n");
-		cpus_read_unlock();
-		return -ENOMEM;
-	}
-
 	/* Just pick the first CPU on first iteration */
 	cpumask_and(current_mask, cpu_online_mask, tr->tracing_cpumask);
+	if (cpumask_empty(current_mask)) {
+		cpus_read_unlock();
+		return -EINVAL;
+	}
 
 	if (hwlat_data.thread_mode == MODE_ROUND_ROBIN) {
 		next_cpu = cpumask_first(current_mask);
@@ -440,7 +437,14 @@ static int start_single_kthread(struct trace_array *tr)
 
 	}
 
-	set_cpus_allowed_ptr(kthread, current_mask);
+	kthread = kthread_create(kthread_fn, NULL, "hwlatd");
+	if (IS_ERR(kthread)) {
+		pr_err(BANNER "could not start sampling thread\n");
+		cpus_read_unlock();
+		return -ENOMEM;
+	}
+
+	kthread_bind_mask(kthread, current_mask);
 
 	kdata->kthread = kthread;
 	wake_up_process(kthread);
-- 
2.43.0

Re: [PATCH] tracing/hwlat: Bind the sampler kthread instead of setting its affinity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] tracing/hwlat: start_single_kthread() leaves
  PF_NO_SETAFFINITY set on hwlatd
--

commit 649d99e25afc0a134fd743f085f8409aa8adb366
Author: Wenwu Hou <hwenwur@gmail.com>

tracing/hwlat: Bind the sampler kthread instead of setting its affinity

This commit replaces set_cpus_allowed_ptr() with kthread_bind_mask() in
start_single_kthread() and validates that tracing_cpumask overlaps with
cpu_online_mask before creating the hwlatd kthread.

> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
> index 3fe274b84f1c2..65e184d800ff1 100644
> --- a/kernel/trace/trace_hwlat.c
> +++ b/kernel/trace/trace_hwlat.c
> @@ -440,7 +437,14 @@ static int start_single_kthread(struct trace_array *tr)
>  
>  	}
>  
> -	set_cpus_allowed_ptr(kthread, current_mask);
> +	kthread = kthread_create(kthread_fn, NULL, "hwlatd");
> +	if (IS_ERR(kthread)) {
> +		pr_err(BANNER "could not start sampling thread\n");
> +		cpus_read_unlock();
> +		return -ENOMEM;
> +	}
> +
> +	kthread_bind_mask(kthread, current_mask);

[Severity: Medium]
Does leaving PF_NO_SETAFFINITY set on hwlatd prevent userspace from
changing its CPU affinity or moving it into a cgroup?

kthread_bind_mask() sets PF_NO_SETAFFINITY, which kthread() only checks on
initial wake-up before calling kthread_fn().  Because kthread_fn() never
clears the flag, sched_setaffinity() and cgroup_attach_permissions()
reject hwlatd with -EINVAL in both round-robin and none modes.  That also
makes the user-affinity override check in move_to_next_cpu() unreachable
from sched_setaffinity():

kernel/trace/trace_hwlat.c:move_to_next_cpu() {
	/*
	 * If for some reason the user modifies the CPU affinity
	 * of this thread, then stop migrating for the duration
	 * of the current test.
	 */
	if (!cpumask_equal(current_mask, current->cpus_ptr))
		goto change_mode;
    ...
}

Clearing PF_NO_SETAFFINITY in kthread_fn() once the thread has started, as
done in 4998e7fda149 ("tracing/osnoise: Switch from PF_NO_SETAFFINITY to
migrate_disable"), would avoid this regression while still skipping
kthread_affine_node() on the initial wake-up.

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