Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs
@ 2020-02-27  2:53 Bijan Mottahedeh
  2020-02-27 16:35 ` Keith Busch
  2020-02-29 18:57 ` Christoph Hellwig
  0 siblings, 2 replies; 4+ messages in thread
From: Bijan Mottahedeh @ 2020-02-27  2:53 UTC (permalink / raw)
  To: linux-nvme

Avoid scheduling a polling thread if it has cqe's to complete, otherwise
another thread may free enough tags out of order, allowing a queue wrap.

This is an interim solution. Longer term, the two-pass completion should
be fixed.  It was done that way when submit and complete processing were
using the same spinlock, so reducing the critical section was helpful,
but that's not needed anymore.

Fixes: dabcefab45d3 ("nvme: provide optimized poll function for separate poll queues")
Signed-off-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
---
 drivers/nvme/host/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ace4dd9..d3f23d6 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1078,9 +1078,9 @@ static int nvme_poll(struct blk_mq_hw_ctx *hctx)
 
 	spin_lock(&nvmeq->cq_poll_lock);
 	found = nvme_process_cq(nvmeq, &start, &end, -1);
+	nvme_complete_cqes(nvmeq, start, end);
 	spin_unlock(&nvmeq->cq_poll_lock);
 
-	nvme_complete_cqes(nvmeq, start, end);
 	return found;
 }
 
-- 
1.8.3.1


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs
  2020-02-27  2:53 [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs Bijan Mottahedeh
@ 2020-02-27 16:35 ` Keith Busch
  2020-02-29 18:57 ` Christoph Hellwig
  1 sibling, 0 replies; 4+ messages in thread
From: Keith Busch @ 2020-02-27 16:35 UTC (permalink / raw)
  To: Bijan Mottahedeh; +Cc: linux-nvme

On Wed, Feb 26, 2020 at 06:53:43PM -0800, Bijan Mottahedeh wrote:
> Avoid scheduling a polling thread if it has cqe's to complete, otherwise
> another thread may free enough tags out of order, allowing a queue wrap.
> 
> This is an interim solution. Longer term, the two-pass completion should
> be fixed.  It was done that way when submit and complete processing were
> using the same spinlock, so reducing the critical section was helpful,
> but that's not needed anymore.
> 
> Fixes: dabcefab45d3 ("nvme: provide optimized poll function for separate poll queues")
> Signed-off-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Reviewed-by: Jens Axboe <axboe@kernel.dk>

Thanks, applied to a new nvme-5.6-rc4 branch.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs
  2020-02-27  2:53 [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs Bijan Mottahedeh
  2020-02-27 16:35 ` Keith Busch
@ 2020-02-29 18:57 ` Christoph Hellwig
  2020-03-02 19:04   ` Bijan Mottahedeh
  1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2020-02-29 18:57 UTC (permalink / raw)
  To: Bijan Mottahedeh; +Cc: linux-nvme

nvme_poll_irqdisable will need the same fix.

Also I think we can merge nvme_process_cq and nvme_complete_cqes now
as the split is becoming rather pointless.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs
  2020-02-29 18:57 ` Christoph Hellwig
@ 2020-03-02 19:04   ` Bijan Mottahedeh
  0 siblings, 0 replies; 4+ messages in thread
From: Bijan Mottahedeh @ 2020-03-02 19:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme

On 2/29/2020 10:57 AM, Christoph Hellwig wrote:
> nvme_poll_irqdisable will need the same fix.
>
> Also I think we can merge nvme_process_cq and nvme_complete_cqes now
> as the split is becoming rather pointless.
Ok, I'll work on a patch for both.

--bijan

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27  2:53 [PATCH v2 1/1] nvme-pci: Hold cq_poll_lock while completing CQEs Bijan Mottahedeh
2020-02-27 16:35 ` Keith Busch
2020-02-29 18:57 ` Christoph Hellwig
2020-03-02 19:04   ` Bijan Mottahedeh

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git