From: George Dunlap <george.dunlap@citrix.com>
To: Juergen Gross <jgross@suse.com>,
keir@xen.org, jbeulich@suse.com, andrew.cooper3@citrix.com,
dario.faggioli@citrix.com, george.dunlap@eu.citrix.com,
xen-devel@lists.xen.org
Subject: Re: [PATCH 4/5] xen: use masking operation instead of test_bit for VPF bits
Date: Mon, 5 Oct 2015 14:24:19 +0100 [thread overview]
Message-ID: <56127A03.9080700@citrix.com> (raw)
In-Reply-To: <1443760830-29095-5-git-send-email-jgross@suse.com>
On 02/10/15 05:40, Juergen Gross wrote:
> Use a bit mask for testing of a set bit instead of test_bit in case no
> atomic operation is needed, as this will lead to smaller and more
> effective code.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
On a side note, a handful of functions seem to access the vcpu structure
with the *domain* lock held, rather than the *scheduler* lock; for example:
xen/arch/x86/hvm/vlapic.c:vlapic_accept_irq()
xen/common/domain.c:vcpu_reset()
Is this a bug (perhaps left over from when we used the domain lock for
vcpus rather than the scheduler lock)?
-George
> ---
> xen/arch/x86/domctl.c | 2 +-
> xen/arch/x86/hvm/hvm.c | 4 ++--
> xen/arch/x86/hvm/vpt.c | 2 +-
> xen/common/domain.c | 4 ++--
> xen/common/domctl.c | 8 ++++----
> xen/common/schedule.c | 16 ++++++++--------
> 6 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 6172c0d..f8a559c 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1213,7 +1213,7 @@ void arch_get_info_guest(struct vcpu *v, vcpu_guest_context_u c)
> c(flags = v->arch.vgc_flags & ~(VGCF_i387_valid|VGCF_in_kernel));
> if ( v->fpu_initialised )
> c(flags |= VGCF_i387_valid);
> - if ( !test_bit(_VPF_down, &v->pause_flags) )
> + if ( !(v->pause_flags & VPF_down) )
> c(flags |= VGCF_online);
> if ( !compat )
> {
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 6afc344..3fa2280 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1728,7 +1728,7 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> {
> /* We don't need to save state for a vcpu that is down; the restore
> * code will leave it down if there is nothing saved. */
> - if ( test_bit(_VPF_down, &v->pause_flags) )
> + if ( v->pause_flags & VPF_down )
> continue;
>
> /* Architecture-specific vmcs/vmcb bits */
> @@ -2512,7 +2512,7 @@ void hvm_vcpu_down(struct vcpu *v)
> /* Any other VCPUs online? ... */
> domain_lock(d);
> for_each_vcpu ( d, v )
> - if ( !test_bit(_VPF_down, &v->pause_flags) )
> + if ( !(v->pause_flags & VPF_down) )
> online_count++;
> domain_unlock(d);
>
> diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
> index 0c8b22e..4fa6566 100644
> --- a/xen/arch/x86/hvm/vpt.c
> +++ b/xen/arch/x86/hvm/vpt.c
> @@ -178,7 +178,7 @@ void pt_save_timer(struct vcpu *v)
> struct list_head *head = &v->arch.hvm_vcpu.tm_list;
> struct periodic_time *pt;
>
> - if ( test_bit(_VPF_blocked, &v->pause_flags) )
> + if ( v->pause_flags & VPF_blocked )
> return;
>
> spin_lock(&v->arch.hvm_vcpu.tm_lock);
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index cda60a9..7c362eb 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1135,7 +1135,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
> return -EINVAL;
>
> /* Run this command on yourself or on other offline VCPUS. */
> - if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )
> + if ( (v != current) && !(v->pause_flags & VPF_down) )
> return -EINVAL;
>
> page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
> @@ -1263,7 +1263,7 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
> break;
>
> case VCPUOP_is_up:
> - rc = !test_bit(_VPF_down, &v->pause_flags);
> + rc = !(v->pause_flags & VPF_down);
> break;
>
> case VCPUOP_get_runstate_info:
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 08de32d..46b967e 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -170,7 +170,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
> vcpu_runstate_get(v, &runstate);
> cpu_time += runstate.time[RUNSTATE_running];
> info->max_vcpu_id = v->vcpu_id;
> - if ( !test_bit(_VPF_down, &v->pause_flags) )
> + if ( !(v->pause_flags & VPF_down) )
> {
> if ( !(v->pause_flags & VPF_blocked) )
> flags &= ~XEN_DOMINF_blocked;
> @@ -231,7 +231,7 @@ static unsigned int default_vcpu0_location(cpumask_t *online)
> rcu_read_lock(&domlist_read_lock);
> for_each_domain ( d )
> for_each_vcpu ( d, v )
> - if ( !test_bit(_VPF_down, &v->pause_flags)
> + if ( !(v->pause_flags & VPF_down)
> && ((cpu = v->processor) < nr_cpus) )
> cnt[cpu]++;
> rcu_read_unlock(&domlist_read_lock);
> @@ -944,8 +944,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>
> vcpu_runstate_get(v, &runstate);
>
> - op->u.getvcpuinfo.online = !test_bit(_VPF_down, &v->pause_flags);
> - op->u.getvcpuinfo.blocked = test_bit(_VPF_blocked, &v->pause_flags);
> + op->u.getvcpuinfo.online = !(v->pause_flags & VPF_down);
> + op->u.getvcpuinfo.blocked = !!(v->pause_flags & VPF_blocked);
> op->u.getvcpuinfo.running = v->is_running;
> op->u.getvcpuinfo.cpu_time = runstate.time[RUNSTATE_running];
> op->u.getvcpuinfo.cpu = v->processor;
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 5ffa1a1..c5f640f 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -119,7 +119,7 @@ static inline void vcpu_urgent_count_update(struct vcpu *v)
>
> if ( unlikely(v->is_urgent) )
> {
> - if ( !test_bit(_VPF_blocked, &v->pause_flags) ||
> + if ( !(v->pause_flags & VPF_blocked) ||
> !test_bit(v->vcpu_id, v->domain->poll_mask) )
> {
> v->is_urgent = 0;
> @@ -128,8 +128,8 @@ static inline void vcpu_urgent_count_update(struct vcpu *v)
> }
> else
> {
> - if ( unlikely(test_bit(_VPF_blocked, &v->pause_flags) &&
> - test_bit(v->vcpu_id, v->domain->poll_mask)) )
> + if ( unlikely(v->pause_flags & VPF_blocked) &&
> + unlikely(test_bit(v->vcpu_id, v->domain->poll_mask)) )
> {
> v->is_urgent = 1;
> atomic_inc(&per_cpu(schedule_data,v->processor).urgent_count);
> @@ -418,7 +418,7 @@ void vcpu_wake(struct vcpu *v)
> vcpu_runstate_change(v, RUNSTATE_runnable, NOW());
> SCHED_OP(VCPU2OP(v), wake, v);
> }
> - else if ( !test_bit(_VPF_blocked, &v->pause_flags) )
> + else if ( !(v->pause_flags & VPF_blocked) )
> {
> if ( v->runstate.state == RUNSTATE_blocked )
> vcpu_runstate_change(v, RUNSTATE_offline, NOW());
> @@ -595,7 +595,7 @@ void vcpu_force_reschedule(struct vcpu *v)
> set_bit(_VPF_migrating, &v->pause_flags);
> vcpu_schedule_unlock_irq(lock, v);
>
> - if ( test_bit(_VPF_migrating, &v->pause_flags) )
> + if ( v->pause_flags & VPF_migrating )
> {
> vcpu_sleep_nosync(v);
> vcpu_migrate(v);
> @@ -763,7 +763,7 @@ static int vcpu_set_affinity(
>
> domain_update_node_affinity(v->domain);
>
> - if ( test_bit(_VPF_migrating, &v->pause_flags) )
> + if ( v->pause_flags & VPF_migrating )
> {
> vcpu_sleep_nosync(v);
> vcpu_migrate(v);
> @@ -1285,7 +1285,7 @@ static void schedule(void)
>
> vcpu_runstate_change(
> prev,
> - (test_bit(_VPF_blocked, &prev->pause_flags) ? RUNSTATE_blocked :
> + ((prev->pause_flags & VPF_blocked) ? RUNSTATE_blocked :
> (vcpu_runnable(prev) ? RUNSTATE_runnable : RUNSTATE_offline)),
> now);
> prev->last_run_time = now;
> @@ -1327,7 +1327,7 @@ void context_saved(struct vcpu *prev)
>
> SCHED_OP(VCPU2OP(prev), context_saved, prev);
>
> - if ( unlikely(test_bit(_VPF_migrating, &prev->pause_flags)) )
> + if ( unlikely(prev->pause_flags & VPF_migrating) )
> vcpu_migrate(prev);
> }
>
>
next prev parent reply other threads:[~2015-10-05 13:24 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-02 4:40 [PATCH 0/5] use mask operations instead of test_bit() Juergen Gross
2015-10-02 4:40 ` [PATCH 1/5] xen: use masking operation instead of test_bit for RTDS bits Juergen Gross
2015-10-02 10:21 ` Dario Faggioli
2015-10-05 13:30 ` George Dunlap
2015-10-02 4:40 ` [PATCH 2/5] xen: use masking operation instead of test_bit for CSFLAG bits Juergen Gross
2015-10-02 10:45 ` Dario Faggioli
2015-10-05 13:30 ` George Dunlap
2015-10-02 4:40 ` [PATCH 3/5] xen: use masking operation instead of test_bit for VGCF bits Juergen Gross
2015-10-02 4:40 ` [PATCH 4/5] xen: use masking operation instead of test_bit for VPF bits Juergen Gross
2015-10-05 13:18 ` George Dunlap
2015-10-05 13:36 ` Jan Beulich
2015-10-05 13:45 ` George Dunlap
2015-10-05 14:05 ` Jan Beulich
2015-10-05 14:31 ` George Dunlap
2015-10-05 13:39 ` Juergen Gross
2015-10-05 13:24 ` George Dunlap [this message]
2015-10-05 13:40 ` Jan Beulich
2015-10-02 4:40 ` [PATCH 5/5] xen: use masking operation instead of test_bit for MCSF bits Juergen Gross
2015-10-02 9:03 ` [PATCH 0/5] use mask operations instead of test_bit() Dario Faggioli
2015-10-02 9:10 ` Juergen Gross
2015-10-02 9:33 ` Dario Faggioli
2015-10-02 9:44 ` Jan Beulich
2015-10-02 9:47 ` Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56127A03.9080700@citrix.com \
--to=george.dunlap@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=dario.faggioli@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=jgross@suse.com \
--cc=keir@xen.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).