linux-kernel - Re: [PATCH] cpu/hotplug: handle unbalanced hotplug enable/disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJFUiJgSDwtpXWPcGu704=5tggOntmyfk=muOtDLRcSUJQc8Rg@mail.gmail.com>
Date:	Fri, 29 Apr 2016 14:47:22 -0700
From:	Lianwei Wang <lianwei.wang@...il.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Peter Zijlstra <peterz@...radead.org>, oleg@...hat.com,
	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org
Subject: Re: [PATCH] cpu/hotplug: handle unbalanced hotplug enable/disable

On Thu, Apr 28, 2016 at 5:44 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
> On Thu, 28 Apr 2016, Lianwei Wang wrote:
>> On Wed, Apr 27, 2016 at 11:15 PM, Thomas Gleixner <tglx@...utronix.de> wrote:
>> > On Wed, 27 Apr 2016, Lianwei Wang wrote:
>> >> Yes. In our project, there is a kernel driver which register a pm
>> >> notifier. On some conditions this pm notifier will return an error and
>> >> abort the suspend process. The counter will be unbalanced in case it
>> >> happened.
>> >
>> > So what? You wreckaged your driver, so you fix it and be done with it.
>>
>> Do you mean no pm_notifier callback can return an error or NOTIFY_BAD
>> to abort the suspend process?
>>
>> It's not the driver issue. The driver return an error to abort the
>> suspend process on purpose. Why do you think it is not allowed to
>> return an error to abort suspend?
>>
>> The issue is very clear as described below.
>> 1. How the issue happened?
>> One of the pm notifier return error to abort suspend before
>> cpu_hotplug_disable() is called on PM_SUSPEND_PREPARE.
>>
>> 2. What's the result?
>> CPU hotplug work in a wrong way, or it doesn't work anymore. No way to
>> recover it.
>>
>> 3. The root cause is that there is no any handling for the unbalanced
>> cpu_hotplug_disable/enable calling. This patch add a protection for
>> such issue.
>
> 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.

I know your maintainers are busy but it is not hard for you to
understand it. If you did not look at the suspend code for a long time
then you can look at it now.

Below are some examples to return error to abort the suspend on
PM_SUSPEND_PREPARE pm notifier.
http://lxr.free-electrons.com/source/arch/x86/power/cpu.c#L290
http://lxr.free-electrons.com/source/arch/s390/kernel/suspend.c#L164
http://lxr.free-electrons.com/source/drivers/s390/cio/css.c#L840
http://lxr.free-electrons.com/source/drivers/devfreq/exynos/exynos5_bus.c#L202

>> Anything not clear?
>
> No.
>
> I completely understand that you are tyring to put the cart before the horse.
No. Your understanding is wrong.
>
> Thanks,
>
>         tglx