From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 5/5] xen: clean up VPF flags macros Date: Mon, 28 Sep 2015 07:23:13 +0200 Message-ID: <5608CEC1.1080207@suse.com> References: <1443182072-15321-1-git-send-email-jgross@suse.com> <1443182072-15321-6-git-send-email-jgross@suse.com> <5605878D02000078000A5BA1@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5605878D02000078000A5BA1@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: tim@xen.org, wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, david.vrabel@citrix.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 09/25/2015 05:42 PM, Jan Beulich wrote: >>>> On 25.09.15 at 13:54, 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. 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. Juergen