linux-kernel - Re: [PATCH] Drivers: hv: vmbus: Don't wait for the ACPI device upon initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 22 Sep 2022 17:19:40 +0000
From:   Wei Liu <wei.liu@...nel.org>
To:     Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
Cc:     skinsburskii@...rosoft.com,
        Stanislav Kinsburskiy <stanislav.kinsburskiy@...il.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michael Kelley <mikelley@...rosoft.com>
Subject: Re: [PATCH] Drivers: hv: vmbus: Don't wait for the ACPI device upon
 initialization

On Wed, Sep 21, 2022 at 06:39:05PM +0000, Stanislav Kinsburskii wrote:
> From: Stanislav Kinsburskiy <stanislav.kinsburskiy@...il.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@...il.com>
> CC: "K. Y. Srinivasan" <kys@...rosoft.com>
> CC: Haiyang Zhang <haiyangz@...rosoft.com>
> CC: Stephen Hemminger <sthemmin@...rosoft.com>
> CC: Wei Liu <wei.liu@...nel.org>
> CC: Dexuan Cui <decui@...rosoft.com>
> CC: linux-hyperv@...r.kernel.org
> CC: linux-kernel@...r.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;
>  	}
>  
> 
>