2020-07-02 15:12:48

by Moshe Shemesh

[permalink] [raw]
Subject: [PATCH net-next 2/7] devlink: Rework devlink health reporter destructor

From: Vladyslav Tarasiuk <[email protected]>

Devlink keeps its own reference to every reporter in a list and inits
refcount to 1 upon reporter's creation. Existing destructor waits to
free the memory indefinitely using msleep() until all references except
devlink's own are put.

Rework this mechanism by moving memory free routine to a separate
function, which is called when the last reporter reference is put.

Besides, it allows to call __devlink_health_reporter_destroy() while
locked on a reporters list mutex in symmetry to
__devlink_health_reporter_create(), which is required in follow-up
patch.

Signed-off-by: Vladyslav Tarasiuk <[email protected]>
Reviewed-by: Moshe Shemesh <[email protected]>
Reviewed-by: Jiri Pirko <[email protected]>
---
net/core/devlink.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index dfd7fe2..dcf8006 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5353,6 +5353,29 @@ struct devlink_health_reporter *
}
EXPORT_SYMBOL_GPL(devlink_health_reporter_create);

+static void
+devlink_health_reporter_free(struct devlink_health_reporter *reporter)
+{
+ mutex_destroy(&reporter->dump_lock);
+ if (reporter->dump_fmsg)
+ devlink_fmsg_free(reporter->dump_fmsg);
+ kfree(reporter);
+}
+
+static void
+devlink_health_reporter_put(struct devlink_health_reporter *reporter)
+{
+ if (refcount_dec_and_test(&reporter->refcount))
+ devlink_health_reporter_free(reporter);
+}
+
+static void
+__devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+ list_del(&reporter->list);
+ devlink_health_reporter_put(reporter);
+}
+
/**
* devlink_health_reporter_destroy - destroy devlink health reporter
*
@@ -5362,14 +5385,8 @@ struct devlink_health_reporter *
devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
{
mutex_lock(&reporter->devlink->reporters_lock);
- list_del(&reporter->list);
+ __devlink_health_reporter_destroy(reporter);
mutex_unlock(&reporter->devlink->reporters_lock);
- while (refcount_read(&reporter->refcount) > 1)
- msleep(100);
- mutex_destroy(&reporter->dump_lock);
- if (reporter->dump_fmsg)
- devlink_fmsg_free(reporter->dump_fmsg);
- kfree(reporter);
}
EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);

@@ -5639,12 +5656,6 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
return NULL;
}

-static void
-devlink_health_reporter_put(struct devlink_health_reporter *reporter)
-{
- refcount_dec(&reporter->refcount);
-}
-
void
devlink_health_reporter_state_update(struct devlink_health_reporter *reporter,
enum devlink_health_reporter_state state)
--
1.8.3.1