On Wed, Aug 31, 2011 at 9:51 PM, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote:
> On Thu, Sep 1, 2011 at 5:05 AM, Turquette, Mike <mturquette@xxxxxx> wrote:
>> On Wed, Aug 31, 2011 at 12:29 AM, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote:
[snip]
>>> +static void devfreq_monitor(struct work_struct *work)
>>> +{
>>> + static unsigned long last_polled_at;
>>> + struct devfreq *devfreq, *tmp;
>>> + int error;
>>> + unsigned long jiffies_passed;
>>> + unsigned long next_jiffies = ULONG_MAX, now = jiffies;
>>> +
>>> + /* Initially last_polled_at = 0, polling every device at bootup */
>>> + jiffies_passed = now - last_polled_at;
>>> + last_polled_at = now;
>>> + if (jiffies_passed == 0)
>>> + jiffies_passed = 1;
>>> +
>>> + mutex_lock(&devfreq_list_lock);
>>
>> Should not lock the list here. If we lock the list for all major
>> operations, it nullifies the performance benefit of having a mutex in
>> struct devfreq.
>>
>
> Ok... then.. how about locking like this? :
>
> mutex_lock(&devfreq_list_lock);
> list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
> mutex_lock(&devfreq->lock);
> mutex_unlock(&devfreq_list_lock);
>
> blahblah
>
> mutex_unlock(&devfreq->lock);
> mutex_lock(&devfreq_list_lock);
> }
> mutex_unlock(&devfreq_list_lock);
I took a step back from the code to rethink the big picture, and I've
come back to the same conclusion conclusion that I had in the V5
patchset. (https://patchwork.kernel.org/patch/1043442/)
Firstly, there is no reason to walk the list here. All of the locking
discussion here could just go away if we didn't walk the list of
devfreq devices every time the delay_work gets fired. If each device
programmed it's own delayed work then this problem simply goes away.
If you don't mind I'd like to post an RFC of devfreq with these
changes implemented so we can review them and discuss the ideas around
some concrete code.
Best regards,
Mike
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
