| Message ID | TYCP286MB232373567792ED1AC5E0849FCA189@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make remove() of any bus based driver void returned | expand |
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.
On Tue, Dec 06, 2022 at 11:37:26AM +0000, Wei Liu wrote: > 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> > [...] Hi Wei: Thanks for the review. > > -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. Nice catch, will fix it. > > > } > > > > 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. Sorry about the confusion. In short, the point of this commit is making remove() of _driver_ void returned. For bus-based driver, device removal is implemented as: 1 device_remove() => 2 bus->remove() => 3 driver->remove() 1 is void. For 2, commit fc7a6209d571 ("bus: Make remove callback return void") forces bus_type::remove be void-returned, which applies for hv_bus(vmbus) too. So it doesn't make sense for 3(driver->remove) to return non-void, in this case it's hv_driver. Thanks, Dawei > > Thanks, > Wei.
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 6c127f061f06..6bbd2e6fa3d4 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -1990,7 +1990,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; @@ -2031,8 +2031,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) @@ -2112,7 +2110,6 @@ static struct hv_driver balloon_drv = { static int __init init_balloon_drv(void) { - return vmbus_driver_register(&balloon_drv); } 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..50c20e4d4147 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,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; } 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 */ -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; @@ -3795,7 +3795,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 bc46721aa01c..d253e7d5959d 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -2093,7 +2093,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; @@ -2109,8 +2109,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 072ce07ba9e0..721b7c34a99e 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -1226,8 +1226,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; @@ -1248,8 +1247,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 3b42264333ef..1f128f28a487 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
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> --- drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +--- drivers/hid/hid-hyperv.c | 4 +--- drivers/hv/hv_balloon.c | 5 +---- drivers/hv/hv_util.c | 4 +--- drivers/input/serio/hyperv-keyboard.c | 4 +--- drivers/net/hyperv/netvsc_drv.c | 4 +--- drivers/pci/controller/pci-hyperv.c | 3 +-- 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, 13 insertions(+), 35 deletions(-)