[1/6] hyperv: Make remove callback of hyperv driver void returned
Commit Message
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(-)
Comments
Hi Dawei,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on xen-tip/linux-next]
[also build test WARNING on linus/master v6.1-rc8 next-20221205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dawei-Li/hyperv-Make-remove-callback-of-hyperv-driver-void-returned/20221205-233927
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
patch link: https://lore.kernel.org/r/TYCP286MB232373567792ED1AC5E0849FCA189%40TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM
patch subject: [PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned
config: x86_64-rhel-8.3-syz
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/19dc60f1e539ba31e531b89e5f57b9ce584be2f1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dawei-Li/hyperv-Make-remove-callback-of-hyperv-driver-void-returned/20221205-233927
git checkout 19dc60f1e539ba31e531b89e5f57b9ce584be2f1
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/pci/controller/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/pci/controller/pci-hyperv.c: In function 'hv_pci_remove':
>> drivers/pci/controller/pci-hyperv.c:3822:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
3822 | int ret;
| ^~~
vim +/ret +3822 drivers/pci/controller/pci-hyperv.c
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3812
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3813 /**
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3814 * hv_pci_remove() - Remove routine for this VMBus channel
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3815 * @hdev: VMBus's tracking struct for this root PCI bus
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3816 *
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3817 * Return: 0 on success, -errno on failure
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3818 */
19dc60f1e539ba3 drivers/pci/controller/pci-hyperv.c Dawei Li 2022-12-05 3819 static void hv_pci_remove(struct hv_device *hdev)
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3820 {
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3821 struct hv_pcibus_device *hbus;
a8e37506e79a7a0 drivers/pci/controller/pci-hyperv.c Dexuan Cui 2019-11-24 @3822 int ret;
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3823
17978524a636d00 drivers/pci/host/pci-hyperv.c Dexuan Cui 2016-11-10 3824 hbus = hv_get_drvdata(hdev);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3825 if (hbus->state == hv_pcibus_installed) {
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3826 tasklet_disable(&hdev->channel->callback_event);
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3827 hbus->state = hv_pcibus_removing;
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3828 tasklet_enable(&hdev->channel->callback_event);
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3829 destroy_workqueue(hbus->wq);
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3830 hbus->wq = NULL;
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3831 /*
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3832 * At this point, no work is running or can be scheduled
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3833 * on hbus-wq. We can't race with hv_pci_devices_present()
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3834 * or hv_pci_eject_device(), it's safe to proceed.
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3835 */
94d22763207ac66 drivers/pci/controller/pci-hyperv.c Long Li 2021-05-12 3836
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3837 /* Remove the bus from PCI's point of view. */
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3838 pci_lock_rescan_remove();
418cb6c8e051119 drivers/pci/controller/pci-hyperv.c Arnd Bergmann 2021-07-27 3839 pci_stop_root_bus(hbus->bridge->bus);
15becc2b56c6eda drivers/pci/controller/pci-hyperv.c Dexuan Cui 2019-03-04 3840 hv_pci_remove_slots(hbus);
418cb6c8e051119 drivers/pci/controller/pci-hyperv.c Arnd Bergmann 2021-07-27 3841 pci_remove_root_bus(hbus->bridge->bus);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3842 pci_unlock_rescan_remove();
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3843 }
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3844
a8e37506e79a7a0 drivers/pci/controller/pci-hyperv.c Dexuan Cui 2019-11-24 3845 ret = hv_pci_bus_exit(hdev, false);
deb22e5c84c884a drivers/pci/host/pci-hyperv.c Vitaly Kuznetsov 2016-04-29 3846
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3847 vmbus_close(hdev->channel);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3848
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3849 iounmap(hbus->cfg_addr);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3850 hv_free_config_window(hbus);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3851 hv_pci_free_bridge_windows(hbus);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3852 irq_domain_remove(hbus->irq_domain);
9e7f9178ab4943b drivers/pci/controller/pci-hyperv.c Boqun Feng 2021-07-27 3853 irq_domain_free_fwnode(hbus->fwnode);
be700103efd1050 drivers/pci/controller/pci-hyperv.c Haiyang Zhang 2019-08-15 3854
38c0d266dc80b81 drivers/pci/controller/pci-hyperv.c Boqun Feng 2021-07-27 3855 hv_put_dom_num(hbus->bridge->domain_nr);
be700103efd1050 drivers/pci/controller/pci-hyperv.c Haiyang Zhang 2019-08-15 3856
877b911a5ba0733 drivers/pci/controller/pci-hyperv.c Dexuan Cui 2019-11-24 3857 kfree(hbus);
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3858 }
4daace0d8ce851f drivers/pci/host/pci-hyperv.c Jake Oshins 2016-02-16 3859
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.
@@ -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)
@@ -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)
@@ -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);
}
@@ -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;
}
/*
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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 = {
@@ -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)
@@ -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 *);
@@ -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