lkml.org 
[lkml]   [2018]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE
From
Date
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

\
 
 \ /
  Last update: 2018-11-23 17:54    [W:0.119 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site