Thread (62 messages) 62 messages, 4 authors, 2020-02-04

Re: [PATCH 02/15] devlink: add functions to take snapshot while locked

From: Jiri Pirko <jiri@resnulli.us>
Date: 2020-02-03 11:40:01

Thu, Jan 30, 2020 at 11:58:57PM CET, jacob.e.keller@intel.com wrote:
quoted hunk ↗ jump to hunk
A future change is going to add a new devlink command to request
a snapshot on demand. This function will want to call the
devlink_region_snapshot_id_get and devlink_region_snapshot_create
functions while already holding the devlink instance lock.

Extract the logic of these two functions into static functions with the
_locked postfix. Modify the original functions to be implemented in
terms of the new locked functions.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
net/core/devlink.c | 95 +++++++++++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 34 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index d1f7bfbf81da..faf4f4c5c539 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3761,6 +3761,63 @@ static void devlink_nl_region_notify(struct devlink_region *region,
	nlmsg_free(msg);
}

+/**
+ *	devlink_region_snapshot_id_get_locked - get snapshot ID
+ *
+ *	Returns a new snapshot id. Must be called while holding the
+ *	devlink instance lock.
+ */
+static u32 devlink_region_snapshot_id_get_locked(struct devlink *devlink)
__devlink_region_snapshot_id_get()

+{
+	return ++devlink->snapshot_id;
+}
+
+/**
+ *	devlink_region_snapshot_create_locked - create a new snapshot
+ *	This will add a new snapshot of a region. The snapshot
+ *	will be stored on the region struct and can be accessed
+ *	from devlink. This is useful for future	analyses of snapshots.
+ *	Multiple snapshots can be created on a region.
+ *	The @snapshot_id should be obtained using the getter function.
+ *
+ *	Must be called only while holding the devlink instance lock.
+ *
+ *	@region: devlink region of the snapshot
+ *	@data: snapshot data
+ *	@snapshot_id: snapshot id to be created
+ *	@destructor: pointer to destructor function to free data
+ */
+static int
+devlink_region_snapshot_create_locked(struct devlink_region *region,
__devlink_region_snapshot_create()

quoted hunk ↗ jump to hunk
+				      u8 *data, u32 snapshot_id,
+				      devlink_snapshot_data_dest_t *destructor)
+{
+	struct devlink_snapshot *snapshot;
+
+	/* check if region can hold one more snapshot */
+	if (region->cur_snapshots == region->max_snapshots)
+		return -ENOMEM;
+
+	if (devlink_region_snapshot_get_by_id(region, snapshot_id))
+		return -EEXIST;
+
+	snapshot = kzalloc(sizeof(*snapshot), GFP_KERNEL);
+	if (!snapshot)
+		return -ENOMEM;
+
+	snapshot->id = snapshot_id;
+	snapshot->region = region;
+	snapshot->data = data;
+	snapshot->data_destructor = destructor;
+
+	list_add_tail(&snapshot->list, &region->snapshot_list);
+
+	region->cur_snapshots++;
+
+	devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_NEW);
+	return 0;
+}
+
static void devlink_region_snapshot_del(struct devlink_region *region,
					struct devlink_snapshot *snapshot)
{
@@ -7611,7 +7668,7 @@ u32 devlink_region_snapshot_id_get(struct devlink *devlink)
	u32 id;

	mutex_lock(&devlink->lock);
-	id = ++devlink->snapshot_id;
+	id = devlink_region_snapshot_id_get_locked(devlink);
	mutex_unlock(&devlink->lock);

	return id;
@@ -7622,7 +7679,7 @@ EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get);
 *	devlink_region_snapshot_create - create a new snapshot
 *	This will add a new snapshot of a region. The snapshot
 *	will be stored on the region struct and can be accessed
- *	from devlink. This is useful for future	analyses of snapshots.
+ *	from devlink. This is useful for future analyses of snapshots.
What this hunk is about? :O


quoted hunk ↗ jump to hunk
 *	Multiple snapshots can be created on a region.
 *	The @snapshot_id should be obtained using the getter function.
 *
@@ -7636,43 +7693,13 @@ int devlink_region_snapshot_create(struct devlink_region *region,
				   devlink_snapshot_data_dest_t *data_destructor)
{
	struct devlink *devlink = region->devlink;
-	struct devlink_snapshot *snapshot;
	int err;

	mutex_lock(&devlink->lock);
-
-	/* check if region can hold one more snapshot */
-	if (region->cur_snapshots == region->max_snapshots) {
-		err = -ENOMEM;
-		goto unlock;
-	}
-
-	if (devlink_region_snapshot_get_by_id(region, snapshot_id)) {
-		err = -EEXIST;
-		goto unlock;
-	}
-
-	snapshot = kzalloc(sizeof(*snapshot), GFP_KERNEL);
-	if (!snapshot) {
-		err = -ENOMEM;
-		goto unlock;
-	}
-
-	snapshot->id = snapshot_id;
-	snapshot->region = region;
-	snapshot->data = data;
-	snapshot->data_destructor = data_destructor;
-
-	list_add_tail(&snapshot->list, &region->snapshot_list);
-
-	region->cur_snapshots++;
-
-	devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_NEW);
+	err = devlink_region_snapshot_create_locked(region, data, snapshot_id,
+						    data_destructor);
	mutex_unlock(&devlink->lock);
-	return 0;

-unlock:
-	mutex_unlock(&devlink->lock);
	return err;
}
EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);
-- 
2.25.0.rc1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help