Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 22 Sep 2022 17:19:40 +0000
From:   Wei Liu <wei.liu@kernel.org>
To:     Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
Cc:     skinsburskii@microsoft.com,
        Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>,
        "K. Y. Srinivasan" <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
        linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
        Michael Kelley <mikelley@microsoft.com>
Subject: Re: [PATCH] Drivers: hv: vmbus: Don't wait for the ACPI device upon
 initialization
Message-ID: <YyyZLMMiDfExQI2v@liuwe-devbox-debian-v2>
References: <166378554568.581670.1124852716698789244.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <166378554568.581670.1124852716698789244.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
Precedence: bulk

On Wed, Sep 21, 2022 at 06:39:05PM +0000, Stanislav Kinsburskii wrote:
> From: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> 
> Waiting to 5 seconds in case of missing VMBUS ACPI device is redundant as the
> device is either present already or won't be available at all.
> This patch enforces synchronous probing to make sure the bus traversal,
> happening upon driver registering will either find the device (if present) or
> not spend any additional time if device is absent.
> 
> Signed-off-by: Stanislav Kinsburskiy <stanislav.kinsburskiy@gmail.com>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Stephen Hemminger <sthemmin@microsoft.com>
> CC: Wei Liu <wei.liu@kernel.org>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: linux-hyperv@vger.kernel.org
> CC: linux-kernel@vger.kernel.org

Looks good to me. I will wait for a few days for others to chime in.

CC Michael.

> ---
>  drivers/hv/vmbus_drv.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 7b9f3fc3adf7..32d0009631a6 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -46,8 +46,6 @@ struct vmbus_dynid {
>  
>  static struct acpi_device  *hv_acpi_dev;
>  
> -static struct completion probe_event;
> -
>  static int hyperv_cpuhp_online;
>  
>  static void *hv_panic_page;
> @@ -2468,7 +2466,6 @@ static int vmbus_acpi_add(struct acpi_device *device)
>  	ret_val = 0;
>  
>  acpi_walk_err:
> -	complete(&probe_event);
>  	if (ret_val)
>  		vmbus_acpi_remove(device);
>  	return ret_val;
> @@ -2647,6 +2644,7 @@ static struct acpi_driver vmbus_acpi_driver = {
>  		.remove = vmbus_acpi_remove,
>  	},
>  	.drv.pm = &vmbus_bus_pm,
> +	.drv.probe_type = PROBE_FORCE_SYNCHRONOUS,
>  };
>  
>  static void hv_kexec_handler(void)
> @@ -2719,7 +2717,7 @@ static struct syscore_ops hv_synic_syscore_ops = {
>  
>  static int __init hv_acpi_init(void)
>  {
> -	int ret, t;
> +	int ret;
>  
>  	if (!hv_is_hyperv_initialized())
>  		return -ENODEV;
> @@ -2727,8 +2725,6 @@ static int __init hv_acpi_init(void)
>  	if (hv_root_partition)
>  		return 0;
>  
> -	init_completion(&probe_event);
> -
>  	/*
>  	 * Get ACPI resources first.
>  	 */
> @@ -2737,9 +2733,8 @@ static int __init hv_acpi_init(void)
>  	if (ret)
>  		return ret;
>  
> -	t = wait_for_completion_timeout(&probe_event, 5*HZ);
> -	if (t == 0) {
> -		ret = -ETIMEDOUT;
> +	if (!hv_acpi_dev) {
> +		ret = -ENODEV;
>  		goto cleanup;
>  	}
>  
> 
>