Re: [PATCH v2] PM: QoS: Add a sysfs CPU latency request with a boot-time value

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] locking: data race on req->node.prio in cpu_latency_qos_update_request
--

commit d19f16780c138f4d33dfa7e439454d2244724233
Author: Roman Kagan <rkagan@xxxxxxxxx>

PM: QoS: Add a sysfs CPU latency request with a boot-time value

This commit adds a CPU latency limit requested by user space for all CPUs,
exposed via a sysfs attribute. It operates similarly to the per-CPU resume
latency request but acts as a single PM QoS request shared by the entire
user space.

Link: https://patch.msgid.link/20261008-b4-cpuidle-shallow-v1-1-c19e71127b14@xxxxxxxxx

> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 1944dbeb0d4c9..f381361939079 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c

[ ... ]

> +static ssize_t pm_qos_cpu_latency_us_show(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	s32 value = READ_ONCE(cpu_latency_qos_sysfs_req.node.prio);
> +
> +	if (value == 0)
> +		return sysfs_emit(buf, "n/a\n");
> +	if (value == PM_QOS_CPU_LATENCY_DEFAULT_VALUE)
> +		value = 0;
> +
> +	return sysfs_emit(buf, "%d\n", value);
> +}
> +
> +static ssize_t pm_qos_cpu_latency_us_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t n)
> +{
> +	s32 value;
> +	int ret;
> +
> +	ret = cpu_latency_qos_parse(buf, &value);
> +	if (ret)
> +		return ret;
> +
> +	cpu_latency_qos_update_request(&cpu_latency_qos_sysfs_req, value);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Can concurrent writes to this sysfs attribute trigger a data race?

This calls cpu_latency_qos_update_request() on the globally shared
cpu_latency_qos_sysfs_req object without external locking. Inside
cpu_latency_qos_update_request(), there is a fast-path check:

    if (new_value == req->node.prio)
        return;

This performs a plain lockless read of req->node.prio, which concurrently
races with modifications under pm_qos_lock via plist_node_init() in
pm_qos_update_target().

The pm_qos_cpu_latency_us_show() function correctly uses READ_ONCE() for
this field, but the plain read on the update fast-path could cause KCSAN
warnings or undefined behavior if multiple threads write to the sysfs
attribute simultaneously.

> +
> +	return n;
> +}
> +
> +static DEVICE_ATTR_RW(pm_qos_cpu_latency_us);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-b4-cpuidle-shallow-v2-1-adede7dd9077@xxxxxxxxx?part=1