From: "ext Guzman Lugo, Fernando" <fernando.lugo@xxxxxx> Subject: RE: [PATCH v3 4/4] omap: mailbox: convert block api to kfifo Date: Tue, 8 Jun 2010 01:14:41 +0200 > > > Hi, > >> -----Original Message----- >> From: Ohad Ben-Cohen [mailto:ohad@xxxxxxxxxx] >> Sent: Monday, June 07, 2010 4:41 PM >> To: Chitriki Rudramuni, Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar; >> Kanigeri, Hari >> Cc: linux-omap@xxxxxxxxxxxxxxx; Hiroshi Doyu >> Subject: Re: [PATCH v3 4/4] omap: mailbox: convert block api to kfifo >> >> Hi Deepak, >> >> On Mon, Jun 7, 2010 at 1:52 PM, Deepak Chitriki <deepak.chitriki@xxxxxx> >> 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: > > I think the best thing to do here is remove the spinlock, if not, > you are preventing that omap_mbox_msg_send be executed from a > tasklet or isr context. That maybe in this moment it is not called > but it could be needed in the future. The caller of > omap_mbox_msg_send should be take care of that function can not be > call simultaneously. What about adding another function, omap_mbox_msg_send_sync()? I don't want to limit the ability of having multiple senders. The above would send the message with bypassing tasklet. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
