[PATCH net-next v2 07/10] wwan: core: no more hold netdev ops owning module
From: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Date: 2021-06-21 22:51:21
Subsystem:
networking drivers, the rest, wwan drivers · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Loic Poulain, Sergey Ryazanov
The WWAN netdev ops owner holding was used to protect from the unexpected memory disappear. This approach causes a dependency cycle (driver -> core -> driver) and effectively prevents a WWAN driver unloading. E.g. WWAN hwsim could not be unloaded until all simulated devices are removed: ~# modprobe wwan_hwsim devices=2 ~# lsmod | grep wwan wwan_hwsim 16384 2 wwan 20480 1 wwan_hwsim ~# rmmod wwan_hwsim rmmod: ERROR: Module wwan_hwsim is in use ~# echo > /sys/kernel/debug/wwan_hwsim/hwsim0/destroy ~# echo > /sys/kernel/debug/wwan_hwsim/hwsim1/destroy ~# lsmod | grep wwan wwan_hwsim 16384 0 wwan 20480 1 wwan_hwsim ~# rmmod wwan_hwsim For a real device driver this will cause an inability to unload module until a served device is physically detached. Since the last commit we are removing all child netdev(s) when a driver unregister the netdev ops. This allows us to permit the driver unloading, since any sane driver will call ops unregistering on a device deinitialization. So, remove the holding of an ops owner to make it easier to unload a driver module. The owner field has also beed removed from the ops structure as there are no more users of this field. Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> --- v1 -> v2: * fix a comment before wwan_unregister_ops() in the mhi/net.c drivers/net/mhi/net.c | 3 +-- drivers/net/wwan/wwan_core.c | 10 ---------- drivers/net/wwan/wwan_hwsim.c | 1 - include/linux/wwan.h | 2 -- 4 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index 6aa753387372..ffd1c01b3f35 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c@@ -383,7 +383,6 @@ static void mhi_net_dellink(void *ctxt, struct net_device *ndev, } static const struct wwan_ops mhi_wwan_ops = { - .owner = THIS_MODULE, .priv_size = sizeof(struct mhi_net_dev), .setup = mhi_net_setup, .newlink = mhi_net_newlink,
@@ -436,7 +435,7 @@ static void mhi_net_remove(struct mhi_device *mhi_dev) struct mhi_net_dev *mhi_netdev = dev_get_drvdata(&mhi_dev->dev); struct mhi_controller *cntrl = mhi_dev->mhi_cntrl; - /* rtnetlink takes care of removing remaining links */ + /* WWAN core takes care of removing remaining links */ wwan_unregister_ops(&cntrl->mhi_dev->dev); if (create_default_iface)
diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index ec6a69b23dd1..b634a0ba1196 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c@@ -929,11 +929,6 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops, return -EBUSY; } - if (!try_module_get(ops->owner)) { - wwan_remove_dev(wwandev); - return -ENODEV; - } - wwandev->ops = ops; wwandev->ops_ctxt = ctxt;
@@ -960,7 +955,6 @@ static int wwan_child_dellink(struct device *dev, void *data) void wwan_unregister_ops(struct device *parent) { struct wwan_device *wwandev = wwan_dev_get_by_parent(parent); - struct module *owner; LIST_HEAD(kill_list); if (WARN_ON(IS_ERR(wwandev)))
@@ -976,8 +970,6 @@ void wwan_unregister_ops(struct device *parent) */ put_device(&wwandev->dev); - owner = wwandev->ops->owner; /* Preserve ops owner */ - rtnl_lock(); /* Prevent concurent netdev(s) creation/destroying */ /* Remove all child netdev(s), using batch removing */
@@ -989,8 +981,6 @@ void wwan_unregister_ops(struct device *parent) rtnl_unlock(); - module_put(owner); - wwandev->ops_ctxt = NULL; wwan_remove_dev(wwandev); }
diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
index c1e850b9c087..a8582a58a385 100644
--- a/drivers/net/wwan/wwan_hwsim.c
+++ b/drivers/net/wwan/wwan_hwsim.c@@ -94,7 +94,6 @@ static void wwan_hwsim_netdev_setup(struct net_device *ndev) } static const struct wwan_ops wwan_hwsim_wwan_rtnl_ops = { - .owner = THIS_MODULE, .priv_size = 0, /* No private data */ .setup = wwan_hwsim_netdev_setup, };
diff --git a/include/linux/wwan.h b/include/linux/wwan.h
index 34222230360c..e1981ea3a2fd 100644
--- a/include/linux/wwan.h
+++ b/include/linux/wwan.h@@ -128,14 +128,12 @@ void *wwan_port_get_drvdata(struct wwan_port *port); /** * struct wwan_ops - WWAN device ops - * @owner: module owner of the WWAN ops * @priv_size: size of private netdev data area * @setup: set up a new netdev * @newlink: register the new netdev * @dellink: remove the given netdev */ struct wwan_ops { - struct module *owner; unsigned int priv_size; void (*setup)(struct net_device *dev); int (*newlink)(void *ctxt, struct net_device *dev,
--
2.26.3