lkml.org 
[lkml]   [2016]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] cpu/hotplug: handle unbalanced hotplug enable/disable
I have come up a patch to make the pm notifier called symmetrically
and currently being tested. I will send it out after pass the test.

On Fri, May 6, 2016 at 12:18 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 6 May 2016, Lianwei Wang wrote:
>> On Thu, May 5, 2016 at 5:13 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > Can you eventually come up with a coherent explanation of the problem down to
>> > the root cause or are we going to play this "move the workaround one step
>> > down" game for another 10 rounds?
>> >
>> Do you agree that any driver can abort the suspend process by
>> returning an error or NOTIFY_BAD if it is not ready to suspend?
>> I have explain it and I also copied the example code that abort
>> suspend by returning an error or NOTIFY_BAD in the pm notifier
>> callback function.
>
> I don't need copied example code which does not tell me what the real problem
> is.
>
>> The cpu_hotplug_disable and cpu_hotplug_enable are called in one of
>> the PM notifier callback. And they are called from two difference
>> place.
>> Below is how it happened:
>> pm_suspend
>> |--enter_state
>> |--suspend_prepare
>> |--pm_notifier_call_chain(PM_SUSPEND_PREPARE)
>> | |--call_back_1
>> | |--call_back_..
>> | |--call_back_n ===> return NOTIFY_BAD to abort call chain and
>> | | suspend process here
>> | |--cpu_hotplug_pm_callback()
>> | | |--cpu_hotplug_disable =====> remember it is not
>> called yet
>> | |--call_back_..
>> |
>> |--pm_notifier_call_chain(PM_POST_SUSPEND)
>> | |--call_back_1
>> | |--call_back_..
>> | |--call_back_n
>> | |--cpu_hotplug_pm_callback()
>> | | |--cpu_hotplug_enable =====> Here it is unbalanced called
>> | |--call_back_..
>> |
>> So, keep in mind that for pm notifier call chain, the
>> PM_SUSPEND_PREPARE notifier and PM_POST_SUSPEND notifier is not always
>> paired called. Sometimes for a driver's pm notifier callback, the
>> PM_POST_SUSPEND is called without PM_SUSPEND_PREPARE.
>
> So that is the real problem: cpu_hotplug_pm_callback(PM_POST_SUSPEND) can be
> called w/o a previous call to cpu_hotplug_pm_callback(PM_SUSPEND_PREPARE).
>
>> > It cannot prevent any unbalanced calls. It mitigates the issue, but that's a
>> > different problem.
>> It did not migrate the issue. It give a warning message to log the
>> unbalanced issue and it also make sure the cpu hotplug continue to
>> work well even someone do an unbalanced call. It is a good checking as
>> the enable_irq/disable_irq do. There are some other unbalanced
>> checking in kernel too. All make sure the kernel has a better
>> stability.
>
> I'm not opposed to do that and I said so several times. But I said as well,
> that we do not add this without fixing the problem which made you write that
> patch in the first place.
>
> So we have a proper explanation for the real problem now, but we have no
> fix.
>
> And again: Your patch is NOT a fix. Simply because it will emit a warning
> everytime the above happens. And that's wrong because the abort is a
> legitimate scenario.
>
> So please come up with a sensible fix for the suspend abort issue and then we
> can add the balance check/fixup to the hotplug_disable/enable() code.
>
> Thanks,
>
> tglx
>
>

\
 
 \ /
  Last update: 2016-05-12 10:41    [W:0.094 / U:4.360 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site