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;
Message-ID: <df9d5443-b563-c4c8-1038-6a01c132a132@linux.microsoft.com>
Date:   Tue, 27 Jun 2023 09:58:05 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH v2] Drivers: hv: Change hv_free_hyperv_page() to take void
 * argument
Content-Language: en-US
To:     Kameron Carr <kameroncarr@linux.microsoft.com>, arnd@arndb.de,
        decui@microsoft.com, haiyangz@microsoft.com, kys@microsoft.com,
        linux-arch@vger.kernel.org, linux-hyperv@vger.kernel.org,
        linux-kernel@vger.kernel.org, wei.liu@kernel.org
References: <1687558189-19734-1-git-send-email-kameroncarr@linux.microsoft.com>
From:   Nuno Das Neves <nunodasneves@linux.microsoft.com>
In-Reply-To: <1687558189-19734-1-git-send-email-kameroncarr@linux.microsoft.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

On 6/23/2023 3:09 PM, Kameron Carr wrote:
> Currently hv_free_hyperv_page() takes an unsigned long argument, which
> is inconsistent with the void * return value from the corresponding
> hv_alloc_hyperv_page() function and variants. This creates unnecessary
> extra casting.
> 
> Change the hv_free_hyperv_page() argument type to void *.
> Also remove redundant casts from invocations of
> hv_alloc_hyperv_page() and variants.
> 
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
> V1 -> V2: Added Signed-off-by
> 
>  drivers/hv/connection.c        | 13 ++++++-------
>  drivers/hv/hv_common.c         | 10 +++++-----
>  include/asm-generic/mshyperv.h |  2 +-
>  3 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 5978e9d..ebf15f3 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -209,8 +209,7 @@ int vmbus_connect(void)
>  	 * Setup the vmbus event connection for channel interrupt
>  	 * abstraction stuff
>  	 */
> -	vmbus_connection.int_page =
> -	(void *)hv_alloc_hyperv_zeroed_page();
> +	vmbus_connection.int_page = hv_alloc_hyperv_zeroed_page();
>  	if (vmbus_connection.int_page == NULL) {
>  		ret = -ENOMEM;
>  		goto cleanup;
> @@ -225,8 +224,8 @@ int vmbus_connect(void)
>  	 * Setup the monitor notification facility. The 1st page for
>  	 * parent->child and the 2nd page for child->parent
>  	 */
> -	vmbus_connection.monitor_pages[0] = (void *)hv_alloc_hyperv_page();
> -	vmbus_connection.monitor_pages[1] = (void *)hv_alloc_hyperv_page();
> +	vmbus_connection.monitor_pages[0] = hv_alloc_hyperv_page();
> +	vmbus_connection.monitor_pages[1] = hv_alloc_hyperv_page();
>  	if ((vmbus_connection.monitor_pages[0] == NULL) ||
>  	    (vmbus_connection.monitor_pages[1] == NULL)) {
>  		ret = -ENOMEM;
> @@ -333,15 +332,15 @@ void vmbus_disconnect(void)
>  		destroy_workqueue(vmbus_connection.work_queue);
>  
>  	if (vmbus_connection.int_page) {
> -		hv_free_hyperv_page((unsigned long)vmbus_connection.int_page);
> +		hv_free_hyperv_page(vmbus_connection.int_page);
>  		vmbus_connection.int_page = NULL;
>  	}
>  
>  	set_memory_encrypted((unsigned long)vmbus_connection.monitor_pages[0], 1);
>  	set_memory_encrypted((unsigned long)vmbus_connection.monitor_pages[1], 1);
>  
> -	hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[0]);
> -	hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[1]);
> +	hv_free_hyperv_page(vmbus_connection.monitor_pages[0]);
> +	hv_free_hyperv_page(vmbus_connection.monitor_pages[1]);
>  	vmbus_connection.monitor_pages[0] = NULL;
>  	vmbus_connection.monitor_pages[1] = NULL;
>  }
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 542a1d5..6a2258f 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -115,12 +115,12 @@ void *hv_alloc_hyperv_zeroed_page(void)
>  }
>  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
>  
> -void hv_free_hyperv_page(unsigned long addr)
> +void hv_free_hyperv_page(void *addr)
>  {
>  	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> -		free_page(addr);
> +		free_page((unsigned long)addr);
>  	else
> -		kfree((void *)addr);
> +		kfree(addr);
>  }
>  EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
>  
> @@ -253,7 +253,7 @@ static void hv_kmsg_dump_unregister(void)
>  	atomic_notifier_chain_unregister(&panic_notifier_list,
>  					 &hyperv_panic_report_block);
>  
> -	hv_free_hyperv_page((unsigned long)hv_panic_page);
> +	hv_free_hyperv_page(hv_panic_page);
>  	hv_panic_page = NULL;
>  }
>  
> @@ -270,7 +270,7 @@ static void hv_kmsg_dump_register(void)
>  	ret = kmsg_dump_register(&hv_kmsg_dumper);
>  	if (ret) {
>  		pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> -		hv_free_hyperv_page((unsigned long)hv_panic_page);
> +		hv_free_hyperv_page(hv_panic_page);
>  		hv_panic_page = NULL;
>  	}
>  }
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 402a8c1..a8f4b65 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -190,7 +190,7 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
>  
>  void *hv_alloc_hyperv_page(void);
>  void *hv_alloc_hyperv_zeroed_page(void);
> -void hv_free_hyperv_page(unsigned long addr);
> +void hv_free_hyperv_page(void *addr);
>  
>  /**
>   * hv_cpu_number_to_vp_number() - Map CPU to VP.