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

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



>>> On 09.02.18 at 11:47, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Feb 09, 2018 at 10:45:25AM +0000, Julien Grall wrote:
>> 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:
> 
> OK, if that's intended I'm fine with it, just wanted to check.

But WARN_ON() should then be given the same property, I think.

Jan

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

 


Rackspace

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