Re: [PATCH] Bluetooth: Incorrect locking when sending data in softirq

[Alexander Aring <alex.aring@gmail.com>]

Hi Peter,

On Wed, Oct 15, 2014 at 05:58:40PM -0400, Peter Hurley wrote:
> On 10/15/2014 05:53 PM, Alexander Aring wrote:
> > Hi Peter,
> > 
> > On Wed, Oct 15, 2014 at 04:46:24PM -0400, Peter Hurley wrote:
> > ...
> >>
> >> That's happening because 6lowpan.c:send_mcast_pkt() is disabling
> >> interrupts with the read_lock_irqsave() before calling send_pkt().
> >>
> >> It's unclear browsing through the lowpan driver why the
> >> irqflags save/restore read_lock flavors are being used; is there a
> >> place where the bluetooth core is calling the driver in atomic
> >> context (ie., where interrupts are disabled)?
> >>
> > 
> > In my opinion bt_xmit is called in atomic context. Make the stacktrace
> > sense now?
> 
> I don't think so.
> 
> The network layer is very careful about keeping interrupts enabled
> and calling network drivers from softirq, so that spin_lock_bh() is
> typically all that's required.
> 
> > It's the callback 'ndo_start_xmit' of 'struct net_device_ops' [0].
> 
> The send_mcast_pkt() isn't showing in the stack trace because it's
> being inlined; and send_mcast_pkt() is definitely disabling interrupts
> and calling send_pkt().
> 

Thanks for explanation, now I know a little more about different context
and handling of them. I need to say I am not an expert into this. :-)


Nevertheless, I found something in "Documentation/networking/netdevices.txt"
about context information for 'ndo_start_xmit':

<qoute>
...
Context: Process with BHs disabled or BH (timer),
         will be called with interrupts disabled by netconsole.
...
</qoute>

Now I am not sure if this helps us and what exactly this means. But "...
interrupts disabled..." is this atomic now?

- Alex