Re: [PATCH 6/6] net: mvneta: Statically assign queues to CPUs
On Sun, Jul 05, 2015 at 03:00:11PM +0200, Willy Tarreau wrote:
Hi Thomas,
On Fri, Jul 03, 2015 at 04:46:24PM +0200, Thomas Petazzoni wrote:
Maxime,
On Fri, 3 Jul 2015 16:25:51 +0200, Maxime Ripard wrote:
+static void mvneta_percpu_enable(void *arg)
+{
+ struct mvneta_port *pp = arg;
+
+ enable_percpu_irq(pp-dev-irq, IRQ_TYPE_NONE);
+}
+
static int mvneta_open(struct net_device *dev)
{
struct mvneta_port *pp = netdev_priv(dev);
@@ -2655,6 +2662,19 @@ static int mvneta_open(struct net_device *dev)
goto err_cleanup_txqs;
}
+ /*
+ * Even though the documentation says that request_percpu_irq
+ * doesn't enable the interrupts automatically, it actually
+ * does so on the local CPU.
+ *
+ * Make sure it's disabled.
+ */
+ disable_percpu_irq(pp-dev-irq);
+
+ /* Enable per-CPU interrupt on the one CPU we care about */
+ smp_call_function_single(rxq_def % num_online_cpus(),
+ mvneta_percpu_enable, pp, true);
What happens if that CPU goes offline through CPU hotplug?
I just tried : if I start mvneta with rxq_def=1, then my irq runs
on CPU1. Then I offline CPU1 and the irqs are automatically handled
by CPU0. Then I online CPU1 and irqs stay on CPU0.
I'm confused, I would have thought that it wouldn't even work.
I guess it can be easily solved with a cpu_notifier though.
More or less related, I found that if I enable a queue number larger
than the CPU count it does work, but then the system complains
during rmmod :
[ 877.146203] [ cut here ]
[ 877.146227] WARNING: CPU: 1 PID: 1731 at fs/proc/generic.c:552
remove_proc_entry+0x144/0x15c()
[ 877.146233] remove_proc_entry: removing non-empty directory 'irq/29',
leaking at least 'mvneta'
[ 877.146238] Modules linked in: mvneta(-) [last unloaded: mvneta]
[ 877.146254] CPU: 1 PID: 1731 Comm: rmmod Tainted: GW
4.1.1-mvebu-6-g3d317ed-dirty #5
[ 877.146260] Hardware name: Marvell Armada 370/XP (Device Tree)
[ 877.146281] [c0017194] (unwind_backtrace) from [c00126d4]
(show_stack+0x10/0x14)
[ 877.146293] [c00126d4] (show_stack) from [c04d32e4]
(dump_stack+0x74/0x90)
[ 877.146305] [c04d32e4] (dump_stack) from [c0025200]
(warn_slowpath_common+0x74/0xb0)
[ 877.146315] [c0025200] (warn_slowpath_common) from [c00252d0]
(warn_slowpath_fmt+0x30/0x40)
[ 877.146325] [c00252d0] (warn_slowpath_fmt) from [c0115694]
(remove_proc_entry+0x144/0x15c)
[ 877.146336] [c0115694] (remove_proc_entry) from [c0060fd4]
(unregister_irq_proc+0x8c/0xb0)
[ 877.146347] [c0060fd4] (unregister_irq_proc) from [c0059f84]
(free_desc+0x28/0x58)
[ 877.146356] [c0059f84] (free_desc) from [c005a028]
(irq_free_descs+0x44/0x80)
[ 877.146368] [c005a028] (irq_free_descs) from [bf015748]
(mvneta_remove+0x3c/0x4c [mvneta])
[ 877.146382] [bf015748] (mvneta_remove [mvneta]) from [c024d6e8]
(platform_drv_remove+0x18/0x30)
[ 877.146393] [c024d6e8] (platform_drv_remove) from [c024bd50]
(__device_release_driver+0x70/0xe4)
[ 877.146402] [c024bd50] (__device_release_driver) from [c024c5b8]
(driver_detach+0xcc/0xd0)
[ 877.146411] [c024c5b8] (driver_detach) from [c024bbe0]
(bus_remove_driver+0x4c/0x90)
[ 877.146425] [c024bbe0] (bus_remove_driver) from [c007d3f0]
(SyS_delete_module+0x164/0x1b4)
[ 877.146437] [c007d3f0] (SyS_delete_module) from [c000f4c0]
(ret_fast_syscall+0x0/0x3c)
[ 877.146443] ---[ end trace 48713a9ae31204b1 ]---
This was on the AX3 (dual-proc) with rxq_def=2.
Hmmm, interesting, I'll look into it, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
Re: [PATCH 6/6] net: mvneta: Statically assign queues to CPUs
Hi Thomas,
On Fri, Jul 03, 2015 at 04:46:24PM +0200, Thomas Petazzoni wrote:
Maxime,
On Fri, 3 Jul 2015 16:25:51 +0200, Maxime Ripard wrote:
+static void mvneta_percpu_enable(void *arg)
+{
+ struct mvneta_port *pp = arg;
+
+ enable_percpu_irq(pp-dev-irq, IRQ_TYPE_NONE);
+}
+
static int mvneta_open(struct net_device *dev)
{
struct mvneta_port *pp = netdev_priv(dev);
@@ -2655,6 +2662,19 @@ static int mvneta_open(struct net_device *dev)
goto err_cleanup_txqs;
}
+ /*
+* Even though the documentation says that request_percpu_irq
+* doesn't enable the interrupts automatically, it actually
+* does so on the local CPU.
+*
+* Make sure it's disabled.
+*/
+ disable_percpu_irq(pp-dev-irq);
+
+ /* Enable per-CPU interrupt on the one CPU we care about */
+ smp_call_function_single(rxq_def % num_online_cpus(),
+mvneta_percpu_enable, pp, true);
What happens if that CPU goes offline through CPU hotplug?
I just tried : if I start mvneta with rxq_def=1, then my irq runs on
CPU1. Then I offline CPU1 and the irqs are automatically handled by CPU0.
Then I online CPU1 and irqs stay on CPU0.
More or less related, I found that if I enable a queue number larger than
the CPU count it does work, but then the system complains during rmmod :
[ 877.146203] [ cut here ]
[ 877.146227] WARNING: CPU: 1 PID: 1731 at fs/proc/generic.c:552
remove_proc_entry+0x144/0x15c()
[ 877.146233] remove_proc_entry: removing non-empty directory 'irq/29',
leaking at least 'mvneta'
[ 877.146238] Modules linked in: mvneta(-) [last unloaded: mvneta]
[ 877.146254] CPU: 1 PID: 1731 Comm: rmmod Tainted: GW
4.1.1-mvebu-6-g3d317ed-dirty #5
[ 877.146260] Hardware name: Marvell Armada 370/XP (Device Tree)
[ 877.146281] [c0017194] (unwind_backtrace) from [c00126d4]
(show_stack+0x10/0x14)
[ 877.146293] [c00126d4] (show_stack) from [c04d32e4]
(dump_stack+0x74/0x90)
[ 877.146305] [c04d32e4] (dump_stack) from [c0025200]
(warn_slowpath_common+0x74/0xb0)
[ 877.146315] [c0025200] (warn_slowpath_common) from [c00252d0]
(warn_slowpath_fmt+0x30/0x40)
[ 877.146325] [c00252d0] (warn_slowpath_fmt) from [c0115694]
(remove_proc_entry+0x144/0x15c)
[ 877.146336] [c0115694] (remove_proc_entry) from [c0060fd4]
(unregister_irq_proc+0x8c/0xb0)
[ 877.146347] [c0060fd4] (unregister_irq_proc) from [c0059f84]
(free_desc+0x28/0x58)
[ 877.146356] [c0059f84] (free_desc) from [c005a028]
(irq_free_descs+0x44/0x80)
[ 877.146368] [c005a028] (irq_free_descs) from [bf015748]
(mvneta_remove+0x3c/0x4c [mvneta])
[ 877.146382] [bf015748] (mvneta_remove [mvneta]) from [c024d6e8]
(platform_drv_remove+0x18/0x30)
[ 877.146393] [c024d6e8] (platform_drv_remove) from [c024bd50]
(__device_release_driver+0x70/0xe4)
[ 877.146402] [c024bd50] (__device_release_driver) from [c024c5b8]
(driver_detach+0xcc/0xd0)
[ 877.146411] [c024c5b8] (driver_detach) from [c024bbe0]
(bus_remove_driver+0x4c/0x90)
[ 877.146425] [c024bbe0] (bus_remove_driver) from [c007d3f0]
(SyS_delete_module+0x164/0x1b4)
[ 877.146437] [c007d3f0] (SyS_delete_module) from [c000f4c0]
(ret_fast_syscall+0x0/0x3c)
[ 877.146443] ---[ end trace 48713a9ae31204b1 ]---
This was on the AX3 (dual-proc) with rxq_def=2.
Hoping this helps,
Willy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] net: mvneta: Statically assign queues to CPUs
Maxime,
On Fri, 3 Jul 2015 16:25:51 +0200, Maxime Ripard wrote:
+static void mvneta_percpu_enable(void *arg)
+{
+ struct mvneta_port *pp = arg;
+
+ enable_percpu_irq(pp-dev-irq, IRQ_TYPE_NONE);
+}
+
static int mvneta_open(struct net_device *dev)
{
struct mvneta_port *pp = netdev_priv(dev);
@@ -2655,6 +2662,19 @@ static int mvneta_open(struct net_device *dev)
goto err_cleanup_txqs;
}
+ /*
+ * Even though the documentation says that request_percpu_irq
+ * doesn't enable the interrupts automatically, it actually
+ * does so on the local CPU.
+ *
+ * Make sure it's disabled.
+ */
+ disable_percpu_irq(pp-dev-irq);
+
+ /* Enable per-CPU interrupt on the one CPU we care about */
+ smp_call_function_single(rxq_def % num_online_cpus(),
+ mvneta_percpu_enable, pp, true);
What happens if that CPU goes offline through CPU hotplug?
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
