Commit 7ef19d3b authored by Jacob Keller's avatar Jacob Keller Committed by David S. Miller

devlink: report error once U32_MAX snapshot ids have been used

The devlink_snapshot_id_get() function returns a snapshot id. The
snapshot id is a u32, so there is no way to indicate an error code.

A future change is going to possibly add additional cases where this
function could fail. Refactor the function to return the snapshot id in
an argument, so that it can return zero or an error value.

This ensures that snapshot ids cannot be confused with error values, and
aids in the future refactor of snapshot id allocation management.

Because there is no current way to release previously used snapshot ids,
add a simple check ensuring that an error is reported in case the
snapshot_id would over flow.
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Reviewed-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7000108f
...@@ -169,6 +169,7 @@ int mlx4_crdump_collect(struct mlx4_dev *dev) ...@@ -169,6 +169,7 @@ int mlx4_crdump_collect(struct mlx4_dev *dev)
struct pci_dev *pdev = dev->persist->pdev; struct pci_dev *pdev = dev->persist->pdev;
unsigned long cr_res_size; unsigned long cr_res_size;
u8 __iomem *cr_space; u8 __iomem *cr_space;
int err;
u32 id; u32 id;
if (!dev->caps.health_buffer_addrs) { if (!dev->caps.health_buffer_addrs) {
...@@ -189,10 +190,14 @@ int mlx4_crdump_collect(struct mlx4_dev *dev) ...@@ -189,10 +190,14 @@ int mlx4_crdump_collect(struct mlx4_dev *dev)
return -ENODEV; return -ENODEV;
} }
crdump_enable_crspace_access(dev, cr_space);
/* Get the available snapshot ID for the dumps */ /* Get the available snapshot ID for the dumps */
id = devlink_region_snapshot_id_get(devlink); err = devlink_region_snapshot_id_get(devlink, &id);
if (err) {
mlx4_err(dev, "crdump: devlink get snapshot id err %d\n", err);
return err;
}
crdump_enable_crspace_access(dev, cr_space);
/* Try to capture dumps */ /* Try to capture dumps */
mlx4_crdump_collect_crspace(dev, cr_space, id); mlx4_crdump_collect_crspace(dev, cr_space, id);
......
...@@ -54,7 +54,11 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file, ...@@ -54,7 +54,11 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE); get_random_bytes(dummy_data, NSIM_DEV_DUMMY_REGION_SIZE);
id = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev)); err = devlink_region_snapshot_id_get(priv_to_devlink(nsim_dev), &id);
if (err) {
pr_err("Failed to get snapshot id\n");
return err;
}
err = devlink_region_snapshot_create(nsim_dev->dummy_region, err = devlink_region_snapshot_create(nsim_dev->dummy_region,
dummy_data, id); dummy_data, id);
if (err) { if (err) {
......
...@@ -976,7 +976,7 @@ devlink_region_create(struct devlink *devlink, ...@@ -976,7 +976,7 @@ devlink_region_create(struct devlink *devlink,
const struct devlink_region_ops *ops, const struct devlink_region_ops *ops,
u32 region_max_snapshots, u64 region_size); u32 region_max_snapshots, u64 region_size);
void devlink_region_destroy(struct devlink_region *region); void devlink_region_destroy(struct devlink_region *region);
u32 devlink_region_snapshot_id_get(struct devlink *devlink); int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id);
int devlink_region_snapshot_create(struct devlink_region *region, int devlink_region_snapshot_create(struct devlink_region *region,
u8 *data, u32 snapshot_id); u8 *data, u32 snapshot_id);
int devlink_info_serial_number_put(struct devlink_info_req *req, int devlink_info_serial_number_put(struct devlink_info_req *req,
......
...@@ -3771,14 +3771,22 @@ static void devlink_nl_region_notify(struct devlink_region *region, ...@@ -3771,14 +3771,22 @@ static void devlink_nl_region_notify(struct devlink_region *region,
/** /**
* __devlink_region_snapshot_id_get - get snapshot ID * __devlink_region_snapshot_id_get - get snapshot ID
* @devlink: devlink instance * @devlink: devlink instance
* @id: storage to return snapshot id
* *
* Returns a new snapshot id. Must be called while holding the * Allocates a new snapshot id. Returns zero on success, or a negative
* devlink instance lock. * error on failure. Must be called while holding the devlink instance
* lock.
*/ */
static u32 __devlink_region_snapshot_id_get(struct devlink *devlink) static int __devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id)
{ {
lockdep_assert_held(&devlink->lock); lockdep_assert_held(&devlink->lock);
return ++devlink->snapshot_id;
if (devlink->snapshot_id >= U32_MAX)
return -ENOSPC;
*id = ++devlink->snapshot_id;
return 0;
} }
/** /**
...@@ -7782,17 +7790,20 @@ EXPORT_SYMBOL_GPL(devlink_region_destroy); ...@@ -7782,17 +7790,20 @@ EXPORT_SYMBOL_GPL(devlink_region_destroy);
* Driver should use the same id for multiple snapshots taken * Driver should use the same id for multiple snapshots taken
* on multiple regions at the same time/by the same trigger. * on multiple regions at the same time/by the same trigger.
* *
* Returns zero on success, or a negative error code on failure.
*
* @devlink: devlink * @devlink: devlink
* @id: storage to return id
*/ */
u32 devlink_region_snapshot_id_get(struct devlink *devlink) int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id)
{ {
u32 id; int err;
mutex_lock(&devlink->lock); mutex_lock(&devlink->lock);
id = __devlink_region_snapshot_id_get(devlink); err = __devlink_region_snapshot_id_get(devlink, id);
mutex_unlock(&devlink->lock); mutex_unlock(&devlink->lock);
return id; return err;
} }
EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get); EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment