Keith,

On 2/17/21 19:28, Keith Busch wrote:

On Wed, Feb 17, 2021 at 08:26:30AM +0100, Heiner Kallweit wrote:

quoted

On 17.02.2021 00:59, Chaitanya Kulkarni wrote:

quoted

On 2/14/21 08:30, Heiner Kallweit wrote:

quoted

Based on the description we should be fine using the less heavy-weight
smp barriers here. On x86 this would be compiler barriers only.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Can you share performance numbers ?

I stumbled across this code piece by chance and don't have hw to test it.
Having said that the change is based on code inspection only.
Unfortunately the barrier comment is very generic and doesn't mention
a problematic scenario. Also the commit message of 3a7afd8ee42a doesn't
mention a potential race. Most likely the barrier can be removed
completely. Helpful would be if someone could explain the potential race
in detail, means which reordering / variable accesses could be racy.
Then we would have a basis to talk about READ_ONCE/WRITE_ONCE vs.
smp barrier vs. dma barrier vs. full barrier.

The variables the driver was protecting no longer exist, so I also agree
the barriers should not be necessary.

Are you saying something like following is needed ?

From a3a73bd3943b479f82ff00582baf2c37c1afd8e8 Mon Sep 17 00:00:00 2001
From: Chaitanya Kulkarni <redacted>
Date: Wed, 17 Feb 2021 20:01:37 -0800
Subject: [PATCH] nvme-pci: remove the barriers

The variable which was protected by the barriers is removed in
The commit f6c4d97b0d82 (" nvme/pci: Remove last_cq_head").

Remove the barriers which was protecting the variable.

Signed-off-by: Chaitanya Kulkarni <redacted>
---
 drivers/nvme/host/pci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0045c5edf629..3729775f6a8a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c

@@ -1062,14 +1062,8 @@ static irqreturn_t nvme_irq(int irq, void *data)
     struct nvme_queue *nvmeq = data;
     irqreturn_t ret = IRQ_NONE;
 
-    /*
-     * The rmb/wmb pair ensures we see all updates from a previous run of
-     * the irq handler, even if that was on another CPU.
-     */
-    rmb();
     if (nvme_process_cq(nvmeq))
         ret = IRQ_HANDLED;
-    wmb();
 
     return ret;
 }

-- 
2.22.1

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme