2023-01-05 15:38:23

by Dawei Li

[permalink] [raw]
Subject: [PATCH v3] Drivers: hv: Make remove callback of hyperv driver void returned

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.

As such, change the remove function for Hyper-V VMBus based
drivers to return void.

Signed-off-by: Dawei Li <[email protected]>
---
v2 -> v3
- Update commit message and restore blank line as it is,
based on the comments from Michael Kelley.

v1 -> v2
- Fixed null dereference issue and deprecated comments, based
on comments by Liu Wei.
- Fixed building issue.
- Rebased on latest hyperv-next.

v1
- https://lore.kernel.org/all/TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM/
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +---
drivers/hid/hid-hyperv.c | 4 +---
drivers/hv/hv_balloon.c | 4 +---
drivers/hv/hv_util.c | 4 +---
drivers/input/serio/hyperv-keyboard.c | 4 +---
drivers/net/hyperv/netvsc_drv.c | 5 ++---
drivers/pci/controller/pci-hyperv.c | 8 ++------
drivers/scsi/storvsc_drv.c | 4 +---
drivers/uio/uio_hv_generic.c | 5 ++---
drivers/video/fbdev/hyperv_fb.c | 5 +----
include/linux/hyperv.h | 2 +-
net/vmw_vsock/hyperv_transport.c | 4 +---
12 files changed, 15 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index ca127ff797f7..d117fff26d99 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -165,7 +165,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
return ret;
}

-static int hyperv_vmbus_remove(struct hv_device *hdev)
+static void hyperv_vmbus_remove(struct hv_device *hdev)
{
struct drm_device *dev = hv_get_drvdata(hdev);
struct hyperv_drm_device *hv = to_hv(dev);
@@ -176,8 +176,6 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
hv_set_drvdata(hdev, NULL);

vmbus_free_mmio(hv->mem->start, hv->fb_size);
-
- return 0;
}

static int hyperv_vmbus_suspend(struct hv_device *hdev)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index ab57b49a44ed..ef16c2a54362 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -535,7 +535,7 @@ static int mousevsc_probe(struct hv_device *device,
}


-static int mousevsc_remove(struct hv_device *dev)
+static void mousevsc_remove(struct hv_device *dev)
{
struct mousevsc_dev *input_dev = hv_get_drvdata(dev);

@@ -544,8 +544,6 @@ static int mousevsc_remove(struct hv_device *dev)
hid_hw_stop(input_dev->hid_device);
hid_destroy_device(input_dev->hid_device);
mousevsc_free_device(input_dev);
-
- return 0;
}

static int mousevsc_suspend(struct hv_device *dev)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index cbe43e2567a7..0d4b8fc47ac6 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -2042,7 +2042,7 @@ static int balloon_probe(struct hv_device *dev,
return ret;
}

-static int balloon_remove(struct hv_device *dev)
+static void balloon_remove(struct hv_device *dev)
{
struct hv_dynmem_device *dm = hv_get_drvdata(dev);
struct hv_hotadd_state *has, *tmp;
@@ -2083,8 +2083,6 @@ static int balloon_remove(struct hv_device *dev)
kfree(has);
}
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
-
- return 0;
}

static int balloon_suspend(struct hv_device *hv_dev)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 835e6039c186..24995ac41c86 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -602,7 +602,7 @@ static int util_probe(struct hv_device *dev,
return ret;
}

-static int util_remove(struct hv_device *dev)
+static void util_remove(struct hv_device *dev)
{
struct hv_util_service *srv = hv_get_drvdata(dev);

@@ -610,8 +610,6 @@ static int util_remove(struct hv_device *dev)
srv->util_deinit();
vmbus_close(dev->channel);
kfree(srv->recv_buffer);
-
- return 0;
}

