Commit 9f0248ea authored by Sergey Ryazanov's avatar Sergey Ryazanov Committed by David S. Miller

wwan: core: no more hold netdev ops owning module

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: default avatarSergey Ryazanov <ryazanov.s.a@gmail.com>
Reviewed-by: default avatarLoic Poulain <loic.poulain@linaro.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 322a0ba9
...@@ -383,7 +383,6 @@ static void mhi_net_dellink(void *ctxt, struct net_device *ndev, ...@@ -383,7 +383,6 @@ static void mhi_net_dellink(void *ctxt, struct net_device *ndev,
} }
static const struct wwan_ops mhi_wwan_ops = { static const struct wwan_ops mhi_wwan_ops = {
.owner = THIS_MODULE,
.priv_size = sizeof(struct mhi_net_dev), .priv_size = sizeof(struct mhi_net_dev),
.setup = mhi_net_setup, .setup = mhi_net_setup,
.newlink = mhi_net_newlink, .newlink = mhi_net_newlink,
...@@ -436,7 +435,7 @@ static void mhi_net_remove(struct mhi_device *mhi_dev) ...@@ -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_net_dev *mhi_netdev = dev_get_drvdata(&mhi_dev->dev);
struct mhi_controller *cntrl = mhi_dev->mhi_cntrl; 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); wwan_unregister_ops(&cntrl->mhi_dev->dev);
if (create_default_iface) if (create_default_iface)
......
...@@ -929,11 +929,6 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops, ...@@ -929,11 +929,6 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
return -EBUSY; return -EBUSY;
} }
if (!try_module_get(ops->owner)) {
wwan_remove_dev(wwandev);
return -ENODEV;
}
wwandev->ops = ops; wwandev->ops = ops;
wwandev->ops_ctxt = ctxt; wwandev->ops_ctxt = ctxt;
...@@ -960,7 +955,6 @@ static int wwan_child_dellink(struct device *dev, void *data) ...@@ -960,7 +955,6 @@ static int wwan_child_dellink(struct device *dev, void *data)
void wwan_unregister_ops(struct device *parent) void wwan_unregister_ops(struct device *parent)
{ {
struct wwan_device *wwandev = wwan_dev_get_by_parent(parent); struct wwan_device *wwandev = wwan_dev_get_by_parent(parent);
struct module *owner;
LIST_HEAD(kill_list); LIST_HEAD(kill_list);
if (WARN_ON(IS_ERR(wwandev))) if (WARN_ON(IS_ERR(wwandev)))
...@@ -976,8 +970,6 @@ void wwan_unregister_ops(struct device *parent) ...@@ -976,8 +970,6 @@ void wwan_unregister_ops(struct device *parent)
*/ */
put_device(&wwandev->dev); put_device(&wwandev->dev);
owner = wwandev->ops->owner; /* Preserve ops owner */
rtnl_lock(); /* Prevent concurent netdev(s) creation/destroying */ rtnl_lock(); /* Prevent concurent netdev(s) creation/destroying */
/* Remove all child netdev(s), using batch removing */ /* Remove all child netdev(s), using batch removing */
...@@ -989,8 +981,6 @@ void wwan_unregister_ops(struct device *parent) ...@@ -989,8 +981,6 @@ void wwan_unregister_ops(struct device *parent)
rtnl_unlock(); rtnl_unlock();
module_put(owner);
wwandev->ops_ctxt = NULL; wwandev->ops_ctxt = NULL;
wwan_remove_dev(wwandev); wwan_remove_dev(wwandev);
} }
......
...@@ -94,7 +94,6 @@ static void wwan_hwsim_netdev_setup(struct net_device *ndev) ...@@ -94,7 +94,6 @@ static void wwan_hwsim_netdev_setup(struct net_device *ndev)
} }
static const struct wwan_ops wwan_hwsim_wwan_rtnl_ops = { static const struct wwan_ops wwan_hwsim_wwan_rtnl_ops = {
.owner = THIS_MODULE,
.priv_size = 0, /* No private data */ .priv_size = 0, /* No private data */
.setup = wwan_hwsim_netdev_setup, .setup = wwan_hwsim_netdev_setup,
}; };
......
...@@ -128,14 +128,12 @@ void *wwan_port_get_drvdata(struct wwan_port *port); ...@@ -128,14 +128,12 @@ void *wwan_port_get_drvdata(struct wwan_port *port);
/** /**
* struct wwan_ops - WWAN device ops * struct wwan_ops - WWAN device ops
* @owner: module owner of the WWAN ops
* @priv_size: size of private netdev data area * @priv_size: size of private netdev data area
* @setup: set up a new netdev * @setup: set up a new netdev
* @newlink: register the new netdev * @newlink: register the new netdev
* @dellink: remove the given netdev * @dellink: remove the given netdev
*/ */
struct wwan_ops { struct wwan_ops {
struct module *owner;
unsigned int priv_size; unsigned int priv_size;
void (*setup)(struct net_device *dev); void (*setup)(struct net_device *dev);
int (*newlink)(void *ctxt, struct net_device *dev, int (*newlink)(void *ctxt, struct net_device *dev,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment