Re: [PATCH 1/7] MIPS: add unlikely() to BUG_ON()

[Coly Li]

On 2011å01æ28æ 01:50, David Daney Wrote:
> Please Cc: linux-mips@xxxxxxxxxxxxxx for MIPS patches.
>
> On 01/27/2011 04:12 AM, Coly Li wrote:
> > Current BUG_ON() in arch/mips/include/asm/bug.h does not use unlikely(),
> > in order to get better branch predict result, source code should call
> > BUG_ON() with unlikely() explicitly. This is not a suggested method to
> > use BUG_ON().
> >
> > This patch adds unlikely() inside BUG_ON implementation on MIPS code,
> > callers can use BUG_ON without explicit unlikely() now.
> >
> > I have no usable MIPS hardware to build and test the fix, any test result
> > of this patch is welcome.
> >
> > Signed-off-by: Coly Li<bosong.ly@xxxxxxxxxx>
> > Cc: David Daney<ddaney@xxxxxxxxxxxxxxxxxx>
> > Cc: Wang Cong<xiyou.wangcong@xxxxxxxxx>
> > Cc: Yong Zhang<yong.zhang0@xxxxxxxxx>
> > ---
> > arch/mips/include/asm/bug.h | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/mips/include/asm/bug.h b/arch/mips/include/asm/bug.h
> > index 540c98a..6771c07 100644
> > --- a/arch/mips/include/asm/bug.h
> > +++ b/arch/mips/include/asm/bug.h
> > @@ -30,7 +30,7 @@ static inline void __BUG_ON(unsigned long condition)
> > : : "r" (condition), "i" (BRK_BUG));
> > }
> >
> > -#define BUG_ON(C) __BUG_ON((unsigned long)(C))
> > +#define BUG_ON(C) __BUG_ON((unsigned long)unlikely(C))
>
> NAK.
>
> __BUG_ON() expands to a single instruction. Frobbing about with unlikely() will have no effect on the generated code and
> is thus gratuitous code churn.

Since unlikely() in arch implemented BUG_ON() is gratuitous, using unlikely() in kernel code like BUG_ON(unlikely(...)) 
for arch implemented BUG_ON is unwelcome neither.

The NAK makes sense, thanks for your reply.

Coly

--
Coly Li
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/