/*
diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
index d62aefb2e245..31def6ce5157 100644
--- a/drivers/input/serio/hyperv-keyboard.c
+++ b/drivers/input/serio/hyperv-keyboard.c
@@ -369,7 +369,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
return error;
}

-static int hv_kbd_remove(struct hv_device *hv_dev)
+static void hv_kbd_remove(struct hv_device *hv_dev)
{
struct hv_kbd_dev *kbd_dev = hv_get_drvdata(hv_dev);

@@ -378,8 +378,6 @@ static int hv_kbd_remove(struct hv_device *hv_dev)
kfree(kbd_dev);

hv_set_drvdata(hv_dev, NULL);
-
- return 0;
}

static int hv_kbd_suspend(struct hv_device *hv_dev)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 89eb4f179a3c..025f805e1ed9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2594,7 +2594,7 @@ static int netvsc_probe(struct hv_device *dev,
return ret;
}

-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,7 @@ 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;
+ return;
}

ndev_ctx = netdev_priv(net);
@@ -2637,7 +2637,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 583d3aad6908..e46d9a14053f 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -3813,13 +3813,10 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
/**
* hv_pci_remove() - Remove routine for this VMBus channel
* @hdev: VMBus's tracking struct for this root PCI bus
- *
- * Return: 0 on success, -errno on failure
*/
-static int hv_pci_remove(struct hv_device *hdev)
+static void hv_pci_remove(struct hv_device *hdev)
{
struct hv_pcibus_device *hbus;
- int ret;

hbus = hv_get_drvdata(hdev);
if (hbus->state == hv_pcibus_installed) {
@@ -3842,7 +3839,7 @@ static int hv_pci_remove(struct hv_device *hdev)
pci_unlock_rescan_remove();
}

- ret = hv_pci_bus_exit(hdev, false);
+ hv_pci_bus_exit(hdev, false);

vmbus_close(hdev->channel);

@@ -3855,7 +3852,6 @@ static int hv_pci_remove(struct hv_device *hdev)
hv_put_dom_num(hbus->bridge->domain_nr);

kfree(hbus);
- return ret;
}

static int hv_pci_suspend(struct hv_device *hdev)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 3c5b7e4227b2..02f9d1a6f4ac 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -2092,7 +2092,7 @@ static int storvsc_change_queue_depth(struct scsi_device *sdev, int queue_depth)
return scsi_change_queue_depth(sdev, queue_depth);
}

-static int storvsc_remove(struct hv_device *dev)
+static void storvsc_remove(struct hv_device *dev)
{
struct storvsc_device *stor_device = hv_get_drvdata(dev);
struct Scsi_Host *host = stor_device->host;
@@ -2108,8 +2108,6 @@ static int storvsc_remove(struct hv_device *dev)
scsi_remove_host(host);
storvsc_dev_remove(dev);
scsi_host_put(host);
-
- return 0;
}

static int storvsc_suspend(struct hv_device *hv_dev)
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index c08a6cfd119f..20d9762331bd 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -355,20 +355,19 @@ hv_uio_probe(struct hv_device *dev,
return ret;
}

-static int
+static void
hv_uio_remove(struct hv_device *dev)
{
struct hv_uio_private_data *pdata = hv_get_drvdata(dev);

if (!pdata)
- return 0;
+ return;

sysfs_remove_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr);
uio_unregister_device(&pdata->info);
hv_uio_cleanup(dev, pdata);

vmbus_free_ring(dev->channel);
- return 0;
}

