Re: [PATCH v2] cpu/hotplug: handle unbalanced hotplug enable/disable

[Lianwei Wang]

On Mon, Jun 6, 2016 at 11:34 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> On Mon, 6 Jun 2016, Lianwei Wang wrote:
>> Currently it just print a warning message but did not
>> reset cpu_hotplug_disabled when the enable/disable is
>> unbalanced. The unbalanced enable/disable will lead
>> the cpu hotplug work abnormally.
>>
>> Do nothing if an unablanced hotplug enable detected.
>>
>> Signed-off-by: Lianwei Wang <lianwei.wang@xxxxxxxxx>
>> ---
>>  kernel/cpu.c | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/cpu.c b/kernel/cpu.c
>> index 3e3f6e49eabb..8011b1e40523 100644
>> --- a/kernel/cpu.c
>> +++ b/kernel/cpu.c
>> @@ -245,6 +245,19 @@ void cpu_hotplug_done(void)
>>       cpuhp_lock_release();
>>  }
>>
>> +static void _cpu_hotplug_disable(void)
>
> What's the purpose of this function?
>
The only purpose is to make the cpu_hotplug_disable as one API and
always disable it from the same interface. It makes the code looks
more beautiful. But yes, we can remove it since it is not necessary
for this change.

>> +{
>> +     cpu_hotplug_disabled++;
>> +}
>> +
>> +static void _cpu_hotplug_enable(void)
>
> Double underscores please
>
Ok, I will update it.
>> +{
>> +     if (WARN(!cpu_hotplug_disabled, "Unbalanced cpu hotplug enable\n"))
>> +             return;
>
> And this want's to be a WARN_ONCE()
>
WARN is convenient for debugging because kernel buffer is limited and
we might lose the first warning message. But WARN_ONCE is good to me
too.

> Thanks,
>
>         tglx