Re: [Xen-devel] [PATCH 5/5] xen: clean up VPF flags macros

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.15 at 07:23, <JGross@xxxxxxxx> wrote:
> On 09/25/2015 05:42 PM, Jan Beulich wrote:
>>>>> On 25.09.15 at 13:54, <JGross@xxxxxxxx> wrote:
>>> Per-VCPU pause flags in sched.h are defined as bit positions and as
>>> values derived from the bit defines. There is only one user of a value
>>> which can be easily converted to use a bit number as well.
>>
>> I'm not convinced:
>>
>>> --- a/xen/common/domctl.c
>>> +++ b/xen/common/domctl.c
>>> @@ -172,7 +172,7 @@ void getdomaininfo(struct domain *d, struct 
> xen_domctl_getdomaininfo *info)
>>>           info->max_vcpu_id = v->vcpu_id;
>>>           if ( !test_bit(_VPF_down, &v->pause_flags) )
>>>           {
>>> -            if ( !(v->pause_flags & VPF_blocked) )
>>> +            if ( !test_bit(_VPF_blocked, &v->pause_flags) )
>>
>> test_bit() is quite a bit more complex an operation than a simple &,
>> and with (on x86) even constant_test_bit() involving a cast to
>> a pointer to volatile I'm afraid we can't even hope that compilers
>> would produce identical code for both in cases like this one (as that
>> casts limits freedom of the compiler). IOW I'd rather see other
>> test_bit(_VPF_...) uses converted the inverse way (which as a nice
>> but minor side effect would yield slightly smaller source code).
> 
> What about introducing __test_bit() being a variant which can be
> reordered by omitting the volatile modifier? I think this would have
> the same effect.

I'm not convinced it always would - the inline function is still more
complex than the plain operation.

> And we could still get rid of many double definitions
> of the same bit. Even if the mask definition of a bit is not error prone
> by relying on the definition of the bit position, it makes it harder to
> find all users of this bit.

Why so? Just omit the leading underscore when grep-ing, and you'll
find all instances (less preprocessor token concatenation, but that's
orthogonal).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel