[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/5] block/nvme: don't flip CQ p
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/5] block/nvme: don't flip CQ phase bits |
Date: |
Fri, 7 Jun 2019 15:28:43 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 6/7/19 7:08 AM, Paolo Bonzini wrote:
> On 06/06/19 23:23, John Snow wrote:
>> So: This looks right; does this fix a bug that can be observed? Do we
>> have any regression tests for block/NVMe?
>
> I don't think it fixes a bug; by the time the CQ entry is picked up by
> QEMU, the device is not supposed to touch it anymore.
>
> However, the idea behind the phase bits is that you can decide whether
> the driver has placed a completion in the queue. When we get here, we have
>
> le16_to_cpu(c->status) & 0x1) == !q->cq_phase
>
> On the next pass through the ring buffer q->cq_phase will be flipped,
> and thus when we see this element we'll get
>
> le16_to_cpu(c->status) & 0x1) == q->cq_phase
>
> and not process it. Since block/nvme.c flips the bit, this mechanism
> does not work and the loop termination relies on the other part of the
> condition, "if (!c->cid) break;".
>
> So the patch is correct, but it would also be nice to also either remove
> phase handling altogether, or check that the phase handling works
> properly and drop the !c->cid test.
>
> Paolo
>
Gotcha, I see, that's why it doesn't cause problems. Thanks :)
--js