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