From: Tony Lindgren <tony@atomide.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: linux-omap@vger.kernel.org, Kanigeri Hari <h-kanigeri2@ti.com>,
Hiroshi Doyu <Hiroshi.DOYU@nokia.com>
Subject: Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness
Date: Wed, 5 May 2010 08:21:35 -0700 [thread overview]
Message-ID: <20100505152135.GV29604@atomide.com> (raw)
In-Reply-To: <s2nda15981b1005040447v335b0d34k1aa812404f478cd4@mail.gmail.com>
* Ohad Ben-Cohen <ohad@wizery.com> [100504 04:42]:
> On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
> >> Fix reverse likeliness
> >>
> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> >> ---
> >> 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:
Looks good to me.
Hiroshi, care to ack/nak all the mailbox and iommu patches you want
me to merge? Or if you have them in some git branch against mainline
-rc6 that would be cool too.
We need to get the ones we want to merge into linux-arm-kernel for
review soon, I don't think they been posted there yet.
Regards,
Tony
> From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
> From: Ohad Ben-Cohen <ohad@wizery.com>
> 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@wizery.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2010-05-05 15:21 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-02 15:44 [PATCH v2 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
2010-05-02 15:44 ` [PATCH v2 1/4] omap: mailbox: convert rwlocks to spinlock Ohad Ben-Cohen
2010-05-02 15:44 ` [PATCH v2 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line Ohad Ben-Cohen
2010-05-02 15:44 ` [PATCH v2 3/4] omap: mailbox: fix reverse likeliness Ohad Ben-Cohen
2010-05-03 18:02 ` Tony Lindgren
2010-05-04 11:47 ` Ohad Ben-Cohen
2010-05-05 15:21 ` Tony Lindgren [this message]
2010-05-05 15:24 ` Ohad Ben-Cohen
2010-05-06 5:19 ` Hiroshi DOYU
2010-05-02 15:44 ` [PATCH v2 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
2010-05-03 5:30 ` Hiroshi DOYU
2010-05-03 6:07 ` Hiroshi DOYU
2010-05-03 9:41 ` Ohad Ben-Cohen
2010-05-03 10:27 ` [PATCH " Ohad Ben-Cohen
2010-05-03 6:35 ` [PATCH v2 " Ohad Ben-Cohen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100505152135.GV29604@atomide.com \
--to=tony@atomide.com \
--cc=Hiroshi.DOYU@nokia.com \
--cc=h-kanigeri2@ti.com \
--cc=linux-omap@vger.kernel.org \
--cc=ohad@wizery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).