nvme: set DMA_ATTR_WEAK_ORDERING attribute on dma buffers

SPARC based platforms suffer significant read performance penalties for nvme reads when Relaxed Ordering is not enabled. This change sets the DMA_ATTR_WEAK_ORDERING (Relaxed Ordering) attribute for nvme block dma buffers.

Both the AIC and SSD versions of the Samsung 2.9TB device were tested with fio using iodepth=32, numjobs=8, and bs=4k.

For reads, a 450% increase in IOPS was observed.
For writes, no change in IOPS was observed.

---
 drivers/nvme/host/pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 951042a375d6..7a6cf6fa95c2 100644
@@ -632,7 +632,7 @@ static int nvme_map_data(struct nvme_dev *dev, struct request *req,
 
 	ret = BLK_MQ_RQ_QUEUE_BUSY;
 	if (!dma_map_sg_attrs(dev->dev, iod->sg, iod->nents, dma_dir,
-				DMA_ATTR_NO_WARN))
+				DMA_ATTR_NO_WARN | DMA_ATTR_WEAK_ORDERING))
 		goto out;
 
 	if (!nvme_setup_prps(dev, req))

@@ -650,7 +650,8 @@ static int nvme_map_data(struct nvme_dev *dev, struct request *req,
 		if (rq_data_dir(req))
 			nvme_dif_remap(req, nvme_dif_prep);
 
-		if (!dma_map_sg(dev->dev, &iod->meta_sg, 1, dma_dir))
+		if (!dma_map_sg_attrs(dev->dev, &iod->meta_sg, 1, dma_dir,
+				DMA_ATTR_WEAK_ORDERING))
 			goto out_unmap;
 	}
 

1.8.3.1

Please explain what it does exactly, and why you think it's safe for NVMe, as that belongs into the changelog.

Bonus points for explaining why DMA_ATTR_WEAK_ORDERING shouldn't be the default behavior for the dma_map_ routines instead of sprinkling it over just about every driver..

Christoph,

Hopefully, this answers your questions.

The SPARC platforms are designed in accordance with PCIe specification and expects the the device driver to set DMA_ATTR_WEAK_ORDERING to avoid restrict ordering rules enforced by PCIe root-complex and ultimately experience higher latency for DMA Writes.

The Root Complex needs to break PCIe Memory Write request into one or more cacheline size requests to host memory. If RO is not set, those cacheline requests are serialized and the next/subsequent write cannot issue until the coherency acknowledgment for the first/earlier write is received. This serialization of write transactions can significantly impact sustained DMA write performance for the given device.

Relaxed ordering should be used as much as possible to avoid the serialization penalty described above. The risk of using weak ordering indiscriminately on all writes, however, is that certain devices and drivers may rely on strong ordering for certain writes. For example, a device may use a specific location in host memory to serve as a "done" flag. The device writes data to a buffer, and those writes can be relaxed because the order in which the data bytes enter the buffer is not visible to software. The device then writes to the "done" flag location, to indicate to the consumer that it has completed the buffer write. The write to "done" cannot pass the prior writes to the data buffer; otherwise software polling on the "done" location could potentially read stale data from the data buffer. Device driver developers need to understand the hardware behavior and only set the DMA_ATTR_WEAK_ORDERING attribute when safe.

Alan Adamson

I always assumed that our streaming mappings are relaxed order for TLP anyway. And at very least Documentation/DMA-attributes.txt seems to imply something different:

DMA_ATTR_WEAK_ORDERING
----------------------

DMA_ATTR_WEAK_ORDERING specifies that reads and writes to the mapping may be weakly ordered, that is that reads and writes may pass each other.

Which to me suggest host reads, which also makes me wonder why these would even apply to our streaming mappings.

Adding the powerpc folks that added the DMA_ATTR_WEAK_ORDERING flag originally back in 2008, but not actual users as far as I can tell - those are all new and from the sparc gang, except for the noveau driver, which is a bit older but only uses for dma_alloc_attrs, where the original description makes sense to me.

I suspect people have applied the same name for different things over time.

When we added it in 2008, there was one specific use case, improving throughput on infiniband, iirc using the mthca device driver. In the Cell IOMMU, this attribute causes concurrent DMA transfers on PCIe devices to disregard the stricter PCI ordering rules: multiple transfers could be initiated together and complete in an arbitrary order depending on e.g. I/O page faults in the IOMMU. Using the dma attributes, this could be controlled to apply only to certain transfers, so a completion queue DMA would still be ordered with regard to other transfers, as it would not come with that attribute.

There are two things that I don't remember unfortunately.

a) why this patch seems to have never made it into the mainline kernel:
http://lists.openfabrics.org/pipermail/general/2008-October/055006.html
We probably had it added to both OFED and the supported distros at the time (RHEL, Fedora, SLES, all in versions I don't remember any more)

b) How this relates to the PCIe relaxed ordering flag on DMA transfers. It's possible that the hardware later learned to correctly set the flag on the actual transfer to have the same effect, or it's possible that we had to add the dma_alloc_attr() interface specifically because the hardware could not do this for some but not other transfers.

Arnd

I put this in for SPARC. In our case the host bridge/RC itself follows very strict ordering unless the relaxed order bit is set in the TLP. This works great for devices that actually allow the driver to enable it. We however also have to support an infiniband card that does not support enabling this in the HW and thus in the TLP but is actually fine with relaxed order for the data buffers (ie the streaming I/O vs the coherent control buffers). In fact w/o relaxed order the performance is absolutely atrocious ... w/ exceeds x86. This flag enables the driver to signal to us when we map the buffer in the IOMMU to enable the relaxed order attribute for our HW.

-chrish

We'll really need to start writing down our semantics. As I said given how our streaming dma mappings (dma_map_single/page/sg) are defined I can't think of any way how relaxed vs strict ordering would matter for them, so just enabling it by default seems like the right thing in this case, instead of having to patch every PCIe driver people might ever use on sparc to work around your bridge.

Fair enough. This will get documented.

People get real squeamish about just turning on relaxed order for driver/devices we actually know something about and test let alone everything. I do agree with your interpretation of the streaming DMA semantics. That does not mean every driver writer did or does. :-)

To be clear, this isn't really a host bridge issue. Technically no driver that runs on SPARC needs this enabled -- even performance critical ... if they do the HW correctly. Again, we have a necessary performance critical device which does not correctly signal its okayness with relaxed order for non-control DMA. This feature of the host bridge is to enable a workaround for just these cases (deemed needed by past history). Said differently, lots of devices are made for the commodity market where shortcuts like this apparently do not affect performance but are critical to performance on large systems.

-chrish