All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Liu <wei.liu@kernel.org>
To: Dawei Li <set_pte_at@outlook.com>
Cc: jgross@suse.com, wei.liu@kernel.org, sstabellini@kernel.org,
	linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
	haiyangz@microsoft.com, linuxppc-dev@lists.ozlabs.org,
	decui@microsoft.com, alsa-devel@alsa-project.org,
	npiggin@gmail.com, linux-kernel@vger.kernel.org,
	oleksandr_tyshchenko@epam.com, srinivas.kandagatla@linaro.org,
	christophe.leroy@csgroup.eu, bgoswami@quicinc.com,
	mpe@ellerman.id.au, xen-devel@lists.xenproject.org,
	johannes@sipsolutions.net, kys@microsoft.com,
	robert.jarzmik@free.fr, roger.pau@citrix.com
Subject: Re: [PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned
Date: Tue, 6 Dec 2022 11:37:26 +0000	[thread overview]
Message-ID: <Y48pdr9DEmXShhFR@liuwe-devbox-debian-v2> (raw)
In-Reply-To: <TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM>

On Mon, Dec 05, 2022 at 11:36:39PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
> 
> This change is for hyperv bus based drivers.
> 
> Signed-off-by: Dawei Li <set_pte_at@outlook.com>
[...]
> -static int netvsc_remove(struct hv_device *dev)
> +static void netvsc_remove(struct hv_device *dev)
>  {
>  	struct net_device_context *ndev_ctx;
>  	struct net_device *vf_netdev, *net;
> @@ -2603,7 +2603,6 @@ static int netvsc_remove(struct hv_device *dev)
>  	net = hv_get_drvdata(dev);
>  	if (net == NULL) {
>  		dev_err(&dev->device, "No net device to remove\n");
> -		return 0;

This is wrong. You are introducing a NULL pointer dereference.

>  	}
>  
>  	ndev_ctx = netdev_priv(net);
> @@ -2637,7 +2636,6 @@ static int netvsc_remove(struct hv_device *dev)
>  
>  	free_percpu(ndev_ctx->vf_stats);
>  	free_netdev(net);
> -	return 0;
>  }
>  
>  static int netvsc_suspend(struct hv_device *dev)
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index ba64284eaf9f..3a09de70d6ea 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3756,7 +3756,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
>   *
>   * Return: 0 on success, -errno on failure
>   */

This comment is no longer needed in the new world.

But, are you sure you're modifying the correct piece of code?

hv_pci_remove is not a hook in the base bus type. It is used in struct
hv_driver.

The same comment applies to all other modifications.

Thanks,
Wei.

WARNING: multiple messages have this Message-ID (diff)
From: Wei Liu <wei.liu@kernel.org>
To: Dawei Li <set_pte_at@outlook.com>
Cc: gregkh@linuxfoundation.org, johannes@sipsolutions.net,
	robert.jarzmik@free.fr, jgross@suse.com, sstabellini@kernel.org,
	oleksandr_tyshchenko@epam.com, roger.pau@citrix.com,
	srinivas.kandagatla@linaro.org, bgoswami@quicinc.com,
	mpe@ellerman.id.au, npiggin@gmail.com,
	christophe.leroy@csgroup.eu, kys@microsoft.com,
	haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
	alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
	xen-devel@lists.xenproject.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned
Date: Tue, 6 Dec 2022 11:37:26 +0000	[thread overview]
Message-ID: <Y48pdr9DEmXShhFR@liuwe-devbox-debian-v2> (raw)
In-Reply-To: <TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM>

On Mon, Dec 05, 2022 at 11:36:39PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
> 
> This change is for hyperv bus based drivers.
> 
> Signed-off-by: Dawei Li <set_pte_at@outlook.com>
[...]
> -static int netvsc_remove(struct hv_device *dev)
> +static void netvsc_remove(struct hv_device *dev)
>  {
>  	struct net_device_context *ndev_ctx;
>  	struct net_device *vf_netdev, *net;
> @@ -2603,7 +2603,6 @@ static int netvsc_remove(struct hv_device *dev)
>  	net = hv_get_drvdata(dev);
>  	if (net == NULL) {
>  		dev_err(&dev->device, "No net device to remove\n");
> -		return 0;

This is wrong. You are introducing a NULL pointer dereference.

>  	}
>  
>  	ndev_ctx = netdev_priv(net);
> @@ -2637,7 +2636,6 @@ static int netvsc_remove(struct hv_device *dev)
>  
>  	free_percpu(ndev_ctx->vf_stats);
>  	free_netdev(net);
> -	return 0;
>  }
>  
>  static int netvsc_suspend(struct hv_device *dev)
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index ba64284eaf9f..3a09de70d6ea 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3756,7 +3756,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
>   *
>   * Return: 0 on success, -errno on failure
>   */

This comment is no longer needed in the new world.

But, are you sure you're modifying the correct piece of code?

hv_pci_remove is not a hook in the base bus type. It is used in struct
hv_driver.

The same comment applies to all other modifications.

Thanks,
Wei.

WARNING: multiple messages have this Message-ID (diff)
From: Wei Liu <wei.liu@kernel.org>
To: Dawei Li <set_pte_at@outlook.com>
Cc: jgross@suse.com, wei.liu@kernel.org, sstabellini@kernel.org,
	linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
	haiyangz@microsoft.com, linuxppc-dev@lists.ozlabs.org,
	decui@microsoft.com, alsa-devel@alsa-project.org,
	npiggin@gmail.com, linux-kernel@vger.kernel.org,
	oleksandr_tyshchenko@epam.com, srinivas.kandagatla@linaro.org,
	bgoswami@quicinc.com, xen-devel@lists.xenproject.org,
	johannes@sipsolutions.net, kys@microsoft.com,
	robert.jarzmik@free.fr, roger.pau@citrix.com
Subject: Re: [PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned
Date: Tue, 6 Dec 2022 11:37:26 +0000	[thread overview]
Message-ID: <Y48pdr9DEmXShhFR@liuwe-devbox-debian-v2> (raw)
In-Reply-To: <TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM>

On Mon, Dec 05, 2022 at 11:36:39PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
> 
> This change is for hyperv bus based drivers.
> 
> Signed-off-by: Dawei Li <set_pte_at@outlook.com>
[...]
> -static int netvsc_remove(struct hv_device *dev)
> +static void netvsc_remove(struct hv_device *dev)
>  {
>  	struct net_device_context *ndev_ctx;
>  	struct net_device *vf_netdev, *net;
> @@ -2603,7 +2603,6 @@ static int netvsc_remove(struct hv_device *dev)
>  	net = hv_get_drvdata(dev);
>  	if (net == NULL) {
>  		dev_err(&dev->device, "No net device to remove\n");
> -		return 0;

This is wrong. You are introducing a NULL pointer dereference.

>  	}
>  
>  	ndev_ctx = netdev_priv(net);
> @@ -2637,7 +2636,6 @@ static int netvsc_remove(struct hv_device *dev)
>  
>  	free_percpu(ndev_ctx->vf_stats);
>  	free_netdev(net);
> -	return 0;
>  }
>  
>  static int netvsc_suspend(struct hv_device *dev)
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index ba64284eaf9f..3a09de70d6ea 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3756,7 +3756,7 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
>   *
>   * Return: 0 on success, -errno on failure
>   */

This comment is no longer needed in the new world.

But, are you sure you're modifying the correct piece of code?

hv_pci_remove is not a hook in the base bus type. It is used in struct
hv_driver.

The same comment applies to all other modifications.

Thanks,
Wei.

  parent reply	other threads:[~2022-12-06 13:59 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20221205153644.60909-1-set_pte_at@outlook.com>
2022-12-05 15:36 ` [PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 22:55   ` kernel test robot
2022-12-05 22:55     ` kernel test robot
2022-12-05 22:55     ` kernel test robot
2022-12-06 11:37   ` Wei Liu [this message]
2022-12-06 11:37     ` Wei Liu
2022-12-06 11:37     ` Wei Liu
2022-12-06 14:17     ` Dawei Li
2022-12-06 14:17       ` Dawei Li
2022-12-06 14:17       ` Dawei Li
2022-12-05 15:36 ` [PATCH 2/6] macio: Make remove callback of macio " Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 16:08   ` Greg KH
2022-12-05 16:08     ` Greg KH
2022-12-05 16:08     ` Greg KH
2022-12-05 15:36 ` [PATCH 3/6] apr: make remove callback of apr " Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36 ` [PATCH 4/6] xen: make remove callback of xen " Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:47   ` Juergen Gross
2022-12-05 15:47     ` Juergen Gross
2022-12-05 15:47     ` Juergen Gross
2022-12-05 16:02   ` Greg KH
2022-12-05 16:02     ` Greg KH
2022-12-05 16:02     ` Greg KH
2022-12-05 15:36 ` [PATCH 5/6] ac97: make remove callback of ac97 " Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 16:03   ` Greg KH
2022-12-05 16:03     ` Greg KH
2022-12-05 16:03     ` Greg KH
2022-12-05 15:36 ` [PATCH 6/6] soundbus: make remove callback of soundbus " Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 15:36   ` Dawei Li
2022-12-05 16:07   ` Greg KH
2022-12-05 16:07     ` Greg KH
2022-12-05 16:07     ` Greg KH

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=Y48pdr9DEmXShhFR@liuwe-devbox-debian-v2 \
    --to=wei.liu@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=bgoswami@quicinc.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=decui@microsoft.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haiyangz@microsoft.com \
    --cc=jgross@suse.com \
    --cc=johannes@sipsolutions.net \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=robert.jarzmik@free.fr \
    --cc=roger.pau@citrix.com \
    --cc=set_pte_at@outlook.com \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.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 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.