Messages in this thread | | | From | "Elliott, Robert (Server Storage)" <> | Subject | RE: Locking issues with cpufreq and sysfs | Date | Tue, 14 Oct 2014 19:18:25 +0000 |
| |
> -----Original Message----- > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > owner@vger.kernel.org] On Behalf Of Prarit Bhargava > Sent: Tuesday, 14 October, 2014 1:24 PM > To: Viresh Kumar > Cc: Saravana Kannan; Rafael J. Wysocki; linux-pm@vger.kernel.org; Linux > Kernel; Robert Schöne > Subject: Re: Locking issues with cpufreq and sysfs > > On 10/14/2014 03:10 AM, Viresh Kumar wrote: > > On 13 October 2014 18:41, Prarit Bhargava <prarit@redhat.com> wrote: > >> > >> The locking is insufficient here, Viresh. I no longer believe that fixes > >> to this locking scheme are the right way to move forward here. I'm > wondering > >> if we can look at other alternatives such as maintaining a refcount or > >> perhaps using a queuing mechanism for governor and policy related changes. > >> ... > So I'm proposing that we move to a single threaded read/write using, if > possible, a single policy lock for now. We might transition this back to a > rwsem later on, however, for the first attempt at cleaning this up I think we > should just stick with a simple lock. In doing that, IMO we remove > cpufreq_rwsem: protects the driver from being unloaded > cpufreq_governor_lock: protects the current governor > each policy has a rwsem (policy->rwsem): protects the cpufreq_policy struct > > and potentially > cpufreq_driver_lock: protects the cpufreq_cpu_data array and cpufreq_driver- > >boost > > After looking at the way the code would be structured, I'm wondering if > cpufreq_governor_mutex: protects the cpufreq_governor_list > is overkill. The loading of a module should be atomic relative to the > cpufreq code, so this lock may not be required. (Admittedly I haven't > tested that...) > > That would leave: > global_kobj_lock: protects the "cpufreq" kobject > each policy has a transition_lock (policy->transition): synchronizes > frequency transitions > and a new lock, perhaps called policy->lock, to serialize all events. >
Please keep performance in mind too. cpufreq_governor_lock contention is a bit of an issue with heavy IO workloads as described in: http://marc.info/?l=linux-pm&m=140924051503827&w=2
--- Rob Elliott HP Server Storage
| |