Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Mark Brown]

On Mon, Nov 26, 2018 at 09:27:02AM +0100, Juri Lelli wrote:

> is highly debatable). I also seem to remember that there might also be
> cases where DT values cannot be changed at all for a (new?) platform
> that happens to be using DTs shipped with an old revision; something
> along these lines was mentioned (by Mark?) during the review process,
> but exact details escape my mind ATM, apologies.

Yeah, DTs might be in firmware which is either non-updatable or which
people are reluctant to update due to it not being recoverable if the
update goes wrong.


signature.asc
Description: PGP signature

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Mark Brown]

On Mon, Nov 26, 2018 at 09:27:02AM +0100, Juri Lelli wrote:

> is highly debatable). I also seem to remember that there might also be
> cases where DT values cannot be changed at all for a (new?) platform
> that happens to be using DTs shipped with an old revision; something
> along these lines was mentioned (by Mark?) during the review process,
> but exact details escape my mind ATM, apologies.

Yeah, DTs might be in firmware which is either non-updatable or which
people are reluctant to update due to it not being recoverable if the
update goes wrong.


signature.asc
Description: PGP signature

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

Hi Juri,

On 26/11/2018 09:27, Juri Lelli wrote:
> Hi,
> 
> On 23/11/18 17:54, Daniel Lezcano wrote:
>> 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 
>> Date:   Thu Nov 3 05:40:18 2016 +
>>
>> 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 
>> Cc: Mark Brown 
>> Cc: Sudeep Holla 
>> Signed-off-by: Juri Lelli 
>> Signed-off-by: Catalin Marinas 
>>
>> Juri do you have a use case where we want to override the capacity?
>>
>> Shall we switch the sysfs attribute to read-only?
> 
> So, I spent a bit of time researching patchset history and IIRC the
> point of having a RW cpu_capacity was to help in situations where one
> wants to change values after boot, because she/he now has "better"
> numbers (remember we advocate to use Dhrystone to populate DTs, but that
> is highly debatable). I also seem to remember that there might also be
> cases where DT values cannot be changed at all for a (new?) platform
> that happens to be using DTs shipped with an old revision; something
> along these lines was mentioned (by Mark?) during the review process,
> but exact details escape my mind ATM, apologies.

Ok, so I guess it makes sense to keep it RW then.

Thanks for the feedback.

  -- Daniel



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

Hi Juri,

On 26/11/2018 09:27, Juri Lelli wrote:
> Hi,
> 
> On 23/11/18 17:54, Daniel Lezcano wrote:
>> 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 
>> Date:   Thu Nov 3 05:40:18 2016 +
>>
>> 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 
>> Cc: Mark Brown 
>> Cc: Sudeep Holla 
>> Signed-off-by: Juri Lelli 
>> Signed-off-by: Catalin Marinas 
>>
>> Juri do you have a use case where we want to override the capacity?
>>
>> Shall we switch the sysfs attribute to read-only?
> 
> So, I spent a bit of time researching patchset history and IIRC the
> point of having a RW cpu_capacity was to help in situations where one
> wants to change values after boot, because she/he now has "better"
> numbers (remember we advocate to use Dhrystone to populate DTs, but that
> is highly debatable). I also seem to remember that there might also be
> cases where DT values cannot be changed at all for a (new?) platform
> that happens to be using DTs shipped with an old revision; something
> along these lines was mentioned (by Mark?) during the review process,
> but exact details escape my mind ATM, apologies.

Ok, so I guess it makes sense to keep it RW then.

Thanks for the feedback.

  -- Daniel



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Juri Lelli]

Hi,

On 23/11/18 17:54, Daniel Lezcano wrote:
> 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 
> Date:   Thu Nov 3 05:40:18 2016 +
> 
> 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 
> Cc: Mark Brown 
> Cc: Sudeep Holla 
> Signed-off-by: Juri Lelli 
> Signed-off-by: Catalin Marinas 
> 
> Juri do you have a use case where we want to override the capacity?
> 
> Shall we switch the sysfs attribute to read-only?

So, I spent a bit of time researching patchset history and IIRC the
point of having a RW cpu_capacity was to help in situations where one
wants to change values after boot, because she/he now has "better"
numbers (remember we advocate to use Dhrystone to populate DTs, but that
is highly debatable). I also seem to remember that there might also be
cases where DT values cannot be changed at all for a (new?) platform
that happens to be using DTs shipped with an old revision; something
along these lines was mentioned (by Mark?) during the review process,
but exact details escape my mind ATM, apologies.

Best,

- Juri

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Juri Lelli]

Hi,

On 23/11/18 17:54, Daniel Lezcano wrote:
> 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 
> Date:   Thu Nov 3 05:40:18 2016 +
> 
> 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 
> Cc: Mark Brown 
> Cc: Sudeep Holla 
> Signed-off-by: Juri Lelli 
> Signed-off-by: Catalin Marinas 
> 
> Juri do you have a use case where we want to override the capacity?
> 
> Shall we switch the sysfs attribute to read-only?

So, I spent a bit of time researching patchset history and IIRC the
point of having a RW cpu_capacity was to help in situations where one
wants to change values after boot, because she/he now has "better"
numbers (remember we advocate to use Dhrystone to populate DTs, but that
is highly debatable). I also seem to remember that there might also be
cases where DT values cannot be changed at all for a (new?) platform
that happens to be using DTs shipped with an old revision; something
along these lines was mentioned (by Mark?) during the review process,
but exact details escape my mind ATM, apologies.

Best,

- Juri

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

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 
Date:   Thu Nov 3 05:40:18 2016 +

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 
Cc: Mark Brown 
Cc: Sudeep Holla 
Signed-off-by: Juri Lelli 
Signed-off-by: Catalin Marinas 

Juri do you have a use case where we want to override the capacity?

Shall we switch the sysfs attribute to read-only?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

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 
Date:   Thu Nov 3 05:40:18 2016 +

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 
Cc: Mark Brown 
Cc: Sudeep Holla 
Signed-off-by: Juri Lelli 
Signed-off-by: Catalin Marinas 

Juri do you have a use case where we want to override the capacity?

Shall we switch the sysfs attribute to read-only?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Sudeep Holla]

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.

--
Regards,
Sudeep

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Sudeep Holla]

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.

--
Regards,
Sudeep

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

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 ?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Daniel Lezcano]

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 ?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Sudeep Holla]

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.

--
Regards,
Sudeep

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Sudeep Holla]

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.

--
Regards,
Sudeep

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Viresh Kumar]

On Mon, Oct 29, 2018 at 9:54 PM 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.
>
> There is no real interest of using a mutex to protect a variable
> assignation when there is no situation where a task can take the lock
> and block.
>
> Replace the mutex by READ_ONCE / WRITE_ONCE.
>
> Signed-off-by: Daniel Lezcano 
> ---
>  drivers/base/arch_topology.c  | 7 +--
>  include/linux/arch_topology.h | 2 +-
>  2 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Viresh Kumar 

Re: [PATCH 2/4] base/drivers/arch_topology: Replace mutex with READ_ONCE / WRITE_ONCE

[Viresh Kumar]

On Mon, Oct 29, 2018 at 9:54 PM 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.
>
> There is no real interest of using a mutex to protect a variable
> assignation when there is no situation where a task can take the lock
> and block.
>
> Replace the mutex by READ_ONCE / WRITE_ONCE.
>
> Signed-off-by: Daniel Lezcano 
> ---
>  drivers/base/arch_topology.c  | 7 +--
>  include/linux/arch_topology.h | 2 +-
>  2 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Viresh Kumar