linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] nvme: free sq/cq dbbuf pointers when dbbuf set fails
@ 2020-11-05 14:28 Minwoo Im
  2020-11-05 16:13 ` Keith Busch
  2020-11-14  9:03 ` Christoph Hellwig
  0 siblings, 2 replies; 4+ messages in thread
From: Minwoo Im @ 2020-11-05 14:28 UTC (permalink / raw)
  To: linux-nvme
  Cc: Keith Busch, Jens Axboe, Minwoo Im, Christoph Hellwig, Sagi Grimberg

If Doorbell Buffer Config command fails even 'dev->dbbuf_dbs != NULL'
which means OACS indicates that NVME_CTRL_OACS_DBBUF_SUPP is set,
nvme_dbbuf_update_and_check_event() will check event even it's not been
successfully set.

This patch fixes mismatch among dbbuf for sq/cqs in case that dbbuf
command fails.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/pci.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0578ff253c47..3be352403839 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -292,9 +292,21 @@ static void nvme_dbbuf_init(struct nvme_dev *dev,
 	nvmeq->dbbuf_cq_ei = &dev->dbbuf_eis[cq_idx(qid, dev->db_stride)];
 }
 
+static void nvme_dbbuf_free(struct nvme_queue *nvmeq)
+{
+	if (!nvmeq->qid)
+		return;
+
+	nvmeq->dbbuf_sq_db = NULL;
+	nvmeq->dbbuf_cq_db = NULL;
+	nvmeq->dbbuf_sq_ei = NULL;
+	nvmeq->dbbuf_cq_ei = NULL;
+}
+
 static void nvme_dbbuf_set(struct nvme_dev *dev)
 {
 	struct nvme_command c;
+	unsigned int i;
 
 	if (!dev->dbbuf_dbs)
 		return;
@@ -308,6 +320,9 @@ static void nvme_dbbuf_set(struct nvme_dev *dev)
 		dev_warn(dev->ctrl.device, "unable to set dbbuf\n");
 		/* Free memory and continue on */
 		nvme_dbbuf_dma_free(dev);
+
+		for (i = 1; i <= dev->online_queues; i++)
+			nvme_dbbuf_free(&dev->queues[i]);
 	}
 }
 
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] nvme: free sq/cq dbbuf pointers when dbbuf set fails
  2020-11-05 14:28 [PATCH] nvme: free sq/cq dbbuf pointers when dbbuf set fails Minwoo Im
@ 2020-11-05 16:13 ` Keith Busch
  2020-11-06  9:02   ` Minwoo Im
  2020-11-14  9:03 ` Christoph Hellwig
  1 sibling, 1 reply; 4+ messages in thread
From: Keith Busch @ 2020-11-05 16:13 UTC (permalink / raw)
  To: Minwoo Im; +Cc: Jens Axboe, Christoph Hellwig, linux-nvme, Sagi Grimberg

On Thu, Nov 05, 2020 at 11:28:47PM +0900, Minwoo Im wrote:
> If Doorbell Buffer Config command fails even 'dev->dbbuf_dbs != NULL'
> which means OACS indicates that NVME_CTRL_OACS_DBBUF_SUPP is set,
> nvme_dbbuf_update_and_check_event() will check event even it's not been
> successfully set.
> 
> This patch fixes mismatch among dbbuf for sq/cqs in case that dbbuf
> command fails.

This fixes if host dbbuf was successfully configured at one point, but
fails on a subsequent reset, correct? Otherwise, I don't see how the
queue's dbbuf values would have been set to non-NULL.

_______________________________________________
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] nvme: free sq/cq dbbuf pointers when dbbuf set fails
  2020-11-05 16:13 ` Keith Busch
@ 2020-11-06  9:02   ` Minwoo Im
  0 siblings, 0 replies; 4+ messages in thread
From: Minwoo Im @ 2020-11-06  9:02 UTC (permalink / raw)
  To: Keith Busch; +Cc: Jens Axboe, Christoph Hellwig, linux-nvme, Sagi Grimberg

On Fri, Nov 6, 2020 at 1:13 AM Keith Busch <kbusch@kernel.org> wrote:
>
> On Thu, Nov 05, 2020 at 11:28:47PM +0900, Minwoo Im wrote:
> > If Doorbell Buffer Config command fails even 'dev->dbbuf_dbs != NULL'
> > which means OACS indicates that NVME_CTRL_OACS_DBBUF_SUPP is set,
> > nvme_dbbuf_update_and_check_event() will check event even it's not been
> > successfully set.
> >
> > This patch fixes mismatch among dbbuf for sq/cqs in case that dbbuf
> > command fails.
>
> This fixes if host dbbuf was successfully configured at one point, but
> fails on a subsequent reset, correct? Otherwise, I don't see how the
> queue's dbbuf values would have been set to non-NULL.

Yes. reset_work() checks OACS with DBBUF_SUPP and if it's reported as
supported, then it will alloc dev->dbbuf_dbs.  After that, However, if
nvme_dbbuf_set() fails with sc != 0 then it should free for each queue's
dbbuf pointer to prevent being checked for events when updating the doorbell.

_______________________________________________
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] nvme: free sq/cq dbbuf pointers when dbbuf set fails
  2020-11-05 14:28 [PATCH] nvme: free sq/cq dbbuf pointers when dbbuf set fails Minwoo Im
  2020-11-05 16:13 ` Keith Busch
@ 2020-11-14  9:03 ` Christoph Hellwig
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2020-11-14  9:03 UTC (permalink / raw)
  To: Minwoo Im
  Cc: Keith Busch, Jens Axboe, Christoph Hellwig, linux-nvme, Sagi Grimberg

Thanks,

applied to nvme-5.10.

_______________________________________________
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, other threads:[~2020-11-14  9:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 14:28 [PATCH] nvme: free sq/cq dbbuf pointers when dbbuf set fails Minwoo Im
2020-11-05 16:13 ` Keith Busch
2020-11-06  9:02   ` Minwoo Im
2020-11-14  9:03 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).