Re: [PATCH 08/15] devlink: add devres managed devlinkm_alloc and devlinkm_free

[Jiri Pirko <jiri@resnulli.us>]

Thu, Jan 30, 2020 at 11:59:03PM CET, jacob.e.keller@intel.com wrote:
>Add devres managed allocation functions for allocating a devlink
>instance. These can be used by device drivers based on the devres
>framework which want to allocate a devlink instance.
>
>For simplicity and to reduce churn in the devlink core code, the devres
>management works by creating a node with a double-pointer. The devlink
>instance is allocated using the normal devlink_alloc and released using
>the normal devlink_free.
>
>An alternative solution where the raw memory for devlink is allocated
>directly via devres_alloc could be done. Such an implementation would
>either significantly increase code duplication or code churn in order to
>refactor the setup from the allocation.
>
>The new devres managed allocation function will be used by the ice
>driver in a following change to implement initial devlink support.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>---
> include/net/devlink.h |  4 ++++
> lib/devres.c          |  1 +
> net/core/devlink.c    | 54 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 59 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 63e954241404..1c3540280396 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -858,11 +858,15 @@ struct ib_device;
> struct net *devlink_net(const struct devlink *devlink);
> void devlink_net_set(struct devlink *devlink, struct net *net);
> struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size);
>+struct devlink *devlinkm_alloc(struct device * dev,
>+			       const struct devlink_ops *ops,
>+			       size_t priv_size);
> int devlink_register(struct devlink *devlink, struct device *dev);
> void devlink_unregister(struct devlink *devlink);
> void devlink_reload_enable(struct devlink *devlink);
> void devlink_reload_disable(struct devlink *devlink);
> void devlink_free(struct devlink *devlink);
>+void devlinkm_free(struct device *dev, struct devlink *devlink);
> int devlink_port_register(struct devlink *devlink,
> 			  struct devlink_port *devlink_port,
> 			  unsigned int port_index);
>diff --git a/lib/devres.c b/lib/devres.c
>index 6ef51f159c54..239c81d40612 100644
>--- a/lib/devres.c
>+++ b/lib/devres.c
>@@ -5,6 +5,7 @@
> #include <linux/gfp.h>
> #include <linux/export.h>
> #include <linux/of_address.h>
>+#include <net/devlink.h>
> 
> enum devm_ioremap_type {
> 	DEVM_IOREMAP = 0,
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 574008c536fa..b2b855d12a11 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -6531,6 +6531,60 @@ void devlink_free(struct devlink *devlink)
> }
> EXPORT_SYMBOL_GPL(devlink_free);
> 
>+static void devres_devlink_release(struct device *dev, void *res)
>+{
>+	devlink_free(*(struct devlink **)res);
>+}
>+
>+static int devres_devlink_match(struct device *dev, void *res, void *data)
>+{
>+	return *(struct devlink **)res == data;
>+}
>+
>+/**
>+ * devlinkm_alloc - Allocate devlink instance managed by devres
>+ * @dev: device to allocate devlink for
>+ * @ops: devlink ops structure
>+ * @priv_size: size of private data portion
>+ *
>+ * Allocate a devlink instance and manage its release via devres.
>+ */
>+struct devlink *devlinkm_alloc(struct device *dev,

Why "devlinkm"? Looks like the usual prefix for this is "devm_"
So "devm_devlink_alloc/free"?


>+			       const struct devlink_ops *ops,
>+			       size_t priv_size)
>+{
>+	struct devlink **ptr, *devlink = NULL;
>+
>+	ptr = devres_alloc(devres_devlink_release, sizeof(*ptr), GFP_KERNEL);
>+	if (!ptr)
>+		return NULL;
>+
>+	devlink = devlink_alloc(ops, priv_size);
>+	if (devlink) {
>+		*ptr = devlink;
>+		devres_add(dev, ptr);
>+	} else {
>+		devres_free(ptr);
>+	}
>+
>+	return devlink;
>+}
>+EXPORT_SYMBOL_GPL(devlinkm_alloc);
>+
>+/**
>+ * devlinkm_free - Free devlink instance managed by devres
>+ * @dev: device to remove the allocated devlink from
>+ * @devlink: devlink instance to free
>+ *
>+ * Find and remove the devres node associated with the given devlink.
>+ */
>+void devlinkm_free(struct device *dev, struct devlink *devlink)
>+{
>+	WARN_ON(devres_release(dev, devres_devlink_release,
>+			       devres_devlink_match, devlink));
>+}
>+EXPORT_SYMBOL_GPL(devlinkm_free);
>+
> static void devlink_port_type_warn(struct work_struct *work)
> {
> 	WARN(true, "Type was not set for devlink port.");
>-- 
>2.25.0.rc1
>