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: G W > 4.1.1-mvebu-00006-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