static struct hv_driver hv_uio_drv = {
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 3ce746a46179..cfa6a7c1eeb3 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1240,8 +1240,7 @@ static int hvfb_probe(struct hv_device *hdev,
return ret;
}

-
-static int hvfb_remove(struct hv_device *hdev)
+static void hvfb_remove(struct hv_device *hdev)
{
struct fb_info *info = hv_get_drvdata(hdev);
struct hvfb_par *par = info->par;
@@ -1262,8 +1261,6 @@ static int hvfb_remove(struct hv_device *hdev)

hvfb_putmem(hdev, info);
framebuffer_release(info);
-
- return 0;
}

static int hvfb_suspend(struct hv_device *hdev)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 85f7c5a63aa6..cd5cb9f6fae0 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1273,7 +1273,7 @@ struct hv_driver {
} dynids;

int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
- int (*remove)(struct hv_device *);
+ void (*remove)(struct hv_device *dev);
void (*shutdown)(struct hv_device *);

int (*suspend)(struct hv_device *);
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 59c3e2697069..7cb1a9d2cdb4 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -879,13 +879,11 @@ static int hvs_probe(struct hv_device *hdev,
return 0;
}

-static int hvs_remove(struct hv_device *hdev)
+static void hvs_remove(struct hv_device *hdev)
{
struct vmbus_channel *chan = hdev->channel;

vmbus_close(chan);
-
- return 0;
}

/* hv_sock connections can not persist across hibernation, and all the hv_sock
--
2.25.1


2023-01-05 15:55:38

by Praveen Kumar

[permalink] [raw]
Subject: Re: [PATCH v3] Drivers: hv: Make remove callback of hyperv driver void returned

On 05-01-2023 20:21, 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.
>
> As such, change the remove function for Hyper-V VMBus based
> drivers to return void.
>
> Signed-off-by: Dawei Li <[email protected]>
> ---
> v2 -> v3
> - Update commit message and restore blank line as it is,
> based on the comments from Michael Kelley.
>
> v1 -> v2
> - Fixed null dereference issue and deprecated comments, based
> on comments by Liu Wei.
> - Fixed building issue.
> - Rebased on latest hyperv-next.
>
> v1
> - https://lore.kernel.org/all/TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM/
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +---
> drivers/hid/hid-hyperv.c | 4 +---
> drivers/hv/hv_balloon.c | 4 +---
> drivers/hv/hv_util.c | 4 +---
> drivers/input/serio/hyperv-keyboard.c | 4 +---
> drivers/net/hyperv/netvsc_drv.c | 5 ++---
> drivers/pci/controller/pci-hyperv.c | 8 ++------
> drivers/scsi/storvsc_drv.c | 4 +---
> drivers/uio/uio_hv_generic.c | 5 ++---
> drivers/video/fbdev/hyperv_fb.c | 5 +----
> include/linux/hyperv.h | 2 +-
> net/vmw_vsock/hyperv_transport.c | 4 +---
> 12 files changed, 15 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index ca127ff797f7..d117fff26d99 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -165,7 +165,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
> return ret;
> }
>
> -static int hyperv_vmbus_remove(struct hv_device *hdev)
> +static void hyperv_vmbus_remove(struct hv_device *hdev)
> {
> struct drm_device *dev = hv_get_drvdata(hdev);
> struct hyperv_drm_device *hv = to_hv(dev);
> @@ -176,8 +176,6 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
> hv_set_drvdata(hdev, NULL);
>
> vmbus_free_mmio(hv->mem->start, hv->fb_size);
> -
> - return 0;
> }
>
> static int hyperv_vmbus_suspend(struct hv_device *hdev)
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index ab57b49a44ed..ef16c2a54362 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -535,7 +535,7 @@ static int mousevsc_probe(struct hv_device *device,
> }
>
>
> -static int mousevsc_remove(struct hv_device *dev)
> +static void mousevsc_remove(struct hv_device *dev)
> {
> struct mousevsc_dev *input_dev = hv_get_drvdata(dev);
>
> @@ -544,8 +544,6 @@ static int mousevsc_remove(struct hv_device *dev)
> hid_hw_stop(input_dev->hid_device);
> hid_destroy_device(input_dev->hid_device);
> mousevsc_free_device(input_dev);
> -
> - return 0;
> }
>
> static int mousevsc_suspend(struct hv_device *dev)
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index cbe43e2567a7..0d4b8fc47ac6 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -2042,7 +2042,7 @@ static int balloon_probe(struct hv_device *dev,
> return ret;
> }
>
> -static int balloon_remove(struct hv_device *dev)
> +static void balloon_remove(struct hv_device *dev)
> {
> struct hv_dynmem_device *dm = hv_get_drvdata(dev);
> struct hv_hotadd_state *has, *tmp;
> @@ -2083,8 +2083,6 @@ static int balloon_remove(struct hv_device *dev)
> kfree(has);
> }
> spin_unlock_irqrestore(&dm_device.ha_lock, flags);
> -
> - return 0;
> }
>
> static int balloon_suspend(struct hv_device *hv_dev)
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 835e6039c186..24995ac41c86 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -602,7 +602,7 @@ static int util_probe(struct hv_device *dev,
> return ret;
> }
>
> -static int util_remove(struct hv_device *dev)
> +static void util_remove(struct hv_device *dev)
> {
> struct hv_util_service *srv = hv_get_drvdata(dev);
>
> @@ -610,8 +610,6 @@ static int util_remove(struct hv_device *dev)
> srv->util_deinit();
> vmbus_close(dev->channel);
> kfree(srv->recv_buffer);
> -
> - return 0;
> }
>
> /*
> diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> index d62aefb2e245..31def6ce5157 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -369,7 +369,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
> return error;
> }
>
> -static int hv_kbd_remove(struct hv_device *hv_dev)
> +static void hv_kbd_remove(struct hv_device *hv_dev)
> {
> struct hv_kbd_dev *kbd_dev = hv_get_drvdata(hv_dev);
>
> @@ -378,8 +378,6 @@ static int hv_kbd_remove(struct hv_device *hv_dev)
> kfree(kbd_dev);
>
> hv_set_drvdata(hv_dev, NULL);
> -
> - return 0;
> }
>
> static int hv_kbd_suspend(struct hv_device *hv_dev)
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 89eb4f179a3c..025f805e1ed9 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2594,7 +2594,7 @@ static int netvsc_probe(struct hv_device *dev,
> return ret;
> }
>
> -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,7 @@ 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;
> + return;
> }
>
> ndev_ctx = netdev_priv(net);
> @@ -2637,7 +2637,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 583d3aad6908..e46d9a14053f 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -3813,13 +3813,10 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
> /**
> * hv_pci_remove() - Remove routine for this VMBus channel
> * @hdev: VMBus's tracking struct for this root PCI bus
> - *
> - * Return: 0 on success, -errno on failure
> */
> -static int hv_pci_remove(struct hv_device *hdev)
> +static void hv_pci_remove(struct hv_device *hdev)
> {
> struct hv_pcibus_device *hbus;
> - int ret;
>
> hbus = hv_get_drvdata(hdev);
> if (hbus->state == hv_pcibus_installed) {
> @@ -3842,7 +3839,7 @@ static int hv_pci_remove(struct hv_device *hdev)
> pci_unlock_rescan_remove();
> }
>
> - ret = hv_pci_bus_exit(hdev, false);
> + hv_pci_bus_exit(hdev, false);
>
> vmbus_close(hdev->channel);
>
> @@ -3855,7 +3852,6 @@ static int hv_pci_remove(struct hv_device *hdev)
> hv_put_dom_num(hbus->bridge->domain_nr);
>
> kfree(hbus);
> - return ret;
> }
>
> static int hv_pci_suspend(struct hv_device *hdev)
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 3c5b7e4227b2..02f9d1a6f4ac 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -2092,7 +2092,7 @@ static int storvsc_change_queue_depth(struct scsi_device *sdev, int queue_depth)
> return scsi_change_queue_depth(sdev, queue_depth);
> }
>
> -static int storvsc_remove(struct hv_device *dev)
> +static void storvsc_remove(struct hv_device *dev)
> {
> struct storvsc_device *stor_device = hv_get_drvdata(dev);
> struct Scsi_Host *host = stor_device->host;
> @@ -2108,8 +2108,6 @@ static int storvsc_remove(struct hv_device *dev)
> scsi_remove_host(host);
> storvsc_dev_remove(dev);
> scsi_host_put(host);
> -
> - return 0;
> }
>
> static int storvsc_suspend(struct hv_device *hv_dev)
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index c08a6cfd119f..20d9762331bd 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -355,20 +355,19 @@ hv_uio_probe(struct hv_device *dev,
> return ret;
> }
>
> -static int
> +static void
> hv_uio_remove(struct hv_device *dev)
> {
> struct hv_uio_private_data *pdata = hv_get_drvdata(dev);
>
> if (!pdata)
> - return 0;
> + return;
>
> sysfs_remove_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr);
> uio_unregister_device(&pdata->info);
> hv_uio_cleanup(dev, pdata);
>
> vmbus_free_ring(dev->channel);
> - return 0;
> }
>
> static struct hv_driver hv_uio_drv = {
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 3ce746a46179..cfa6a7c1eeb3 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -1240,8 +1240,7 @@ static int hvfb_probe(struct hv_device *hdev,
> return ret;
> }
>
> -
> -static int hvfb_remove(struct hv_device *hdev)
> +static void hvfb_remove(struct hv_device *hdev)
> {
> struct fb_info *info = hv_get_drvdata(hdev);
> struct hvfb_par *par = info->par;
> @@ -1262,8 +1261,6 @@ static int hvfb_remove(struct hv_device *hdev)
>
> hvfb_putmem(hdev, info);
> framebuffer_release(info);
> -
> - return 0;
> }
>
> static int hvfb_suspend(struct hv_device *hdev)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 85f7c5a63aa6..cd5cb9f6fae0 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1273,7 +1273,7 @@ struct hv_driver {
> } dynids;
>
> int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
> - int (*remove)(struct hv_device *);
> + void (*remove)(struct hv_device *dev);

nitpick for consistency from previous version

void (*remove)(struct hv_device *);


> void (*shutdown)(struct hv_device *);
>
> int (*suspend)(struct hv_device *);
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 59c3e2697069..7cb1a9d2cdb4 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -879,13 +879,11 @@ static int hvs_probe(struct hv_device *hdev,
> return 0;
> }
>
> -static int hvs_remove(struct hv_device *hdev)
> +static void hvs_remove(struct hv_device *hdev)
> {
> struct vmbus_channel *chan = hdev->channel;
>
> vmbus_close(chan);
> -
> - return 0;
> }
>
> /* hv_sock connections can not persist across hibernation, and all the hv_sock

2023-01-05 16:18:27

by Dawei Li

[permalink] [raw]
Subject: Re: [PATCH v3] Drivers: hv: Make remove callback of hyperv driver void returned

On Thu, Jan 05, 2023 at 09:19:31PM +0530, Praveen Kumar wrote:

Hi Praveen,

Thanks for reviewing.

> On 05-01-2023 20:21, 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.
> >
> > As such, change the remove function for Hyper-V VMBus based
> > drivers to return void.
> >
> > Signed-off-by: Dawei Li <[email protected]>
> > ---
> > v2 -> v3
> > - Update commit message and restore blank line as it is,
> > based on the comments from Michael Kelley.
> >
> > v1 -> v2
> > - Fixed null dereference issue and deprecated comments, based
> > on comments by Liu Wei.
> > - Fixed building issue.
> > - Rebased on latest hyperv-next.
> >
> > v1
> > - https://lore.kernel.org/all/TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM/
> > ---
> > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +---
> > drivers/hid/hid-hyperv.c | 4 +---
> > drivers/hv/hv_balloon.c | 4 +---
> > drivers/hv/hv_util.c | 4 +---
> > drivers/input/serio/hyperv-keyboard.c | 4 +---
> > drivers/net/hyperv/netvsc_drv.c | 5 ++---
> > drivers/pci/controller/pci-hyperv.c | 8 ++------
> > drivers/scsi/storvsc_drv.c | 4 +---
> > drivers/uio/uio_hv_generic.c | 5 ++---
> > drivers/video/fbdev/hyperv_fb.c | 5 +----
> > include/linux/hyperv.h | 2 +-
> > net/vmw_vsock/hyperv_transport.c | 4 +---
> > 12 files changed, 15 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > index ca127ff797f7..d117fff26d99 100644
> > --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > @@ -165,7 +165,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
> > return ret;
> > }
> >
> > -static int hyperv_vmbus_remove(struct hv_device *hdev)
> > +static void hyperv_vmbus_remove(struct hv_device *hdev)
> > {
> > struct drm_device *dev = hv_get_drvdata(hdev);
> > struct hyperv_drm_device *hv = to_hv(dev);
> > @@ -176,8 +176,6 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
> > hv_set_drvdata(hdev, NULL);
> >
> > vmbus_free_mmio(hv->mem->start, hv->fb_size);
> > -
> > - return 0;
> > }
> >
> > static int hyperv_vmbus_suspend(struct hv_device *hdev)
> > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> > index ab57b49a44ed..ef16c2a54362 100644
> > --- a/drivers/hid/hid-hyperv.c
> > +++ b/drivers/hid/hid-hyperv.c
> > @@ -535,7 +535,7 @@ static int mousevsc_probe(struct hv_device *device,
> > }
> >
> >
> > -static int mousevsc_remove(struct hv_device *dev)
> > +static void mousevsc_remove(struct hv_device *dev)
> > {
> > struct mousevsc_dev *input_dev = hv_get_drvdata(dev);
> >
> > @@ -544,8 +544,6 @@ static int mousevsc_remove(struct hv_device *dev)
> > hid_hw_stop(input_dev->hid_device);
> > hid_destroy_device(input_dev->hid_device);
> > mousevsc_free_device(input_dev);
> > -
> > - return 0;
> > }
> >
> > static int mousevsc_suspend(struct hv_device *dev)
> > diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > index cbe43e2567a7..0d4b8fc47ac6 100644
> > --- a/drivers/hv/hv_balloon.c
> > +++ b/drivers/hv/hv_balloon.c
> > @@ -2042,7 +2042,7 @@ static int balloon_probe(struct hv_device *dev,
> > return ret;
> > }
> >
> > -static int balloon_remove(struct hv_device *dev)
> > +static void balloon_remove(struct hv_device *dev)
> > {
> > struct hv_dynmem_device *dm = hv_get_drvdata(dev);
> > struct hv_hotadd_state *has, *tmp;
> > @@ -2083,8 +2083,6 @@ static int balloon_remove(struct hv_device *dev)
> > kfree(has);
> > }
> > spin_unlock_irqrestore(&dm_device.ha_lock, flags);
> > -
> > - return 0;
> > }
> >
> > static int balloon_suspend(struct hv_device *hv_dev)
> > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> > index 835e6039c186..24995ac41c86 100644
> > --- a/drivers/hv/hv_util.c
> > +++ b/drivers/hv/hv_util.c
> > @@ -602,7 +602,7 @@ static int util_probe(struct hv_device *dev,
> > return ret;
> > }
> >
> > -static int util_remove(struct hv_device *dev)
> > +static void util_remove(struct hv_device *dev)
> > {
> > struct hv_util_service *srv = hv_get_drvdata(dev);
> >
> > @@ -610,8 +610,6 @@ static int util_remove(struct hv_device *dev)
> > srv->util_deinit();
> > vmbus_close(dev->channel);
> > kfree(srv->recv_buffer);
> > -
> > - return 0;
> > }
> >
> > /*
> > diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> > index d62aefb2e245..31def6ce5157 100644
> > --- a/drivers/input/serio/hyperv-keyboard.c
> > +++ b/drivers/input/serio/hyperv-keyboard.c
> > @@ -369,7 +369,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
> > return error;
> > }
> >
> > -static int hv_kbd_remove(struct hv_device *hv_dev)
> > +static void hv_kbd_remove(struct hv_device *hv_dev)
> > {
> > struct hv_kbd_dev *kbd_dev = hv_get_drvdata(hv_dev);
> >
> > @@ -378,8 +378,6 @@ static int hv_kbd_remove(struct hv_device *hv_dev)
> > kfree(kbd_dev);
> >
> > hv_set_drvdata(hv_dev, NULL);
> > -
> > - return 0;
> > }
> >
> > static int hv_kbd_suspend(struct hv_device *hv_dev)
> > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> > index 89eb4f179a3c..025f805e1ed9 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -2594,7 +2594,7 @@ static int netvsc_probe(struct hv_device *dev,
> > return ret;
> > }
> >
> > -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,7 @@ 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;
> > + return;
> > }
> >
> > ndev_ctx = netdev_priv(net);
> > @@ -2637,7 +2637,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 583d3aad6908..e46d9a14053f 100644
> > --- a/drivers/pci/controller/pci-hyperv.c
> > +++ b/drivers/pci/controller/pci-hyperv.c
> > @@ -3813,13 +3813,10 @@ static int hv_pci_bus_exit(struct hv_device *hdev, bool keep_devs)
> > /**
> > * hv_pci_remove() - Remove routine for this VMBus channel
> > * @hdev: VMBus's tracking struct for this root PCI bus
> > - *
> > - * Return: 0 on success, -errno on failure
> > */
> > -static int hv_pci_remove(struct hv_device *hdev)
> > +static void hv_pci_remove(struct hv_device *hdev)
> > {
> > struct hv_pcibus_device *hbus;
> > - int ret;
> >
> > hbus = hv_get_drvdata(hdev);
> > if (hbus->state == hv_pcibus_installed) {
> > @@ -3842,7 +3839,7 @@ static int hv_pci_remove(struct hv_device *hdev)
> > pci_unlock_rescan_remove();
> > }
> >
> > - ret = hv_pci_bus_exit(hdev, false);
> > + hv_pci_bus_exit(hdev, false);
> >
> > vmbus_close(hdev->channel);
> >
> > @@ -3855,7 +3852,6 @@ static int hv_pci_remove(struct hv_device *hdev)
> > hv_put_dom_num(hbus->bridge->domain_nr);
> >
> > kfree(hbus);
> > - return ret;
> > }
> >
> > static int hv_pci_suspend(struct hv_device *hdev)
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 3c5b7e4227b2..02f9d1a6f4ac 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -2092,7 +2092,7 @@ static int storvsc_change_queue_depth(struct scsi_device *sdev, int queue_depth)
> > return scsi_change_queue_depth(sdev, queue_depth);
> > }
> >
> > -static int storvsc_remove(struct hv_device *dev)
> > +static void storvsc_remove(struct hv_device *dev)
> > {
> > struct storvsc_device *stor_device = hv_get_drvdata(dev);
> > struct Scsi_Host *host = stor_device->host;
> > @@ -2108,8 +2108,6 @@ static int storvsc_remove(struct hv_device *dev)
> > scsi_remove_host(host);
> > storvsc_dev_remove(dev);
> > scsi_host_put(host);
> > -
> > - return 0;
> > }
> >
> > static int storvsc_suspend(struct hv_device *hv_dev)
> > diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> > index c08a6cfd119f..20d9762331bd 100644
> > --- a/drivers/uio/uio_hv_generic.c
> > +++ b/drivers/uio/uio_hv_generic.c
> > @@ -355,20 +355,19 @@ hv_uio_probe(struct hv_device *dev,
> > return ret;
> > }
> >
> > -static int
> > +static void
> > hv_uio_remove(struct hv_device *dev)
> > {
> > struct hv_uio_private_data *pdata = hv_get_drvdata(dev);
> >
> > if (!pdata)
> > - return 0;
> > + return;
> >
> > sysfs_remove_bin_file(&dev->channel->kobj, &ring_buffer_bin_attr);
> > uio_unregister_device(&pdata->info);
> > hv_uio_cleanup(dev, pdata);
> >
> > vmbus_free_ring(dev->channel);
> > - return 0;
> > }
> >
> > static struct hv_driver hv_uio_drv = {
> > diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> > index 3ce746a46179..cfa6a7c1eeb3 100644
> > --- a/drivers/video/fbdev/hyperv_fb.c
> > +++ b/drivers/video/fbdev/hyperv_fb.c
> > @@ -1240,8 +1240,7 @@ static int hvfb_probe(struct hv_device *hdev,
> > return ret;
> > }
> >
> > -
> > -static int hvfb_remove(struct hv_device *hdev)
> > +static void hvfb_remove(struct hv_device *hdev)
> > {
> > struct fb_info *info = hv_get_drvdata(hdev);
> > struct hvfb_par *par = info->par;
> > @@ -1262,8 +1261,6 @@ static int hvfb_remove(struct hv_device *hdev)
> >
> > hvfb_putmem(hdev, info);
> > framebuffer_release(info);
> > -
> > - return 0;
> > }
> >
> > static int hvfb_suspend(struct hv_device *hdev)
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > index 85f7c5a63aa6..cd5cb9f6fae0 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -1273,7 +1273,7 @@ struct hv_driver {
> > } dynids;
> >
> > int (*probe)(struct hv_device *, const struct hv_vmbus_device_id *);
> > - int (*remove)(struct hv_device *);
> > + void (*remove)(struct hv_device *dev);
>
> nitpick for consistency from previous version
>
> void (*remove)(struct hv_device *);

In that way, checkpatch.pl will complain:

$ ./scripts/checkpatch.pl 0001-Drivers-hv-Make-remove-callback-of-hyperv-driver-voi.patch
WARNING: function definition argument 'struct hv_device *' should also have an identifier name
#288: FILE: include/linux/hyperv.h:1276:
+ void (*remove)(struct hv_device *);

total: 0 errors, 1 warnings, 0 checks, 209 lines checked

Thanks,
Dawei

>
>
> > void (*shutdown)(struct hv_device *);
> >
> > int (*suspend)(struct hv_device *);
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 59c3e2697069..7cb1a9d2cdb4 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -879,13 +879,11 @@ static int hvs_probe(struct hv_device *hdev,
> > return 0;
> > }
> >
> > -static int hvs_remove(struct hv_device *hdev)
> > +static void hvs_remove(struct hv_device *hdev)
> > {
> > struct vmbus_channel *chan = hdev->channel;
> >
> > vmbus_close(chan);
> > -
> > - return 0;
> > }
> >
> > /* hv_sock connections can not persist across hibernation, and all the hv_sock
>

2023-01-17 14:58:23

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3] Drivers: hv: Make remove callback of hyperv driver void returned

On Thu, Jan 05, 2023 at 10:51:23PM +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.
>
> As such, change the remove function for Hyper-V VMBus based
> drivers to return void.
>
> Signed-off-by: Dawei Li <[email protected]>

Applied to hyperv-next. Thanks.

Wei.