Re: [PATCH] xen/events: fix build

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.11.2020 08:27, Jürgen Groß wrote:
> On 11.11.20 08:20, Jan Beulich wrote:
>> On 11.11.2020 06:49, Juergen Gross wrote:
>>> --- a/xen/common/event_channel.c
>>> +++ b/xen/common/event_channel.c
>>> @@ -61,7 +61,9 @@ static inline void evtchn_write_lock(struct evtchn 
>>> *evtchn)
>>>   {
>>>       write_lock(&evtchn->lock);
>>>   
>>> +#ifndef NDEBUG
>>>       evtchn->old_state = evtchn->state;
>>> +#endif
>>>   }
>>>   
>>>   static inline void evtchn_write_unlock(struct evtchn *evtchn)
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> index 7251b3ae3e..95f96e7a33 100644
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -114,9 +114,7 @@ struct evtchn
>>>           u16 virq;      /* state == ECS_VIRQ */
>>>       } u;
>>>       u8 priority;
>>> -#ifndef NDEBUG
>>>       u8 old_state;      /* State when taking lock in write mode. */
>>> -#endif
>>>       u8 last_priority;
>>>       u16 last_vcpu_id;
>>>   #ifdef CONFIG_XSM
>>
>> Did you mean just either of the two changes (and then the former),
>> not both? If so
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> and I'll be happy to drop the other half for committing.
> 
> The header fix is required for NDEBUG builds, while the other one is
> removing a write with no accompanied read for NDEBUG builds.

Oh, that's because of our absurd ASSERT() implementation in the
NDEBUG case. I stand by my position that the field should not be
there in the first place for release builds. Even more so with
the original patch having got re-based ahead of what was patch 1
of the series, which I did not the least because I want that one
backported urgently, while I continue to be hesitant altogether
about the other one.

I guess the course of action is then to put #ifndef NDEBUG
around the ASSERT() itself, however strange this may look. Or
introduce an evtchn_old_state() wrapper, perhaps returning
ECS_RESERVED in the NDEBUG case. I guess it'll be quicker if I
take your patch and massage it before throwing in, than to have
you make a v2.

Janan