[PATCH] nvmet: protect sqhd update by a lock

[James Smart]

The real issue, where memory barrier isn't enough, is between the value 
of the sqhd increment followed by the modulo. Many of a time I've seen 
interruption in between these points.

As far as assignment of the cqe - as long a sqhd is getting updated 
properly, such that the cqe will return either the value just set  or a 
newer one, even if another cqe returns the same newer value, it's fine 
per the sqhd definition in nvme.

As it's no big deal - I'll repost with the lock moved

-- james


On 10/16/2017 7:44 AM, Johannes Thumshirn wrote:
> On Mon, Oct 16, 2017 at 07:37:48AM -0700, James Smart wrote:
>> Agree, is a little awkward. But works fine. The re-read will contain at
>> least the value that it was updated to under the lock. If it's actually the
>> value of a simultaneous update that is one further, that's actually fine too
>> - as sqhd isn't 1:1 with a cqe. Yes a cqe must contain the sqhd, but sqhd
>> can increment independently from a particuluar cqe. Key is that sqhd itself
>> must increment sanely so that whenever stamped in a cqe it's valid.
>>
>> I have no problem recutting with the lock around the assignment if desired.
> This to me sounds like a lock wasn't the right choice but a memory barrier
> would be needed. But OTOH I'm by no means a locking expert. What I _think_ is
> going on is, spin_lock_irqsave() is called, all the preempt_disable() and
> local_irq_save() (which are expensive) do their work and thus stretch timing
> enough that the desired effect is in place.
>
> But this is all guesswork.
>
> Byte,
> 	Johannes
>