Fernando,
Thanks for looking at the patch.
On Tue, Jul 20, 2010 at 4:59 PM, Guzman Lugo, Fernando
<fernando.lugo@xxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
