linux-kernel - Re: [patch V2 43/67] leds/trigger/cpu: Convert to hotplug state machine

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]

Date:	Thu, 14 Jul 2016 09:19:27 +0200
From:	Jacek Anaszewski <j.anaszewski@...sung.com>
To:	Anna-Maria Gleixner <anna-maria@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>, rt@...utronix.de,
	Richard Cochran <rcochran@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Richard Purdie <rpurdie@...ys.net>, linux-leds@...r.kernel.org
Subject: Re: [patch V2 43/67] leds/trigger/cpu: Convert to hotplug state machine

Hi Anna,

On 07/13/2016 07:16 PM, Anna-Maria Gleixner wrote:

 > -------- Original Message --------
 > Subject: [patch V2 43/67] leds/trigger/cpu: Convert to hotplug state 
machine
 > Date: Wed, 13 Jul 2016 17:16:45 +0000
 > From: Anna-Maria Gleixner <anna-maria@...utronix.de>
 > To: LKML <linux-kernel@...r.kernel.org>
 > CC: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar 
<mingo@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, 
rt@...utronix.de, Richard Cochran <rcochran@...utronix.de>, Sebastian 
Andrzej Siewior <bigeasy@...utronix.de>, Jacek Anaszewski 
<j.anaszewski@...sung.com>, Linus Torvalds 
<torvalds@...ux-foundation.org>, Linus Walleij 
<linus.walleij@...aro.org>, Paul Gortmaker 
<paul.gortmaker@...driver.com>, Richard Purdie <rpurdie@...ys.net>, 
linux-leds@...r.kernel.org, Anna-Maria Gleixner <anna-maria@...utronix.de>
 >
 > From: Richard Cochran <rcochran@...utronix.de>
 >
 > This is a straightforward conversion. We place this callback last
 > in the list so that the LED illuminates only after a successful
 > bring up sequence.
 >
 > Signed-off-by: Richard Cochran <rcochran@...utronix.de>
 > Reviewed-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
 > Cc: Jacek Anaszewski <j.anaszewski@...sung.com>
 > Cc: Linus Torvalds <torvalds@...ux-foundation.org>
 > Cc: Linus Walleij <linus.walleij@...aro.org>
 > Cc: Paul Gortmaker <paul.gortmaker@...driver.com>
 > Cc: Peter Zijlstra <peterz@...radead.org>
 > Cc: Richard Purdie <rpurdie@...ys.net>
 > Cc: Thomas Gleixner <tglx@...utronix.de>
 > Cc: linux-leds@...r.kernel.org
 > Signed-off-by: Anna-Maria Gleixner <anna-maria@...utronix.de>
 > ---
 >  drivers/leds/trigger/ledtrig-cpu.c | 32 +++++++++++++++-----------------
 >  include/linux/cpuhotplug.h         |  1 +
 >  2 files changed, 16 insertions(+), 17 deletions(-)
 >
 > diff --git a/drivers/leds/trigger/ledtrig-cpu.c 
b/drivers/leds/trigger/ledtrig-cpu.c
 > index 938467f..4a6a182 100644
 > --- a/drivers/leds/trigger/ledtrig-cpu.c
 > +++ b/drivers/leds/trigger/ledtrig-cpu.c
 > @@ -92,25 +92,17 @@ static struct syscore_ops ledtrig_cpu_syscore_ops = {
 >       .resume         = ledtrig_cpu_syscore_resume,
 >  };
 >
 > -static int ledtrig_cpu_notify(struct notifier_block *self,
 > -                                        unsigned long action, void 
*hcpu)
 > +static int ledtrig_starting_cpu(unsigned int cpu)
 >  {
 > -     switch (action & ~CPU_TASKS_FROZEN) {
 > -     case CPU_STARTING:
 > -             ledtrig_cpu(CPU_LED_START);
 > -             break;
 > -             break;
 > -     }
 > -
 > -     return NOTIFY_OK;
 > +     ledtrig_cpu(CPU_LED_START);
 > +     return 0;
 >  }
 >
 > -
 > -static struct notifier_block ledtrig_cpu_nb = {
 > -     .notifier_call = ledtrig_cpu_notify,
 > -};
 > +static int ledtrig_dying_cpu(unsigned int cpu)
 > +{
 > +     ledtrig_cpu(CPU_LED_STOP);
 > +     return 0;
 > +}
 >
 >  static int __init ledtrig_cpu_init(void)
 >  {
 > @@ -133,7 +125,13 @@ static int __init ledtrig_cpu_init(void)
 >       }
 >
 >       register_syscore_ops(&ledtrig_cpu_syscore_ops);
 > -     register_cpu_notifier(&ledtrig_cpu_nb);
 > +
 > +     /*
 > +      * FIXME: Why needs this to happen in the interrupt disabled
 > +      * low level bringup phase of a cpu?
 > +      */

Why wasn't it possible to clarify the issue before the
submission?

 > +     cpuhp_setup_state(CPUHP_AP_LEDTRIG_STARTING, "AP_LEDTRIG_STARTING",
 > +                       ledtrig_starting_cpu, ledtrig_dying_cpu);
 >
 >       pr_info("ledtrig-cpu: registered to indicate activity on CPUs\n");
 >
 > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
 > index c052b04..ef7bfa6 100644
 > --- a/include/linux/cpuhotplug.h
 > +++ b/include/linux/cpuhotplug.h
 > @@ -43,6 +43,7 @@ enum cpuhp_state {
 >       CPUHP_AP_QCOM_TIMER_STARTING,
 >       CPUHP_AP_MIPS_GIC_TIMER_STARTING,
 >       CPUHP_AP_KVM_STARTING,
 > +     CPUHP_AP_LEDTRIG_STARTING,
 >       CPUHP_AP_NOTIFY_STARTING,
 >       CPUHP_AP_ONLINE,
 >       CPUHP_TEARDOWN_CPU,
 > --


-- 
Best regards,
Jacek Anaszewski