Skip to content

Commit

Permalink
wwan: core: no more hold netdev ops owning module
Browse files Browse the repository at this point in the history
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>
Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Sergey Ryazanov authored and David S. Miller committed Jun 22, 2021
1 parent 322a0ba commit 9f0248e
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 15 deletions.
3 changes: 1 addition & 2 deletions drivers/net/mhi/net.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 0 additions & 10 deletions drivers/net/wwan/wwan_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)))
Expand All @@ -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 */
Expand All @@ -989,8 +981,6 @@ void wwan_unregister_ops(struct device *parent)

rtnl_unlock();

module_put(owner);

wwandev->ops_ctxt = NULL;
wwan_remove_dev(wwandev);
}
Expand Down
1 change: 0 additions & 1 deletion drivers/net/wwan/wwan_hwsim.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
2 changes: 0 additions & 2 deletions include/linux/wwan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 9f0248e

Please sign in to comment.