ACK/cmnt: [SRU][F:linux-bluefield][PATCH 1/1] UBUNTU: SAUCE: pka: Handle ring open scenario when rings are busy

[Kleber Souza]

On 19.02.21 21:28, Mahantesh Salimath wrote:
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux-bluefield/+bug/1916289

The BugLink should use the short format:

https://bugs.launchpad.net/bugs/1916289

> 
>      * Introduce mutex lock/unlock when updating
>        ring status while open/close ring device. If ring status
>        is busy while open operation then return -EBUSY error.
> 
>      * If -EBUSY error is received while trying to open ring,
>        this means the ring is being used by another application.
>        Hence, continue to look for other rings that might be available.
>        If ring open fails then no need to close the ring as this might affect
>        other 'pka' instances using that ring.
> 
>      * Also, first check if the ring is available (by opening the ring)
>        before initializing the ring region. This will remove the need to reset
>        ring region if ring is not available.
> 
> Signed-off-by: Mahantesh Salimath <mahantesh at nvidia.com>
> Reviewed-by: Khalil Blaiech <kblaiech at nvidia.com>
> Signed-off-by: Mahantesh Salimath <mahantesh at nvidia.com>

The BugLink issue can be fixed when applying the patch, it doesn't need to be
re-sent. Apart from that the changes look good.

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> 
> diff --git a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.c b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.c
> index 74512ba..4539bae 100644
> --- a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.c
> +++ b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.c
> @@ -339,6 +339,7 @@ static int pka_dev_init_ring(pka_dev_ring_t *ring, uint32_t ring_id,
>           return -ENOMEM;
>       }
> 
> +    mutex_init(&ring->mutex);
>       ring->status  = PKA_DEV_RING_STATUS_INITIALIZED;
>   
>       return ret;
> @@ -1707,20 +1708,35 @@ int __pka_dev_open_ring(uint32_t ring_id)
>   
>       shim = ring->shim;
>   
> +    mutex_lock(&ring->mutex);
> +
>       if (shim->status == PKA_SHIM_STATUS_UNDEFINED ||
>             shim->status == PKA_SHIM_STATUS_CREATED ||
>             shim->status == PKA_SHIM_STATUS_FINALIZED)
> -        return -EPERM;
> +    {
> +        ret = -EPERM;
> +        goto unlock_return;
> +    }
> +
> +    if (ring->status == PKA_DEV_RING_STATUS_BUSY)
> +    {
> +        ret = -EBUSY;
> +        goto unlock_return;
> +    }
>   
>       if (ring->status != PKA_DEV_RING_STATUS_INITIALIZED)
> -        return -EPERM;
> +    {
> +        ret = -EPERM;
> +        goto unlock_return;
> +    }
>   
>       // Set ring information words.
>       ret = pka_dev_set_ring_info(ring);
>       if (ret)
>       {
>           PKA_ERROR(PKA_DEV, "failed to set ring information\n");
> -        return -EWOULDBLOCK;
> +        ret = -EWOULDBLOCK;
> +        goto unlock_return;
>       }
>   
>       if (shim->busy_ring_num == 0)
> @@ -1729,6 +1745,8 @@ int __pka_dev_open_ring(uint32_t ring_id)
>       ring->status = PKA_DEV_RING_STATUS_BUSY;
>       shim->busy_ring_num += 1;
>   
> +unlock_return:
> +    mutex_unlock(&ring->mutex);
>       return ret;
>   }
>   
> @@ -1755,9 +1773,14 @@ int __pka_dev_close_ring(uint32_t ring_id)
>   
>       shim = ring->shim;
>   
> +    mutex_lock(&ring->mutex);
> +
>       if (shim->status != PKA_SHIM_STATUS_RUNNING &&
>               ring->status != PKA_DEV_RING_STATUS_BUSY)
> -        return -EPERM;
> +    {
> +        ret = -EPERM;
> +        goto unlock_return;
> +    }
>   
>       ring->status         = PKA_DEV_RING_STATUS_INITIALIZED;
>       shim->busy_ring_num -= 1;
> @@ -1765,6 +1788,8 @@ int __pka_dev_close_ring(uint32_t ring_id)
>       if (shim->busy_ring_num == 0)
>           shim->status = PKA_SHIM_STATUS_STOPPED;
>   
> +unlock_return:
> +    mutex_unlock(&ring->mutex);
>       return ret;
>   }
>   
> diff --git a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.h b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.h
> index 12545ae..5a6cc8fd 100644
> --- a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.h
> +++ b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_dev.h
> @@ -128,6 +128,8 @@ typedef struct
>       uint32_t                num_cmd_desc;   ///< number of command descriptors.
>   
>       int8_t                  status;         ///< status of the ring.
> +
> +    struct mutex            mutex;          ///< mutex lock for sharing ring device
>   } pka_dev_ring_t;
>   
>   /// defines for pka_dev_ring->status
> diff --git a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> index 36dfed0..f93efb66 100644
> --- a/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> +++ b/drivers/platform/mellanox/mlxbf_pka/mlxbf_pka_drv.c
> @@ -270,20 +270,20 @@ static int pka_drv_ring_open(void *device_data)
>   	if (!try_module_get(info->module))
>   		return -ENODEV;
>   
> -	/* Initialize regions */
> -	error = pka_drv_ring_regions_init(ring_dev);
> +	ring_info.ring_id = ring_dev->device_id;
> +	error = pka_dev_open_ring(&ring_info);
>   	if (error) {
> -		PKA_ERROR(PKA_DRIVER, "failed to initialize regions\n");
> +		PKA_DEBUG(PKA_DRIVER,
> +			  "failed to open ring %u\n", ring_dev->device_id);
>   		module_put(info->module);
>   		return error;
>   	}
>   
> -	ring_info.ring_id = ring_dev->device_id;
> -	error = pka_dev_open_ring(&ring_info);
> +	/* Initialize regions */
> +	error = pka_drv_ring_regions_init(ring_dev);
>   	if (error) {
> -		PKA_ERROR(PKA_DRIVER,
> -			  "failed to open ring %u\n", ring_dev->device_id);
> -		pka_drv_ring_regions_cleanup(ring_dev);
> +		PKA_DEBUG(PKA_DRIVER, "failed to initialize regions\n");
> +		pka_dev_close_ring(&ring_info);
>   		module_put(info->module);
>   		return error;
>   	}
> @@ -303,14 +303,15 @@ static void pka_drv_ring_release(void *device_data)
>   		  "release ring device %u (device_data:%p)\n",
>   		  ring_dev->device_id, ring_dev);
>   
> +	pka_drv_ring_regions_cleanup(ring_dev);
> +
>   	ring_info.ring_id = ring_dev->device_id;
>   	error = pka_dev_close_ring(&ring_info);
>   	if (error)
> -		PKA_ERROR(PKA_DRIVER,
> +		PKA_DEBUG(PKA_DRIVER,
>   			  "failed to close ring %u\n",
>   			  ring_dev->device_id);
>   
> -	pka_drv_ring_regions_cleanup(ring_dev);
>   	module_put(info->module);
>   }
>   
>