[PATCH v2] nvmet: protect sqhd update by a lock

[Christoph Hellwig]

On Tue, Oct 17, 2017 at 07:19:30AM -0700, James Smart wrote:
> On 10/16/2017 11:43 PM, Christoph Hellwig wrote:
> > >   	if (status)
> > >   		nvmet_set_status(req, status);
> > > +	spin_lock_irqsave(&req->sq->sqhd_lock, flags);
> > >   	if (req->sq->size)
> > >   		req->sq->sqhd = (req->sq->sqhd + 1) % req->sq->size;
> > >   	req->rsp->sq_head = cpu_to_le16(req->sq->sqhd);
> > > +	spin_unlock_irqrestore(&req->sq->sqhd_lock, flags);
> > What performance impact does this have?   I'm really reluctant to put
> > an irq disabling spinlock into a hot path for a feature that
> > theoretically is in the spec but ignored by every host.
> > 
> > I'd much rather play games with cmpxchg or similar here.
> 
> It's not ignored by every host. Its ignored by the linux host. There are
> other hosts out there that follow the spec and pay attention to sqhd - and
> the linux target would like to be used with them.
> 
> I'm open to other alternatives, but in the past, with pci adapter interfaces
> with similar "ring" index updates, the increment followed by modulo has
> always proven to be an issue for such tricks.

Something like:

	do {
		old_sqhd = req->sq->sqhd;
		new_sqhd = (old + 1) % req->sq->size;
	} while (cmpxchg(&req->sq->sqhd, old_sqhd, new_sqhd) != old_sqhd);

should do the job.