| Message ID | 1151799e-7faa-3d86-c610-9b9ebbd62637@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC] net: core: support managed resource allocation in ndo_open | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 7710 this patch: 7710 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 53 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8075 this patch: 8075 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Wed, 11 Nov 2020 21:56:10 +0100 Heiner Kallweit wrote: > Quite often certain resources (irq, bigger chunks of memory) are > allocated not at probe time but in ndo_open. This requires to relaese > such resources in the right order in ndo_stop(), and in ndo_open() > error paths. Having said that the requirements are basically the same > as for releasing probe-time allocated resources in remove callback > and probe error paths. > So why not use the same mechanism to faciliate this? We have a big > number of device-managed resource allocation functions, so all we > need is a device suited for managed ndo_open resource allocation. > This RFC patch adds such a device to struct net_device. All we need > is a dozen lines of code. Resources then can be allocated with e.g. > > struct device *devm = &dev->devres_up; > devm_kzalloc(devm, size, gfp); > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> This will not work well with the best known practice on how to change rings parameters at runtime (allocate new set, swap, free old set). Personally I'm not a fan of the managed stuff, and I think neither is Dave. It just makes code harder to prove correct.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 72643c193..1fd2c1f3d 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1803,6 +1803,7 @@ enum netdev_priv_flags { * @garp_port: GARP * @mrp_port: MRP * + * @devres_up: for managed resource allocation in ndo_open() * @dev: Class/net/name entry * @sysfs_groups: Space for optional device, statistics and wireless * sysfs groups @@ -2121,6 +2122,7 @@ struct net_device { struct mrp_port __rcu *mrp_port; #endif + struct device devres_up; struct device dev; const struct attribute_group *sysfs_groups[4]; const struct attribute_group *sysfs_rx_queue_group; diff --git a/net/core/dev.c b/net/core/dev.c index 81abc4f98..f2c345579 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1488,6 +1488,11 @@ void netdev_notify_peers(struct net_device *dev) } EXPORT_SYMBOL(netdev_notify_peers); +static void netdev_release_devres_up(struct device *dev) +{ + memset(dev, 0, sizeof(*dev)); +} + static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack) { const struct net_device_ops *ops = dev->netdev_ops; @@ -1519,14 +1524,18 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack) if (ops->ndo_validate_addr) ret = ops->ndo_validate_addr(dev); + device_initialize(&dev->devres_up); + dev->devres_up.release = netdev_release_devres_up; + if (!ret && ops->ndo_open) ret = ops->ndo_open(dev); netpoll_poll_enable(dev); - if (ret) + if (ret) { + put_device(&dev->devres_up); clear_bit(__LINK_STATE_START, &dev->state); - else { + } else { dev->flags |= IFF_UP; dev_set_rx_mode(dev); dev_activate(dev); @@ -1606,6 +1615,8 @@ static void __dev_close_many(struct list_head *head) if (ops->ndo_stop) ops->ndo_stop(dev); + put_device(&dev->devres_up); + dev->flags &= ~IFF_UP; netpoll_poll_enable(dev); }
Quite often certain resources (irq, bigger chunks of memory) are allocated not at probe time but in ndo_open. This requires to relaese such resources in the right order in ndo_stop(), and in ndo_open() error paths. Having said that the requirements are basically the same as for releasing probe-time allocated resources in remove callback and probe error paths. So why not use the same mechanism to faciliate this? We have a big number of device-managed resource allocation functions, so all we need is a device suited for managed ndo_open resource allocation. This RFC patch adds such a device to struct net_device. All we need is a dozen lines of code. Resources then can be allocated with e.g. struct device *devm = &dev->devres_up; devm_kzalloc(devm, size, gfp); Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- include/linux/netdevice.h | 2 ++ net/core/dev.c | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)