linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Dexuan Cui <decui@microsoft.com>
Cc: wei.liu@kernel.org, kys@microsoft.com, haiyangz@microsoft.com,
	sthemmin@microsoft.com, bhelgaas@google.com,
	linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, mikelley@microsoft.com,
	robh@kernel.org, kw@linux.com, jakeo@microsoft.com
Subject: Re: [PATCH] PCI: hv: Do not set PCI_COMMAND_MEMORY to reduce VM boot time
Date: Fri, 29 Apr 2022 11:15:17 +0100	[thread overview]
Message-ID: <Ymu6tYItgM6dtNdd@lpieralisi> (raw)
In-Reply-To: <20220419220007.26550-1-decui@microsoft.com>

On Tue, Apr 19, 2022 at 03:00:07PM -0700, Dexuan Cui wrote:
> A VM on Azure can have 14 GPUs, and each GPU may have a huge MMIO BAR,
> e.g. 128 GB. Currently the boot time of such a VM can be 4+ minutes, and
> most of the time is used by the host to unmap/map the vBAR from/to pBAR
> when the VM clears and sets the PCI_COMMAND_MEMORY bit: each unmap/map
> operation for a 128GB BAR needs about 1.8 seconds, and the pci-hyperv
> driver and the Linux PCI subsystem flip the PCI_COMMAND_MEMORY bit
> eight times (see pci_setup_device() -> pci_read_bases() and
> pci_std_update_resource()), increasing the boot time by 1.8 * 8 = 14.4
> seconds per GPU, i.e. 14.4 * 14 = 201.6 seconds in total.
> 
> Fix the slowness by not turning on the PCI_COMMAND_MEMORY in pci-hyperv.c,
> so the bit stays in the off state before the PCI device driver calls
> pci_enable_device(): when the bit is off, pci_read_bases() and
> pci_std_update_resource() don't cause Hyper-V to unmap/map the vBARs.
> With this change, the boot time of such a VM is reduced by
> 1.8 * (8-1) * 14 = 176.4 seconds.

I believe you need to clarify this commit message. It took me a while
to understand what you are really doing.

What this patch is doing is bootstrapping PCI devices with command
memory clear because there is no need to have it set (?) in the first
place and because, if it is set, the PCI core layer needs to toggle it
on and off in order to eg size BAR regions, which causes the slow down
you are reporting.

I assume, given the above, that there is strictly no need to have
devices with command memory set at kernel startup handover and if
there was it would not be set in the PCI Hyper-V host controller
driver (because that's what you are _removing_).

I think this should not be merged as a fix and I'd be careful
about possible regressions before sending it to stable kernels,
if you wish to do so.

It is fine by me to go via the Hyper-V tree even though I don't
see why that's better, unless you want to send it as a fix and
I think you should not.

You can add my tag but the commit log should be rewritten and
you should add a Link: to the discussion thread.

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> Tested-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Cc: Jake Oshins <jakeo@microsoft.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index d270a204324e..f9fbbd8d94db 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2082,12 +2082,17 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)
>  				}
>  			}
>  			if (high_size <= 1 && low_size <= 1) {
> -				/* Set the memory enable bit. */
> -				_hv_pcifront_read_config(hpdev, PCI_COMMAND, 2,
> -							 &command);
> -				command |= PCI_COMMAND_MEMORY;
> -				_hv_pcifront_write_config(hpdev, PCI_COMMAND, 2,
> -							  command);
> +				/*
> +				 * No need to set the PCI_COMMAND_MEMORY bit as
> +				 * the core PCI driver doesn't require the bit
> +				 * to be pre-set. Actually here we intentionally
> +				 * keep the bit off so that the PCI BAR probing
> +				 * in the core PCI driver doesn't cause Hyper-V
> +				 * to unnecessarily unmap/map the virtual BARs
> +				 * from/to the physical BARs multiple times.
> +				 * This reduces the VM boot time significantly
> +				 * if the BAR sizes are huge.
> +				 */
>  				break;
>  			}
>  		}
> -- 
> 2.17.1
> 

  parent reply	other threads:[~2022-04-29 10:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 22:00 [PATCH] PCI: hv: Do not set PCI_COMMAND_MEMORY to reduce VM boot time Dexuan Cui
2022-04-20 14:47 ` Michael Kelley (LINUX)
2022-04-29 10:15 ` Lorenzo Pieralisi [this message]
2022-04-29 15:47   ` Dexuan Cui
2022-04-21 18:26 Dexuan Cui
2022-04-26 16:44 ` Lorenzo Pieralisi
2022-04-26 18:31   ` Dexuan Cui
2022-04-26 19:25     ` Jake Oshins
2022-04-27 22:41       ` Alex Williamson
2022-04-28 19:12       ` Bjorn Helgaas

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=Ymu6tYItgM6dtNdd@lpieralisi \
    --to=lorenzo.pieralisi@arm.com \
    --cc=bhelgaas@google.com \
    --cc=decui@microsoft.com \
    --cc=haiyangz@microsoft.com \
    --cc=jakeo@microsoft.com \
    --cc=kw@linux.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=robh@kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=wei.liu@kernel.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).