diff mbox series

[RFC] net: core: support managed resource allocation in ndo_open

Message ID 1151799e-7faa-3d86-c610-9b9ebbd62637@gmail.com
State Superseded
Headers show
Series [RFC] net: core: support managed resource allocation in ndo_open | expand

Commit Message

Heiner Kallweit Nov. 11, 2020, 8:56 p.m. UTC
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(-)

Comments

Jakub Kicinski Nov. 12, 2020, 10:14 p.m. UTC | #1
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 mbox series

Patch

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);
 	}