Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Ohad Ben-Cohen <ohad@xxxxxxxxxx>]

On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> * Ohad Ben-Cohen <ohad@xxxxxxxxxx> [100502 08:40]:
>> Fix reverse likeliness
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@xxxxxxxxxx>
>> ---
>> If you want, you can also reach me at < ohadb at ti dot com >.
>>
>>  arch/arm/plat-omap/mailbox.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index 5140efc..5309213 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>>   fail_alloc_txq:
>>       free_irq(mbox->irq, mbox);
>>   fail_request_irq:
>> -     if (unlikely(mbox->ops->shutdown))
>> +     if (likely(mbox->ops->shutdown))
>>               mbox->ops->shutdown(mbox);
>>
>>       return ret;
>> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>>
>>       free_irq(mbox->irq, mbox);
>>
>> -     if (unlikely(mbox->ops->shutdown)) {
>> +     if (likely(mbox->ops->shutdown)) {
>>               spin_lock(&mboxes_lock);
>>               if (mbox_configured > 0)
>>                       mbox_configured--;
>
> Does this code path need to be optimized? :)
>
> How about just get rid of the (un)likely here?

I like this :)

If we're at it, there are additional cold-path (un)likely macros I
want to target:

>From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen <ohad@xxxxxxxxxx>
Date: Sat, 17 Apr 2010 01:57:43 +0300
Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths

Signed-off-by: Ohad Ben-Cohen <ohad@xxxxxxxxxx>
---
 arch/arm/plat-omap/mailbox.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..7c60550 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
        int ret = 0;
        struct omap_mbox_queue *mq;

-       if (likely(mbox->ops->startup)) {
+       if (mbox->ops->startup) {
                spin_lock(&mboxes_lock);
                if (!mbox_configured)
                        ret = mbox->ops->startup(mbox);

-               if (unlikely(ret)) {
+               if (ret) {
                        spin_unlock(&mboxes_lock);
                        return ret;
                }
@@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)

        ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
                                mbox->name, mbox);
-       if (unlikely(ret)) {
+       if (ret) {
                printk(KERN_ERR
                        "failed to register mailbox interrupt:%d\n", ret);
                goto fail_request_irq;
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
  fail_alloc_txq:
        free_irq(mbox->irq, mbox);
  fail_request_irq:
-       if (unlikely(mbox->ops->shutdown))
+       if (mbox->ops->shutdown)
                mbox->ops->shutdown(mbox);

        return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)

        free_irq(mbox->irq, mbox);

-       if (unlikely(mbox->ops->shutdown)) {
+       if (mbox->ops->shutdown) {
                spin_lock(&mboxes_lock);
                if (mbox_configured > 0)
                        mbox_configured--;
-- 
1.6.3.3

I'll wait a day or two for more comments, and send a v3 series.

Thanks,
Ohad.

>
> Regards,
>
> Tony
>
--
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