All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Kelley <mikelley@microsoft.com>
To: Yi Sun <yi.y.sun@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"jgross@suse.com" <jgross@suse.com>,
	"chao.p.peng@intel.com" <chao.p.peng@intel.com>,
	"chao.gao@intel.com" <chao.gao@intel.com>,
	"isaku.yamahata@intel.com" <isaku.yamahata@intel.com>,
	Tianyu Lan <Tianyu.Lan@microsoft.com>,
	KY Srinivasan <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Stephen Hemminger <sthemmin@microsoft.com>
Subject: RE: [PATCH v1 2/2] x86/hyperv: make HvNotifyLongSpinWait hypercall
Date: Wed, 24 Oct 2018 16:53:00 +0000	[thread overview]
Message-ID: <CY4PR21MB07737AA8E3F7E7353D42F5C8D7F60@CY4PR21MB0773.namprd21.prod.outlook.com> (raw)
In-Reply-To: <1539954835-34035-3-git-send-email-yi.y.sun@linux.intel.com>

From: Yi Sun <yi.y.sun@linux.intel.com>  Sent: Friday, October 19, 2018 6:14 AM
> 
> The HvNotifyLongSpinWait hypercall (HVCALL_NOTIFY_LONG_SPIN_WAIT)
> is used by a guest OS to notify the hypervisor that the calling
> virtual processor is attempting to acquire a resource that is
> potentially held by another virtual processor within the same
> Virtual Machine. This scheduling hint improves the scalability of
> VMs with more than one virtual processor on Hyper-V.
> 
> Per MSFT TLFS, the retry number (SpinWaitInfo) is sent to hypervisor
> only when the retry number exceeds the recommended number. If
> recommended number is 0xFFFFFFFF, never retry.

The HvNotifyLongSpinWait hypercall should be understood to be
advisory only.  As you noted, it is a scheduling hint to the
hypervisor that some virtual CPU in the VM holds a spin lock.  Even
though Linux knows which vCPU holds the spin lock, the hypercall
does not provide a way to give that information to Hyper-V.  The
hypercall always returns immediately.

The "retry number" is a bit mis-named in the Hyper-V Top Level
Functional Spec (TLFS).  It is essentially a threshold value.  Hyper-V is
saying "don't bother to advise me about the spin lock until you have
done a certain number of spins."  This threshold prevents
over-notifying Hyper-V such that the notification becomes somewhat
meaningless.   It's not immediately clear to me why the hypercall passes
that value as an input, but maybe it lets the Hyper-V scheduler prioritize
among vCPUs based on how many times they have spun for a lock.  I
think we were told that current Hyper-V implementations ignore this
input value anyway.

I believe the description of the sentinel value 0xFFFFFFFF in the
Hyper-V TLFS is incorrect.  Because it is the max possible threshold
value, that value in the EBX register just means to not ever bother to
notify.   The description should be "0xFFFFFFFF indicates never to notify."
The value does *not* indicate anything about retrying to obtain the
spin lock.

>  static bool __initdata hv_pvspin = true;
> 
> +bool hv_notify_long_spin_wait(int retry_num)

retry_num should probably be declared as unsigned int.  You
don't want it to be treated as a negative number if the high
order bit is set.

> +{
> +	/*
> +	 * Per MSFT TLFS, the SpinWaitInfo is sent to hypervisor only when
> +	 * the retry number exceeds the recommended number.
> +	 *
> +	 * If recommended number is 0xFFFFFFFF, never retry.
> +	 */
> +	if (ms_hyperv.num_spin_retry == HYPERV_SPINLOCK_RETRY_NEVER)
> +		return false;
> +
> +	if ((0 == retry_num % ms_hyperv.num_spin_retry) && retry_num)

I don't know if the "%" function is right here.  Your implementation will
notify Hyper-V on every multiple of num_spin_retry.   The alternative is to
notify once when the threshold is exceeded, and never again for this
particular attempt to obtain a spin lock.  We should check with the Hyper-V
team for which approach they expect to be used.

> +		hv_do_fast_hypercall8(HVCALL_NOTIFY_LONG_SPIN_WAIT,
> +				      retry_num);

The Hyper-V TLFS seems to be inconsistent on whether the input parameter
is 32-bits or 64-bits.   In one place it is typed as UINT64, but in another place
it is shown as only 4 bytes.  Need to clear this up with the Hyper-V team as
well.

> +
> +	return true;

I don't see a need for this function to return true vs. false.  Any calling code
should not change its behavior based on num_spin_retry.   This function will
either notify Hyper-V or not notify Hyper-V, depending on whether the number
of attempts to obtain the spinlock meets the threshold.  But calling code will
do the same thing regardless of whether such a notification is made. 

Michael


  parent reply	other threads:[~2018-10-24 16:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19 13:13 [PATCH v1 0/2] Enable HvNotifyLongSpinWait for Hyper-V Yi Sun
2018-10-19 13:13 ` [PATCH v1 1/2] x86/hyperv: get spinlock retry number on Hyper-V Yi Sun
2018-10-19 13:13 ` [PATCH v1 2/2] x86/hyperv: make HvNotifyLongSpinWait hypercall Yi Sun
2018-10-19 14:20   ` Juergen Gross
2018-10-22  1:53     ` Yi Sun
2018-10-22  7:32       ` Juergen Gross
2018-10-22 16:31         ` Waiman Long
2018-10-22 17:15           ` Peter Zijlstra
2018-10-22 17:27             ` Waiman Long
2018-10-22 17:31               ` Peter Zijlstra
2018-10-22 18:01                 ` Waiman Long
2018-10-23  2:57             ` Yi Sun
2018-10-23  8:51               ` Peter Zijlstra
2018-10-23  9:33                 ` Yi Sun
2018-10-31  1:54                   ` Yi Sun
2018-10-31 14:10                     ` Peter Zijlstra
2018-10-31 15:07                       ` Waiman Long
2018-10-31 17:15                         ` Peter Zijlstra
2018-11-01  3:20                           ` Yi Sun
2018-11-01  8:59                             ` Peter Zijlstra
2018-11-01 12:59                             ` Waiman Long
2018-11-05  6:54                               ` Yi Sun
2018-10-24 16:53   ` Michael Kelley [this message]
2018-10-25  2:23     ` Yi Sun
2018-10-31  2:06     ` Yi Sun

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=CY4PR21MB07737AA8E3F7E7353D42F5C8D7F60@CY4PR21MB0773.namprd21.prod.outlook.com \
    --to=mikelley@microsoft.com \
    --cc=Tianyu.Lan@microsoft.com \
    --cc=chao.gao@intel.com \
    --cc=chao.p.peng@intel.com \
    --cc=haiyangz@microsoft.com \
    --cc=isaku.yamahata@intel.com \
    --cc=jgross@suse.com \
    --cc=kys@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yi.y.sun@linux.intel.com \
    --subject='RE: [PATCH v1 2/2] x86/hyperv: make HvNotifyLongSpinWait hypercall' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.