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

[Sudeep Holla <sudeep.holla@arm.com>]

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