devlink: add function to take snapshot while locked
authorJacob Keller <jacob.e.keller@intel.com>
Thu, 26 Mar 2020 18:37:11 +0000 (11:37 -0700)
committerDavid S. Miller <davem@davemloft.net>
Fri, 27 Mar 2020 02:39:26 +0000 (19:39 -0700)
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_create function while already holding the
devlink instance lock.

Extract the logic of this function into a static function prefixed by
`__` to indicate that it is an internal helper function. Modify the
original function to be implemented in terms of the new locked
function.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/core/devlink.c

index 05a8d1e..5d6887b 100644 (file)
@@ -3768,6 +3768,52 @@ out_free_msg:
        nlmsg_free(msg);
 }
 
+/**
+ *     __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.
+ *     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
+ */
+static int
+__devlink_region_snapshot_create(struct devlink_region *region,
+                                u8 *data, u32 snapshot_id)
+{
+       struct devlink *devlink = region->devlink;
+       struct devlink_snapshot *snapshot;
+
+       lockdep_assert_held(&devlink->lock);
+
+       /* 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;
+
+       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)
 {
@@ -7753,42 +7799,12 @@ int devlink_region_snapshot_create(struct devlink_region *region,
                                   u8 *data, u32 snapshot_id)
 {
        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;
-
-       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(region, data, snapshot_id);
        mutex_unlock(&devlink->lock);
-       return 0;
 
-unlock:
-       mutex_unlock(&devlink->lock);
        return err;
 }
 EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);