[PATCH] nvme: avoid cqe corruption when update at the same time as read

Keith Busch keith.busch at intel.com
Thu Mar 10 07:07:46 PST 2016


On Thu, Mar 10, 2016 at 06:36:50AM -0800, Christoph Hellwig wrote:
> On Thu, Mar 10, 2016 at 12:08:48PM +0100, Marta Rybczynska wrote:
> > Looks like a good refactoring to me. However, it seems to me that in
> > nvme_cqe_valid we should be checking for phase, not for head.
> 
> Yeah, this doesn't work as-is.  I still think that non-obvious
> check should be split out into something.  Maybe just:
> 
> static inline bool nvme_cqe_valid(struct nvme_queue *nvmeq, u16 head, u16 phase)
> {
> 	return (le16_to_cpu(nvmeq->cqes[head].status) & 1) == phase;
> }

Looks good.

The code had copied the CQE since the initial commit, so just want to
mention a torn CQE read could cause real trouble only if the Phase is
out of sync with the Command ID. These are in the same DWORD and many
(most?) archs compile to read those 4-bytes atomically.

We certainly can't rely on that, so this is a good change.



More information about the Linux-nvme mailing list