Messages in this thread Patch in this message | | | From | Lianwei Wang <> | Date | Wed, 4 May 2016 00:23:52 -0700 | Subject | Re: [PATCH] cpu/hotplug: handle unbalanced hotplug enable/disable |
| |
On Mon, May 2, 2016 at 1:11 AM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 29 Apr 2016, Lianwei Wang wrote: >> On Thu, Apr 28, 2016 at 5:44 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > Wrong. This is the symptom. The root cause is in #1. Therefor you are trying >> > to fix the symptom and not the root cause >> > >> I don't understand why you keep saying that the issue is in the pm >> notifier callback. As I told you, the pm notifier return an error(or >> NOTIFY_BAD) on purpose to abort the suspend process. This is work as >> design. Any driver can abort the suspend process if it is not ready to >> suspend. > > Right. That's not the issue. The issue is that as a consequence we end up with > an unbalanced count. So how do we end up with an unbalanced count? That's what > needs to be fixed and not worked around. > In this example, the unbalanced count is caused by the cpu_hotplug_pm_callback pm notifier callback function. We can add a variable to avoid the unbalanced call of cpu_hotplug_enable ,e.g. diff --git a/kernel/cpu.c b/kernel/cpu.c index 3e3f6e49eabb..aa6694f0e9d3 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -1140,16 +1140,21 @@ static int cpu_hotplug_pm_callback(struct notifier_block *nb, unsigned long action, void *ptr) { + static int disabled; + switch (action) {
case PM_SUSPEND_PREPARE: case PM_HIBERNATION_PREPARE: cpu_hotplug_disable(); + disabled = 1; break;
case PM_POST_SUSPEND: case PM_POST_HIBERNATION: - cpu_hotplug_enable(); + if (disabled) + cpu_hotplug_enable(); + disabled = 0; break;
default: Please let me know if you like to fix it in this way.
But actually I think we don't need to add a new variable to check if the cpu_hotplug_disable() is called or not. We already have a disable counter which can be used to check if the cpu_hotpug_disable is called or not, as my original patch do in cpu_hotplug_enable() function. Maybe the reset comments and reset cpu_hotplug_disabled to 0 operation confuse you. I should check it firstly and do nothing if it is already 0. e.g. +static void _cpu_hotplug_enable(void) +{ + if (WARN(!cpu_hotplug_disabled, "Unbalanced cpu hotplug enable\n")) + return; + + cpu_hotplug_disabled--; +} I like to fix it in the cpu_hotplug_enable because it is a public kernel API and fix in it can prevent any other unbalanced calling. I will update the patch.
>> > I completely understand that you are tyring to put the cart before the horse. >> No. Your understanding is wrong. > > My understanding is very correct. We have a situation which leads to an > unbalanced count. Instead of fixing that, you fix up the unbalanced count. > Yes, that's right. We are on the same page. The only difference is that where/how to fix it. See my two solutions above and let me know which one you like? > Thanks, > > tglx
| |