Re: [PATCH 1/2] xen/lib: Introduce printk_once() and replace some opencoded examples

["Jan Beulich" <JBeulich@suse.com>]

>>> On 17.05.19 at 20:58, <andrew.cooper3@citrix.com> wrote:
> Reflow the ZynqMP message for grepability, and fix the omission of a newline.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two remarks:

> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -105,6 +105,17 @@ debugtrace_printk(const char *fmt, ...) {}
>  #define _p(_x) ((void *)(unsigned long)(_x))
>  extern void printk(const char *format, ...)
>      __attribute__ ((format (printf, 1, 2)));
> +
> +#define printk_once(fmt, args...)               \
> +({                                              \
> +    static bool __read_mostly once_;            \
> +    if ( unlikely(!once_) )                     \
> +    {                                           \
> +        once_ = true;                           \
> +        printk(fmt, ## args);                   \
> +    }                                           \
> +})

Just like Linux we accept this having a small race window, i.e.
it not truly being "once" in unlikely cases. I think it would be
worthwhile to make this intention explicit in the commit
message.

Unlike Linux'es the macro doesn't have a "return value". Is
this intentional? I view this as particularly useful for the
WARN_ONCE() sort-of counterpart, but I have to admit I can't
immediately see a good use here, so I'm largely curious.
(The 2 uses I could find in x86-specific code in Linux look
to me like they'd better be WARN_ONCE().)

Jan



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