lkml.org 
[lkml]   [2014]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] cpufreq, store_scaling_governor requires policy->rwsem to be held for duration of changing governors [v2]
On 07/31/2014 11:26 AM, Prarit Bhargava wrote:
>
>
> On 07/31/2014 02:38 PM, Rafael J. Wysocki wrote:
>> On Thursday, July 31, 2014 01:57:29 PM Prarit Bhargava wrote:
>>>
>>> On 07/31/2014 12:36 PM, Rafael J. Wysocki wrote:
>>>> On Thursday, July 31, 2014 06:23:18 AM Prarit Bhargava wrote:
>>>>>
>>>>> On 07/30/2014 10:16 PM, Rafael J. Wysocki wrote:
>>>>>> On Wednesday, July 30, 2014 06:36:00 PM Saravana Kannan wrote:
>>>>>>> On 07/30/2014 02:40 PM, Rafael J. Wysocki wrote:
>>>>>>>> On Wednesday, July 30, 2014 10:18:25 AM Prarit Bhargava wrote:
>>>>>>>>>
>>>>>>>>> On 07/29/2014 08:03 PM, Rafael J. Wysocki wrote:
>>>>>>>>>> On Tuesday, July 29, 2014 07:46:02 AM Prarit Bhargava wrote:
>>>>>>>>
>>>>>>>> [cut]
>>>>>>>>
>>>>>>>>>>> This patch effectively reverts commit 955ef483.
>>>>>>>
>>>>>>> The issue reported in this patch is valid. We are seeing that internally
>>>>>>> too. I believe I reported it in another thread (within the past month).
>>>>>>>
>>>>>>> However, the original patch fixes a real deadlock issue (I'm too tired
>>>>>>> to look it up now). We can revet the original, but it's going to bring
>>>>>>> back the original issue. I just want to make sure Prarit and Raphael
>>>>>>> realize this before proceeding.
>>>>>>>
>>>>>>> I do have plans for a proper fix for the mainline (not stable branches),
>>>>>>> but plan to do that after the current set of suspend/hotplug patches go
>>>>>>> through. The fix would be easier to make after that.
>>>>>>>
>>>>>>>>>>
>>>>>>>>>> OK, I'm convinced by this.
>>>>>>>>>>
>>>>>>>>>> I suppose we should push it for -stable from 3.10 through 3.15.x, right?
>>>>>>>>>
>>>>>>>>> Rafael, I think that is a good idea. I'm not sure what the protocol is for
>>>>>>>>> adding stable@kernel.org though ...
>>>>>
>>>>> Rafael, let me (again) re-write the patch description. I think Saravana has
>>>>> raised an important issue that I have not clearly identified why it is safe to
>>>>> remove this code in my patch description. Also, I want to clearly identify the
>>>>> appropriate -stable releases to push this out to.
>>>>>
>>>>> I'll submit a [v3] later today or tomorrow.
>>>>
>>>> In any case that's too late for 3.16 final, unless there's an -rc8.
>>>>
>>>> Thanks for doing that work!
>>>
>>> Ugh ... I tried this (yet another) large system and hit another panic :(.
>>>
>>> I'm investigating now, and I'm hoping this is just something "new".
>>
>> Well, I've applied your patch as is and I can push it to Linus.
>>
>> However, if you want to update the changelog, I'll not do that, but in that
>> case the patch will have to wait for the next week.
>
> Rafael, please let it wait for next week. I _need_ to make sure this is correct
> and I'd rather not pushed something half-done.
>

Prarit,

I'm not an expert on sysfs locking, but I would think the specific sysfs
lock would depend on the file/attribute group. So, can you please try to
hotplug a core in/out (to trigger the POLICY_EXIT) and then read a sysfs
file exported by the governor? scaling_governor doesn't cut it since
that file is not removed on policy exit event to governor. If it's
ondemand, try reading/write it's sampling rate file.

The main problem here is upon POLICY_EXIT to the governor, the governor
tries to remove its sysfs file. So, if you have the policy lock held
while sending POLICY_EXIT to the governor, you'll cause the:
lock policy
lock sysfs

But trying to read the same file would cause:
lock sysfs
lock policy

-Saravana

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


\
 
 \ /
  Last update: 2014-07-31 23:01    [W:0.109 / U:0.496 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site