Re: [PATCH RFC] nvme/fc: sq flow control

[Martin Wilck <mwilck@suse.com>]

On Wed, 2020-02-26 at 00:59 +0100, Hannes Reinecke wrote:
> As per NVMe-oF spec sq flow control is actually mandatory, and we
> should
> be implementing it to avoid the controller to return a fatal status
> error, and try to play nicely with controllers using sq flow control
> to implement QoS.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/nvme/host/fc.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index a19ddb61039d..628397bd5065 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> ...
> @@ -2177,6 +2181,18 @@ nvme_fc_unmap_data(struct nvme_fc_ctrl *ctrl,
> struct request *rq,
>  	freq->sg_cnt = 0;
>  }
>  
> +static int nvme_fc_update_sq_tail(struct nvme_fc_queue *queue, int
> incr)
> +{
> +	int old_sqtl, new_sqtl;
> +
> +	do {
> +		old_sqtl = queue->sq_tail;
> +		new_sqtl = (old_sqtl + incr) % queue->ctrl-
> >ctrl.sqsize;
> +	} while (cmpxchg(&queue->sq_tail, old_sqtl, new_sqtl) !=
> +		 old_sqtl);

Shouldn't you use READ_ONCE() to update old_sqtl, or better even, use
the return value of cmpxchg()?

Regards
Martin



_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme