Re: [PATCH 1/7] Port WARN_ON_ONCE() from Linux

From: Julien Grall <julien.grall@arm.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>,
	"Sameer Goel" <sameer.goel@linaro.org>
Cc: sstabellini@kernel.org, wei.liu2@citrix.com,
	mjaggi@caviumnetworks.com, george.dunlap@eu.citrix.com,
	Andrew.Cooper3@citrix.com, jbeulich@suse.com,
	Ian.Jackson@citrix.com, xen-devel@lists.xenproject.org,
	nd@arm.com, shankerd@codeaurora.org
Subject: Re: [PATCH 1/7] Port WARN_ON_ONCE() from Linux
Date: Fri, 9 Feb 2018 10:45:25 +0000	[thread overview]
Message-ID: <6c94284e-f559-273d-021e-05ea35b9ec2e@arm.com> (raw)
In-Reply-To: <20180209102947.6dwuchlgkibx4okr@MacBook-Pro-de-Roger.local>

Hi,

On 02/09/2018 10:29 AM, Roger Pau Monné wrote:
> On Thu, Feb 08, 2018 at 08:10:49PM -0700, Sameer Goel wrote:
>> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
>> index 1d9771340c..697212a061 100644
>> --- a/xen/include/xen/lib.h
>> +++ b/xen/include/xen/lib.h
>> @@ -11,6 +11,19 @@
>>   #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);                    \
>         ^ bool
> 
>> +                                                    \
>> +    if ( unlikely(__ret_warn_once && !__warned) )     \
>> +    {                                               \
>> +        __warned = true;                            \
>> +        WARN();                                     \
>> +    }                                               \
>> +    unlikely(__ret_warn_once);                      \
> 
> Does this macro really need to return something? It seems weird to me
> to allow usages like: if ( WARN_ON_ONCE...

This construct is used in Linux (included in the driver ported):

if (WARN_ON_ONCE(fwspec->iommu_priv)) {
      master = fwspec->iommu_priv;
      smmu = master->smmu;
} else {
....
}

IHMO the makes the code nicer to read over:

WARN_ON_ONCE(...)
if ( ... ) {
} else {
}

Cheers,

-- 
Julien Grall

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