Re: [Xen-devel] [RFC v3 1/4] Port WARN_ON_ONCE() from Linux

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.17 at 04:59, <sgoel@xxxxxxxxxxxxxx> wrote:
> Port WARN_ON_ONCE macro from Linux. A return value is expected from this
> macro, so the implementation  does not follow the Xen convention of wrapping
> macros in a do..while.

There's no such convention for macros producing a value.

> ---

Missing S-o-b.

> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -11,6 +11,17 @@
>  #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_ONCE(p)      ({                          \
> +    static bool __section(".data.unlikely") __warned; \

I think this will need an addition to xen.lds.S (both x86 and ARM).

> +    int __ret_warn_once = !!(p);                    \

bool and please don't use leading underscores for identifiers when
those are in conflict with the C spec.

> +    if (unlikely(__ret_warn_once && !__warned)) {   \

I don't think using likely() / unlikely() on expressions involving && or
|| is ever a useful thing - in the case here you really mean

    if (unlikely(__ret_warn_once) && unlikely(!__warned)) {

> +        __warned = true;                            \
> +        WARN_ON(1);                                 \

WARN()

I also think that you would better use Xen style here, despite
BUG_ON() and WARN_ON() themselves slightly violating this. The
file clearly is not a Linux derived file (anymore).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel