[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen: Rework WARN_ON() to return whether a warning was triggered



On Tue, 15 Dec 2020, Jan Beulich wrote:
> On 15.12.2020 14:19, Julien Grall wrote:
> > On 15/12/2020 11:46, Jan Beulich wrote:
> >> On 15.12.2020 12:26, Julien Grall wrote:
> >>> --- a/xen/include/xen/lib.h
> >>> +++ b/xen/include/xen/lib.h
> >>> @@ -23,7 +23,13 @@
> >>>   #include <asm/bug.h>
> >>>   
> >>>   #define BUG_ON(p)  do { if (unlikely(p)) BUG();  } while (0)
> >>> -#define WARN_ON(p) do { if (unlikely(p)) WARN(); } while (0)
> >>> +#define WARN_ON(p)  ({                  \
> >>> +    bool __ret_warn_on = (p);           \
> >>
> >> Please can you avoid leading underscores here?
> > 
> > I can.
> > 
> >>
> >>> +                                        \
> >>> +    if ( unlikely(__ret_warn_on) )      \
> >>> +        WARN();                         \
> >>> +    unlikely(__ret_warn_on);            \
> >>> +})
> >>
> >> Is this latter unlikely() having any effect? So far I thought it
> >> would need to be immediately inside a control construct or be an
> >> operand to && or ||.
> > 
> > The unlikely() is directly taken from the Linux implementation.
> > 
> > My guess is the compiler is still able to use the information for the 
> > branch prediction in the case of:
> > 
> > if ( WARN_ON(...) )
> 
> Maybe. Or maybe not. I don't suppose the Linux commit introducing
> it clarifies this?

I did a bit of digging but it looks like the unlikely has been there
forever. I'd just keep it as is.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.