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.
> 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.
