Fernando, Thanks for looking at the patch.
On Tue, Jul 20, 2010 at 4:59 PM, Guzman Lugo, Fernando <[email protected]> wrote: > > > Hi Hari, > >> >> @@ -252,28 +252,30 @@ static int omap_mbox_startup(struct omap_mbox *mbox) >> } >> } >> >> - ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, >> - mbox->name, mbox); >> - if (unlikely(ret)) { >> - printk(KERN_ERR >> - "failed to register mailbox interrupt:%d\n", ret); >> - goto fail_request_irq; >> - } >> + if (atomic_inc_return(&mbox->use_count) == 1) { > > What happen if a thread is preempted by other thread which also uses the same > mailbox after doing the atomic_inc? > > The second thread also will call atomic_inc_return and in this case the value > returned will be 2 and it will not initialize the mbox queues but it will > return success status, in this point the second thread will think it could > get the mailbox handle without any issue. However the first thread still can > fail and not initialize correctly the mailbox leading second thread to not > work properly or crash. > > I think all the initialization should be protected and not just the use_count. > -- How about changing mboxes_lock from spinlock to mutex and protecting the initialization code ? I guess then the variables don't have to be atomic too. please share your thougts. > Please let me know what you think. > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
