* [PATCH] net: mvneta: Add missing hotplug notifier transition
@ 2016-03-11 9:10 Anna-Maria Gleixner
2016-03-11 10:28 ` Gregory CLEMENT
2016-03-14 19:22 ` David Miller
0 siblings, 2 replies; 4+ messages in thread
From: Anna-Maria Gleixner @ 2016-03-11 9:10 UTC (permalink / raw)
To: linux-kernel; +Cc: rt, Anna-Maria Gleixner, Thomas Petazzoni, netdev
The mvneta_percpu_notifier() hotplug callback lacks handling of the
CPU_DOWN_FAILED case. That means, if CPU_DOWN_PREPARE failes, the
driver is not well configured on the CPU.
Add handling for CPU_DOWN_FAILED[_FROZEN] hotplug notifier transition
to setup the driver.
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
---
drivers/net/ethernet/marvell/mvneta.c | 2 ++
1 file changed, 2 insertions(+)
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2920,6 +2920,8 @@ static int mvneta_percpu_notifier(struct
switch (action) {
case CPU_ONLINE:
case CPU_ONLINE_FROZEN:
+ case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
spin_lock(&pp->lock);
/* Configuring the driver for a new CPU while the
* driver is stopping is racy, so just avoid it.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: mvneta: Add missing hotplug notifier transition
2016-03-11 9:10 [PATCH] net: mvneta: Add missing hotplug notifier transition Anna-Maria Gleixner
@ 2016-03-11 10:28 ` Gregory CLEMENT
2016-03-11 10:48 ` Thomas Gleixner
2016-03-14 19:22 ` David Miller
1 sibling, 1 reply; 4+ messages in thread
From: Gregory CLEMENT @ 2016-03-11 10:28 UTC (permalink / raw)
To: Anna-Maria Gleixner; +Cc: linux-kernel, rt, Thomas Petazzoni, netdev
Hi Anna-Maria,
On ven., mars 11 2016, Anna-Maria Gleixner <anna-maria@linutronix.de> wrote:
> The mvneta_percpu_notifier() hotplug callback lacks handling of the
> CPU_DOWN_FAILED case. That means, if CPU_DOWN_PREPARE failes, the
> driver is not well configured on the CPU.
>
> Add handling for CPU_DOWN_FAILED[_FROZEN] hotplug notifier transition
> to setup the driver.
I agree that we need to handle this case, however reusing CPU_ONLINE
case for it seems too much. Indeed in the case of a CPU down failure all
the napi are already synchronized thanks to the CPU_DOWN_PREPARE
case. Moreover, we didn't call yet mvneta_percpu_elect() so we don't have
to call it again. I would prefer something like that:
@@ -2987,6 +2987,23 @@ static int mvneta_percpu_notifier(struct notifier_block *nfb,
pp, true);
break;
+ case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
+ /* Re-enable per-CPU interrupts on the CPU that failed
+ * to bring up.
+ */
+ smp_call_function_single(cpu, mvneta_percpu_enable,
+ pp, true);
+
+ napi_enable(&port->napi);
+ /* Unmask all ethernet port interrupts */
+ on_each_cpu(mvneta_percpu_unmask_interrupt, pp, true);
+ mvreg_write(pp, MVNETA_INTR_MISC_MASK,
+ MVNETA_CAUSE_PHY_STATUS_CHANGE |
+ MVNETA_CAUSE_LINK_CHANGE |
+ MVNETA_CAUSE_PSC_SYNC_CHANGE);
+ netif_tx_start_all_queues(pp->dev);
+ break;
case CPU_DEAD:
case CPU_DEAD_FROZEN:
/* Check if a new CPU must be elected now this on is down */
Also it would be nice to appy this fix on the stable kernel, the bug was
introduced with this commit: f86428854480 ("net: mvneta: Statically
assign queues to CPUs")
Thanks,
Gregory
>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> ---
> drivers/net/ethernet/marvell/mvneta.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -2920,6 +2920,8 @@ static int mvneta_percpu_notifier(struct
> switch (action) {
> case CPU_ONLINE:
> case CPU_ONLINE_FROZEN:
> + case CPU_DOWN_FAILED:
> + case CPU_DOWN_FAILED_FROZEN:
> spin_lock(&pp->lock);
> /* Configuring the driver for a new CPU while the
> * driver is stopping is racy, so just avoid it.
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: mvneta: Add missing hotplug notifier transition
2016-03-11 10:28 ` Gregory CLEMENT
@ 2016-03-11 10:48 ` Thomas Gleixner
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Gleixner @ 2016-03-11 10:48 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Anna-Maria Gleixner, Thomas Petazzoni, netdev, linux-kernel, rt
Gregory,
On Fri, 11 Mar 2016, Gregory CLEMENT wrote:
> On ven., mars 11 2016, Anna-Maria Gleixner <anna-maria@linutronix.de> wrote:
>
> > The mvneta_percpu_notifier() hotplug callback lacks handling of the
> > CPU_DOWN_FAILED case. That means, if CPU_DOWN_PREPARE failes, the
> > driver is not well configured on the CPU.
> >
> > Add handling for CPU_DOWN_FAILED[_FROZEN] hotplug notifier transition
> > to setup the driver.
>
> I agree that we need to handle this case, however reusing CPU_ONLINE
> case for it seems too much.
Why is that too much? If it does the job, then it's definitely preferrable
over an extra case for a non hotpath error handling operation.
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: mvneta: Add missing hotplug notifier transition
2016-03-11 9:10 [PATCH] net: mvneta: Add missing hotplug notifier transition Anna-Maria Gleixner
2016-03-11 10:28 ` Gregory CLEMENT
@ 2016-03-14 19:22 ` David Miller
1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2016-03-14 19:22 UTC (permalink / raw)
To: anna-maria; +Cc: linux-kernel, rt, thomas.petazzoni, netdev
From: Anna-Maria Gleixner <anna-maria@linutronix.de>
Date: Fri, 11 Mar 2016 10:10:23 +0100
> The mvneta_percpu_notifier() hotplug callback lacks handling of the
> CPU_DOWN_FAILED case. That means, if CPU_DOWN_PREPARE failes, the
> driver is not well configured on the CPU.
>
> Add handling for CPU_DOWN_FAILED[_FROZEN] hotplug notifier transition
> to setup the driver.
>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-14 19:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-11 9:10 [PATCH] net: mvneta: Add missing hotplug notifier transition Anna-Maria Gleixner
2016-03-11 10:28 ` Gregory CLEMENT
2016-03-11 10:48 ` Thomas Gleixner
2016-03-14 19:22 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).