Re: [PATCH v3 4/4] omap: mailbox: convert block api to kfifo

[Deepak Chitriki <deepak.chitriki@ti.com>]

Ohad Ben-Cohen wrote:
> Hi Deepak,
>
> On Mon, Jun 7, 2010 at 1:52 PM, Deepak Chitriki <deepak.chitriki@ti.com> wrote:
>   
>> With this patch I observed "inconsistent lock state" warning.
>>     
>
> Thanks for the report!
>
>   
>> Kfifo is acccessed in omap_mbox_msg_send() and mbox_tx_tasklet()
>> functions.In order to protect this critical section we need to protect by
>> using spin_lock_bh() so that the tasklet cannot preempt
>> omap_mobx_msg_send().
>>     
>
> This is actually not the problem: it's ok if mbox_tx_tasklet preempts
> omap_mbox_msg_send. In fact, such a use case is even ok if we don't
> take a spinlock at all: kfifo is designed in a way that if you have
> only 1 consumer and 1 producer, they can both access kfifo
> simultaneously without any locking. That's why we don't take the
> spinlock in the mbox_tx_tasklet. The reason, btw, that we take a
> spinlock in omap_mbox_msg_send is to allow multiple simultaneous
> sending contexts (taking a spinlock there ensures serialization of
> those multiple simultaneous sending contexts).
>
> The problem here lies in the fact (that I've just learnt) that
> dspbridge also sends mbox messages from a tasklet context
> (dpc_tasklet). Lockdep identifies that the spinlock is taken in a
> softirq context (dspbridge's dpc_tasklet) after it was previously
> taken in a softirq-enabled context. Your proposed fix will solve the
> lockdep issue here, but:
>
> Do we really want to have tasklets in dspbridge ? Are we that
> performance critical ?
>
> In general I'd prefer to switch to workqueues in both dspbridge and
> mailbox. I'm really not convinced we have to use tasklets in those
> modules, and workqueues are much more system friendly. This way we
> would also not have to stop all bottom halves when sending a mailbox
> message.
>
> Somewhat relevant note about mailbox performance: omap_mbox_msg_send
> often (i.e. when the kfifo is empty) can just send the message
> directly, without triggering the tasklet to do that. Applying such a
> change will substantially improve the mailbox performance, and will
> make the shift to workqueues even more reasonable. I've got a patch
> for that, I'll post it soon.
>   
tasklet is run in atomic context,so I wonder how mailbox performance 
will increase if you switch from tasklet to workqueue?

> What do you think (looping in Fernando, Omar and Hari) ?
>
> Thanks,
> Ohad.
>