Messages in this thread | | | Subject | Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE | From | Daniel Lezcano <> | Date | Fri, 23 Nov 2018 17:54:22 +0100 |
| |
On 23/11/2018 17:20, Sudeep Holla wrote: > On Fri, Nov 23, 2018 at 05:04:18PM +0100, Daniel Lezcano wrote: >> On 23/11/2018 14:58, Sudeep Holla wrote: >>> On Mon, Oct 29, 2018 at 05:23:18PM +0100, Daniel Lezcano wrote: >>>> The mutex protects a per_cpu variable access. The potential race can >>>> happen only when the cpufreq governor module is loaded and at the same >>>> time the cpu capacity is changed in the sysfs. >>>> >>> >>> I wonder if we really need that sysfs entry to be writable. For some >>> reason, I had assumed it's read only, obviously it's not. I prefer to >>> make it RO if there's no strong reason other than debug purposes. >> >> Are you suggesting to remove the READ_ONCE/WRITE_ONCE patch and set the >> sysfs file read-only ? >> > > Just to be sure, if we retain RW capability we still need to fix the > race you are pointing out. > > However I just don't see the need for RW cpu_capacity sysfs and hence > asking the reason here. IIRC I had pointed this out long back(not sure > internally or externally) but seemed to have missed the version that got > merged. So I am just asking if we really need write capability given that > it has known issues. > > If user-space starts writing the value to influence the scheduler, then > it makes it difficult for kernel to change the way it uses the > cpu_capacity in future. > > Sorry if there's valid usecase and I am just making noise here.
It's ok [added Juri Lelli]
I've been through the history:
commit be8f185d8af4dbd450023a42a48c6faa8cbcdfe6 Author: Juri Lelli <juri.lelli@arm.com> Date: Thu Nov 3 05:40:18 2016 +0000
arm64: add sysfs cpu_capacity attribute
Add a sysfs cpu_capacity attribute with which it is possible to read and write (thus over-writing default values) CPUs capacity. This might be useful in situations where values needs changing after boot.
The new attribute shows up as:
/sys/devices/system/cpu/cpu*/cpu_capacity
Cc: Will Deacon <will.deacon@arm.com> Cc: Mark Brown <broonie@kernel.org> Cc: Sudeep Holla <sudeep.holla@arm.com> Signed-off-by: Juri Lelli <juri.lelli@arm.com> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Juri do you have a use case where we want to override the capacity?
Shall we switch the sysfs attribute to read-only?
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |