[PATCH V2 0/2] nvme-pci: remove the barriers in nvme_irq()

[PATCH V2 0/2] nvme-pci: remove the barriers in nvme_irq()

[Chaitanya Kulkarni]

Hi,

The first patch removes barrier from the nvme_irq() and second patch
does the code cleanup for the same.

Below is the test log for the fio verify.

-ck

Chaitanya Kulkarni (2):
  nvme-pci; remove the barriers nvme_irq()
  nvme-pci: cleanup nvme_irq()

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

# gitlog -2
157098245bab (HEAD -> nvme-5.12) nvme-pci: cleanup nvme_irq()
42d531edfe67 nvme-pci; remove the barriers nvme_irq()
# ./compile_nvme.sh 
+ umount /mnt/nvme0n1
+ clear_dmesg
umount: /mnt/nvme0n1: not mounted
+ modprobe -r nvme-fabrics
+ modprobe -r nvme_loop
+ modprobe -r nvmet
+ modprobe -r nvme
+ sleep 1
+ modprobe -r nvme-core
+ lsmod
+ grep nvme
+ git apply ./all-fixes.diff
+ git diff
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..cedf704adac6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1167,7 +1167,8 @@ static int nvme_submit_user_cmd(struct request_queue *q,
                if (ret)
                        goto out;
                bio = req->bio;
-               bio_set_dev(bio, bdev);
+               if(bdev)
+                       bio_set_dev(bio, bdev);
                if (bdev && meta_buffer && meta_len) {
                        meta = nvme_add_user_metadata(bio, meta_buffer, meta_len,
                                        meta_seed, write);
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index cb6f86572b24..38fffee6b85e 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -688,7 +688,8 @@ static struct nvmf_transport_ops nvme_loop_transport = {
        .name           = "loop",
        .module         = THIS_MODULE,
        .create_ctrl    = nvme_loop_create_ctrl,
-       .allowed_opts   = NVMF_OPT_TRADDR,
+       .allowed_opts   = NVMF_OPT_TRADDR | NVMF_OPT_CTRL_LOSS_TMO,
+
 };
 
 static int __init nvme_loop_init_module(void)
++ nproc
+ make -j 64 M=drivers/nvme/ modules
+ HOST=drivers/nvme/host
+ TARGET=drivers/nvme/target
++ uname -r
+ HOST_DEST=/lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/host/
++ uname -r
+ TARGET_DEST=/lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/target/
+ cp drivers/nvme/host/nvme-core.ko drivers/nvme/host/nvme-fabrics.ko drivers/nvme/host/nvme-fc.ko drivers/nvme/host/nvme.ko drivers/nvme/host/nvme-rdma.ko drivers/nvme/host/nvme-tcp.ko /lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/host//
+ cp drivers/nvme/target/nvme-fcloop.ko drivers/nvme/target/nvme-loop.ko drivers/nvme/target/nvmet-fc.ko drivers/nvme/target/nvmet.ko drivers/nvme/target/nvmet-rdma.ko drivers/nvme/target/nvmet-tcp.ko /lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/target//
+ ls -lrth /lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/host/ /lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/target//
/lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/host/:
total 8.0M
-rw-r--r--. 1 root 3.0M Feb 23 12:30 nvme-core.ko
-rw-r--r--. 1 root 643K Feb 23 12:30 nvme-fabrics.ko
-rw-r--r--. 1 root 1.2M Feb 23 12:30 nvme-fc.ko
-rw-r--r--. 1 root 1.1M Feb 23 12:30 nvme.ko
-rw-r--r--. 1 root 1.2M Feb 23 12:30 nvme-rdma.ko
-rw-r--r--. 1 root 1.1M Feb 23 12:30 nvme-tcp.ko

/lib/modules/5.11.0-rc5nvme+/kernel/drivers/nvme/target//:
total 8.2M
-rw-r--r--. 1 root 712K Feb 23 12:30 nvme-fcloop.ko
-rw-r--r--. 1 root 619K Feb 23 12:30 nvme-loop.ko
-rw-r--r--. 1 root 1.1M Feb 23 12:30 nvmet-fc.ko
-rw-r--r--. 1 root 4.0M Feb 23 12:30 nvmet.ko
-rw-r--r--. 1 root 1.1M Feb 23 12:30 nvmet-rdma.ko
-rw-r--r--. 1 root 852K Feb 23 12:30 nvmet-tcp.ko
+ modprobe nvme
# 
# cat fio/verify.fio 
[write-and-verify]
rw=randwrite
bs=4k
direct=1
ioengine=libaio
iodepth=4
norandommap
randrepeat=1
verify=crc32c
size=950m
group_reporting
# fio fio/verify.fio --filename=/dev/nvme0n1 
write-and-verify: (g=0): rw=randwrite, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=libaio, iodepth=4
fio-3.8-5-g464b
Starting 1 process
Jobs: 1 (f=1): [f(1)][100.0%][r=104MiB/s,w=0KiB/s][r=26.6k,w=0 IOPS][eta 00m:00s]      
write-and-verify: (groupid=0, jobs=1): err= 0: pid=37040: Tue Feb 23 12:31:35 2021
   read: IOPS=26.4k, BW=103MiB/s (108MB/s)(602MiB/5831msec)
    slat (usec): min=11, max=5211, avg=32.40, stdev=24.38
    clat (usec): min=6, max=6821, avg=116.81, stdev=37.38
     lat (usec): min=65, max=6838, avg=149.39, stdev=45.87
    clat percentiles (usec):
     |  1.00th=[   81],  5.00th=[   98], 10.00th=[  103], 20.00th=[  105],
     | 30.00th=[  108], 40.00th=[  111], 50.00th=[  113], 60.00th=[  115],
     | 70.00th=[  118], 80.00th=[  123], 90.00th=[  133], 95.00th=[  153],
     | 99.00th=[  219], 99.50th=[  243], 99.90th=[  302], 99.95th=[  338],
     | 99.99th=[  482]
  write: IOPS=24.2k, BW=94.4MiB/s (99.0MB/s)(950MiB/10062msec)
    slat (usec): min=14, max=4146, avg=37.36, stdev=22.06
    clat (usec): min=56, max=4261, avg=126.98, stdev=27.62
     lat (usec): min=84, max=4327, avg=164.52, stdev=35.50
    clat percentiles (usec):
     |  1.00th=[   84],  5.00th=[   95], 10.00th=[  108], 20.00th=[  116],
     | 30.00th=[  120], 40.00th=[  124], 50.00th=[  126], 60.00th=[  127],
     | 70.00th=[  129], 80.00th=[  135], 90.00th=[  143], 95.00th=[  157],
     | 99.00th=[  225], 99.50th=[  258], 99.90th=[  351], 99.95th=[  404],
     | 99.99th=[  486]
   bw (  KiB/s): min=12344, max=99056, per=95.83%, avg=92647.62, stdev=18427.63, samples=21
   iops        : min= 3086, max=24764, avg=23161.90, stdev=4606.91, samples=21
  lat (usec)   : 10=0.01%, 50=0.01%, 100=6.46%, 250=93.05%, 500=0.48%
  lat (usec)   : 750=0.01%
  lat (msec)   : 4=0.01%, 10=0.01%
  cpu          : usr=12.95%, sys=86.81%, ctx=174, majf=0, minf=3626
  IO depths    : 1=0.1%, 2=0.1%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=0.0%
     submit    : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
     complete  : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
     issued rwts: total=154025,243200,0,0 short=0,0,0,0 dropped=0,0,0,0
     latency   : target=0, window=0, percentile=100.00%, depth=4

Run status group 0 (all jobs):
   READ: bw=103MiB/s (108MB/s), 103MiB/s-103MiB/s (108MB/s-108MB/s), io=602MiB (631MB), run=5831-5831msec
  WRITE: bw=94.4MiB/s (99.0MB/s), 94.4MiB/s-94.4MiB/s (99.0MB/s-99.0MB/s), io=950MiB (996MB), run=10062-10062msec

Disk stats (read/write):
  nvme0n1: ios=154119/243200, merge=0/0, ticks=8957/14742, in_queue=23699, util=99.49%
# modprobe -r nvme 
# 

-- 
2.22.1


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

[PATCH V2 1/2] nvme-pci; remove the barriers nvme_irq()

[Chaitanya Kulkarni]

The barriers were added to the nvme_irq() in the
commit 3a7afd8ee42a ("nvme-pci: remove the CQ lock for interrupt driven queues")
to prevent compiler from doing memory optimization for the variabes that
were protected previously by spinlock in nvme_irq() at completion queue
processing and with queue head check condition..

The variable nvmeq->last_cq_head from those checks was removed in the
commit f6c4d97b0d82 ("nvme/pci: Remove last_cq_head") that was not
allwing poll queues from mistakenly triggering the spurious interrupt
detection.

Remove the barriers which were protecting the updates to the variables.

Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 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

[PATCH V2 2/2] nvme-pci: cleanup nvme_irq()

[Chaitanya Kulkarni]

Get rid of the local variable that is not needed & just return.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/host/pci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 3729775f6a8a..2d07bdc76d0b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1060,12 +1060,10 @@ static inline int nvme_process_cq(struct nvme_queue *nvmeq)
 static irqreturn_t nvme_irq(int irq, void *data)
 {
 	struct nvme_queue *nvmeq = data;
-	irqreturn_t ret = IRQ_NONE;
 
 	if (nvme_process_cq(nvmeq))
-		ret = IRQ_HANDLED;
-
-	return ret;
+		return IRQ_HANDLED;
+	return IRQ_NONE;
 }
 
 static irqreturn_t nvme_irq_check(int irq, void *data)
-- 
2.22.1


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

Re: [PATCH V2 1/2] nvme-pci; remove the barriers nvme_irq()

[Minwoo Im]

If and when it's picked up, the title of this patch should be with:

	s/nvme-pci;/nvme-pci:/

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

Re: [PATCH V2 0/2] nvme-pci: remove the barriers in nvme_irq()

[Christoph Hellwig]

Thanks,

applied to nvme-5.13.

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