Re: [PATCH/RFC] remove irqs_disabled warning from local_bh_enable

[Johannes Berg]

On Fri, 2008-06-20 at 17:27 +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@xxxxxxx> wrote:
> 
> > > Subject: clean up and comment local_bh_enable code
> > > 
> > > There's no need to use local_irq_save() over local_irq_disable() in 
> > > the local_bh_enable code since it is a bug to call it with irqs 
> > > disabled and do_softirq will enable irqs if there is any pending 
> > > work. Consolidate the code from local_bh_enable and ..._ip to avoid 
> > > having a disconnect between them in the warnings they trigger that 
> > > is currently there. Also always trigger the warning on in_irq(), not 
> > > just in the trace-irqflags case.
> > 
> > applied to tip/core/softirq for testing, thanks Johannes.
> 
> ok, -tip testing found that your patch triggers a new warning on an old 
> testbox that uses 3c59x vortex and netlogging:
> 
> ----->
> calling  vortex_init+0x0/0xb0
> PCI: Found IRQ 10 for device 0000:00:0b.0
> PCI: Sharing IRQ 10 with 0000:00:0a.0
> PCI: Sharing IRQ 10 with 0000:00:0b.1
> 3c59x: Donald Becker and others.
> 0000:00:0b.0: 3Com PCI 3c556 Laptop Tornado at e0800400.
> PCI: Enabling bus mastering for device 0000:00:0b.0
> initcall vortex_init+0x0/0xb0 returned 0 after 47 msecs
> ...
> calling  init_netconsole+0x0/0x1b0
> netconsole: local port 4444
> netconsole: local IP 10.0.1.9
> netconsole: interface eth0
> netconsole: remote port 4444
> netconsole: remote IP 10.0.1.16
> netconsole: remote ethernet address 00:19:xx:xx:xx:xx
> netconsole: device eth0 not up yet, forcing it
> eth0:  setting half-duplex.
> eth0:  setting full-duplex.
> ------------[ cut here ]------------
> WARNING: at kernel/softirq.c:137 local_bh_enable_ip+0xd1/0xe0()
> Pid: 1, comm: swapper Not tainted 2.6.26-rc6-tip #2091
>  [<c0125ecf>] warn_on_slowpath+0x4f/0x70
>  [<c0126834>] ? release_console_sem+0x1b4/0x1d0
>  [<c0126d00>] ? vprintk+0x2a0/0x450

Now you can't printk in irq context any more?

vprintk:
        /*
         * Try to acquire and then immediately release the
         * console semaphore. The release will do all the 
         * actual magic (print out buffers, wake up klogd,
         * etc). 
         *
         * The acquire_console_semaphore_for_printk() function
         * will release 'logbuf_lock' regardless of whether it
         * actually gets the semaphore or not.
         */
        if (acquire_console_semaphore_for_printk(this_cpu))
                release_console_sem();


johannes

Attachment: signature.asc
Description: This is a digitally signed message part