Re: [REGRESSION] Commit a66b2e50 "Preserve sysfs files across suspend/resume" causes a regression in intel_pstate

Re: [REGRESSION] Commit a66b2e50 "Preserve sysfs files across suspend/resume" causes a regression in intel_pstate

[Rafael J. Wysocki]

On Wednesday, July 10, 2013 09:15:12 AM Dirk Brandewie wrote:
> Hi All,

Hi,

> Tianyu debugged into https://bugzilla.kernel.org/show_bug.cgi?id=59781 and found
> that commit a66b2e50 is causing the regression.
> 
> Tianyu has proposed a fix (patch attached to bugzilla) but having scaling
> drivers receive hotplug notifications through two paths seems weird.
> 
> Looking at the core code and some of the other scaling drivers
> I don't see an obvious fix.  Maybe adding optional suspend/resume callbacks
> to the scaling driver interface?
> 
> All the scaling drivers that need to do stateful  work in the init/exit
> callbacks are being affected by this change so I think there are other
> subtle side-effects out there that haven't been noticed yet.
> 
> I am not sure how we should proceed here?

Well, first off, I'll queue up a revert of commit a66b2e50, as this was really
about being nice to user space than anything else.  And it has caused subtle
problems to happen already elsewhere.

Then we can figure out how to address the original issue.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [REGRESSION] Commit a66b2e50 "Preserve sysfs files across suspend/resume" causes a regression in intel_pstate

[Srivatsa S. Bhat]

On 07/11/2013 03:34 AM, Rafael J. Wysocki wrote:
> On Wednesday, July 10, 2013 09:15:12 AM Dirk Brandewie wrote:
>> Hi All,
> 
> Hi,
> 
>> Tianyu debugged into https://bugzilla.kernel.org/show_bug.cgi?id=59781 and found
>> that commit a66b2e50 is causing the regression.
>>
>> Tianyu has proposed a fix (patch attached to bugzilla) but having scaling
>> drivers receive hotplug notifications through two paths seems weird.
>>
>> Looking at the core code and some of the other scaling drivers
>> I don't see an obvious fix.  Maybe adding optional suspend/resume callbacks
>> to the scaling driver interface?
>>
>> All the scaling drivers that need to do stateful  work in the init/exit
>> callbacks are being affected by this change so I think there are other
>> subtle side-effects out there that haven't been noticed yet.
>>
>> I am not sure how we should proceed here?
> 
> Well, first off, I'll queue up a revert of commit a66b2e50, as this was really
> about being nice to user space than anything else.  And it has caused subtle
> problems to happen already elsewhere.
> 
> Then we can figure out how to address the original issue.
> 

Sure, that sounds like a good plan. Sorry about the mess it caused :(
I do have a (large-ish) fix in the works (which I posted in my previous mail),
but reverting the original commit first will be better, especially from the
stable-tree perspective.

I can provide you the revert tomorrow after testing (the revert needs to take
care of commit f51e1eb6 which went in later), but if you do it yourself by then,
you can add my ACK to it.

Thanks a lot!
Regards,
Srivatsa S. Bhat