Re: [PATCH v7 17/20] tracing/remotes: Add poll_ms tracefs file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] tracing: delayed work rescheduled with old poll_ms value due to race
--

commit 8789c4fba42732f9e8b7a1922700dca27018009e
Author: Vincent Donnefort <vdonnefort@google.com>

tracing/remotes: Add poll_ms tracefs file

This commit adds a tracefs file to configure the trace remote polling period.
It keeps the default value to 100ms.

> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 0a360690b174e..b2547eb117f85 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
[ ... ]
> @@ -1138,6 +1139,40 @@ static int dump_on_panic_show(struct seq_file *s, void *unused)
>  }
>  DEFINE_TRACE_REMOTE_ATTRIBUTE(dump_on_panic);
>  
> +static ssize_t poll_ms_write(struct file *filp, const char __user *ubuf, size_t cnt, loff_t *ppos)
> +{
> +	struct seq_file *seq = filp->private_data;
> +	struct trace_remote *remote = seq->private;
> +	unsigned int val;
> +	int ret;
> +
> +	ret = kstrtouint_from_user(ubuf, cnt, 10, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (!val)
> +		return -EINVAL;
> +
> +	guard(mutex)(&remote->lock);
> +
> +	if (val < remote->poll_ms && remote->poll_cnt)
> +		mod_delayed_work(system_percpu_wq, &remote->poll_work, msecs_to_jiffies(val));
> +
> +	WRITE_ONCE(remote->poll_ms, val);

[Severity: Low]
Does this code introduce a race condition where the delayed work reschedules 
with the old polling interval?

If the delayed worker is expedited via mod_delayed_work(), and it executes 
before WRITE_ONCE(remote->poll_ms, val) completes (e.g., if this thread is 
preempted or the worker runs concurrently on another CPU), it will read the 
old remote->poll_ms value.

The worker trace_remote_do_poll() schedules itself like this:

    schedule_delayed_work(&remote->poll_work,
                          yield ? 0 : msecs_to_jiffies(READ_ONCE(remote->poll_ms)));

Because it would see the old value, it could reschedule itself with the old 
delay, effectively ignoring the user's requested speedup for one entire cycle.

Should the new state be written before the worker is expedited?

> +
> +	return cnt;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924145754.1285146-1-vdonnefort@google.com?part=17