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, ®ion->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, ®ion->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