[Xen-devel,RFC,v4,1/8] Port WARN_ON_ONCE() from Linux

Message ID 20171219031703.23420-2-sameer.goel@linaro.org
State New
Headers show
Series
  • SMMUv3 driver
Related show

Commit Message

Sameer Goel Dec. 19, 2017, 3:16 a.m.
Port WARN_ON_ONCE macro from Linux.

Signed-off-by: Sameer Goel <sameer.goel@linaro.org>
---
 xen/include/xen/lib.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Julien Grall Jan. 23, 2018, 11:33 a.m. | #1
Hi Sameer,

On 19/12/17 03:16, Sameer Goel wrote:
> Port WARN_ON_ONCE macro from Linux.
> 
> Signed-off-by: Sameer Goel <sameer.goel@linaro.org>
> ---
>   xen/include/xen/lib.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index ed00ae1379..83206c0848 100644
> --- 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; \
> +    int __ret_warn_once = !!(p);                    \
> +                                                    \
> +    if (unlikely(__ret_warn_once && !__warned)) {   \

I realise BUG_ON/WARN_ON above is not using Xen coding style. However, 
can you make this code follow Xen coding style?

FWIW with the coding style change:

Acked-by: Julien Grall <julien.grall@linaro.org>

This will also require an ack from "THE REST" maintainers as this is 
common code.

Cheers,

> +        __warned = true;                            \
> +        WARN_ON(1);                                 \
> +    }                                               \
> +    unlikely(__ret_warn_once);                      \
> +})
> +
>   #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>   /* Force a compilation error if condition is true */
>   #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
>
Wei Liu Jan. 23, 2018, 4:13 p.m. | #2
On Mon, Dec 18, 2017 at 08:16:56PM -0700, Sameer Goel wrote:
> Port WARN_ON_ONCE macro from Linux.
> 
> Signed-off-by: Sameer Goel <sameer.goel@linaro.org>
> ---
>  xen/include/xen/lib.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index ed00ae1379..83206c0848 100644
> --- 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; \

You introduced a new section without corresponding changes to
{arm,x86}/xen.lds.S

> +    int __ret_warn_once = !!(p);                    \
> +                                                    \
> +    if (unlikely(__ret_warn_once && !__warned)) {   \
> +        __warned = true;                            \
> +        WARN_ON(1);                                 \

This isn't really a direct port from Linux. At this point I wonder why
you didn't use WARN() directly?

Wei.

> +    }                                               \
> +    unlikely(__ret_warn_once);                      \
> +})
> +
>  #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>  /* Force a compilation error if condition is true */
>  #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
> -- 
> 2.14.1
>
Sameer Goel Jan. 26, 2018, 1:45 a.m. | #3
On 1/23/2018 9:13 AM, Wei Liu wrote:
> On Mon, Dec 18, 2017 at 08:16:56PM -0700, Sameer Goel wrote:
>> Port WARN_ON_ONCE macro from Linux.
>>
>> Signed-off-by: Sameer Goel <sameer.goel@linaro.org>
>> ---
>>  xen/include/xen/lib.h | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
>> index ed00ae1379..83206c0848 100644
>> --- 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; \
> You introduced a new section without corresponding changes to
> {arm,x86}/xen.lds.S
I will fix this.
>
>> +    int __ret_warn_once = !!(p);                    \
>> +                                                    \
>> +    if (unlikely(__ret_warn_once && !__warned)) {   \
>> +        __warned = true;                            \
>> +        WARN_ON(1);                                 \
> This isn't really a direct port from Linux. At this point I wonder why
> you didn't use WARN() directly?
>
> Wei.
>
>> +    }                                               \
>> +    unlikely(__ret_warn_once);                      \
>> +})
>> +
>>  #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>>  /* Force a compilation error if condition is true */
>>  #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
>> -- 
>> 2.14.1
>>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

Patch

diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index ed00ae1379..83206c0848 100644
--- 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; \
+    int __ret_warn_once = !!(p);                    \
+                                                    \
+    if (unlikely(__ret_warn_once && !__warned)) {   \
+        __warned = true;                            \
+        WARN_ON(1);                                 \
+    }                                               \
+    unlikely(__ret_warn_once);                      \
+})
+
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
 /* Force a compilation error if condition is true */
 #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })