Re: [PATCH v2 3/3] xen: support runqueue steal time on xen

From: Stefano Stabellini <sstabellini@kernel.org>
To: Juergen Gross <jgross@suse.com>
Cc: xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com,
	sstabellini@kernel.org, linux-kernel@vger.kernel.org,
	david.vrabel@citrix.com
Subject: Re: [PATCH v2 3/3] xen: support runqueue steal time on xen
Date: Wed, 6 Jul 2016 11:03:37 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.10.1607061103300.2575__20366.9985676699$1467799495$gmane$org@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <1467781230-12484-4-git-send-email-jgross@suse.com>

On Wed, 6 Jul 2016, Juergen Gross wrote:
> Up to now reading the stolen time of a remote cpu was not possible in a
> performant way under Xen. This made support of runqueue steal time via
> paravirt_steal_rq_enabled impossible.
> 
> With the addition of an appropriate hypervisor interface this is now
> possible, so add the support.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> V2: removed static variable as requested by Stefano Stabellini
> ---
>  drivers/xen/time.c | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/xen/time.c b/drivers/xen/time.c
> index 2257b66..a7fe35b 100644
> --- a/drivers/xen/time.c
> +++ b/drivers/xen/time.c
> @@ -47,27 +47,31 @@ static u64 get64(const u64 *p)
>  	return ret;
>  }
>  
> -/*
> - * Runstate accounting
> - */
> -void xen_get_runstate_snapshot(struct vcpu_runstate_info *res)
> +static void xen_get_runstate_snapshot_cpu(struct vcpu_runstate_info *res,
> +					  unsigned int cpu)
>  {
>  	u64 state_time;
>  	struct vcpu_runstate_info *state;
>  
>  	BUG_ON(preemptible());
>  
> -	state = this_cpu_ptr(&xen_runstate);
> +	state = per_cpu_ptr(&xen_runstate, cpu);
>  
> -	/*
> -	 * The runstate info is always updated by the hypervisor on
> -	 * the current CPU, so there's no need to use anything
> -	 * stronger than a compiler barrier when fetching it.
> -	 */
>  	do {
>  		state_time = get64(&state->state_entry_time);
> +		rmb();	/* Hypervisor might update data. */
>  		*res = READ_ONCE(*state);
> -	} while (get64(&state->state_entry_time) != state_time);
> +		rmb();	/* Hypervisor might update data. */
> +	} while (get64(&state->state_entry_time) != state_time ||
> +		 (state_time & XEN_RUNSTATE_UPDATE));
> +}
> +
> +/*
> + * Runstate accounting
> + */
> +void xen_get_runstate_snapshot(struct vcpu_runstate_info *res)
> +{
> +	xen_get_runstate_snapshot_cpu(res, smp_processor_id());
>  }
>  
>  /* return true when a vcpu could run but has no real cpu to run on */
> @@ -80,8 +84,7 @@ static u64 xen_steal_clock(int cpu)
>  {
>  	struct vcpu_runstate_info state;
>  
> -	BUG_ON(cpu != smp_processor_id());
> -	xen_get_runstate_snapshot(&state);
> +	xen_get_runstate_snapshot_cpu(&state, cpu);
>  	return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
>  }
>  
> @@ -98,11 +101,14 @@ void xen_setup_runstate_info(int cpu)
>  
>  void __init xen_time_setup_guest(void)
>  {
> +	bool xen_runstate_remote;
> +
> +	xen_runstate_remote = !HYPERVISOR_vm_assist(VMASST_CMD_enable,
> +					VMASST_TYPE_runstate_update_flag);
> +
>  	pv_time_ops.steal_clock = xen_steal_clock;
>  
>  	static_key_slow_inc(&paravirt_steal_enabled);
> -	/*
> -	 * We can't set paravirt_steal_rq_enabled as this would require the
> -	 * capability to read another cpu's runstate info.
> -	 */
> +	if (xen_runstate_remote)
> +		static_key_slow_inc(&paravirt_steal_rq_enabled);
>  }
> -- 
> 2.6.6
> 

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