Re: [PATCH net-next v3 02/11] devlink: Add callback to query for snapshot id before snapshot create
From: Jakub Kicinski <hidden>
Date: 2018-07-13 01:03:38
On Thu, 12 Jul 2018 15:13:09 +0300, Alex Vesker wrote:
To restrict the driver with the snapshot ID selection a new callback is introduced for the driver to get the snapshot ID before creating a new snapshot. This will also allow giving the same ID for multiple snapshots taken of different regions on the same time.
I'm not in position to criticize other people's commit messages :), but I find this one hard to parse. I think what you meant to say is that you add a helper for numbering the snapshot per-devlink instance. There is no callback to be seen here. You *prevent* from giving the same ID to multiple snapshot even if they are from different regions.
quoted hunk ↗ jump to hunk
diff --git a/net/core/devlink.c b/net/core/devlink.c index cac8561..6c92ddd 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c@@ -4193,6 +4193,27 @@ void devlink_region_destroy(struct devlink_region *region) } EXPORT_SYMBOL_GPL(devlink_region_destroy); +/** + * devlink_region_shapshot_id_get - get snapshot ID + * + * This callback should be called when adding a new snapshot, + * Driver should use the same id for multiple snapshots taken + * on multiple regions at the same time/by the same trigger. + * + * @devlink: devlink + */ +u32 devlink_region_shapshot_id_get(struct devlink *devlink) +{ + u32 id; + + mutex_lock(&devlink->lock); + id = ++devlink->snapshot_id;
Any reason not to use an IDA? The reuse may seem unlikely, OTOH IDA isn't going to cost much, so why risk it...
+ mutex_unlock(&devlink->lock); + + return id; +} +EXPORT_SYMBOL_GPL(devlink_region_shapshot_id_get);
Sorry for only spotting this now.