All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagi@grimberg.me>
To: Keith Busch <kbusch@kernel.org>
Cc: Michal Kalderon <mkalderon@marvell.com>,
	Christoph Hellwig <hch@lst.de>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	Shai Malin <smalin@marvell.com>, Ariel Elior <aelior@marvell.com>
Subject: Re: BUG: scheduling while atomic when nvmet_rdma_queue_response fails in posting a request
Date: Tue, 8 Jun 2021 17:03:48 -0700	[thread overview]
Message-ID: <4a031bc0-fba3-12d6-428f-a378aba1897e@grimberg.me> (raw)
In-Reply-To: <20210608184134.GA339600@dhcp-10-100-145-180.wdc.com>


>> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
>> index 7d607f435e36..6d2eea322779 100644
>> --- a/drivers/nvme/target/rdma.c
>> +++ b/drivers/nvme/target/rdma.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/wait.h>
>>   #include <linux/inet.h>
>>   #include <asm/unaligned.h>
>> +#include <linux/async.h>
>>
>>   #include <rdma/ib_verbs.h>
>>   #include <rdma/rdma_cm.h>
>> @@ -712,6 +713,12 @@ static void nvmet_rdma_send_done(struct ib_cq *cq,
>> struct ib_wc *wc)
>>          }
>>   }
>>
>> +static void nvmet_rdma_async_release_rsp(void *data, async_cookie_t cookie)
>> +{
>> +       struct nvmet_rdma_rsp *rsp = data;
>> +       nvmet_rdma_release_rsp(rsp);
>> +}
>> +
>>   static void nvmet_rdma_queue_response(struct nvmet_req *req)
>>   {
>>          struct nvmet_rdma_rsp *rsp =
>> @@ -745,7 +752,12 @@ static void nvmet_rdma_queue_response(struct nvmet_req
>> *req)
>>
>>          if (unlikely(ib_post_send(cm_id->qp, first_wr, NULL))) {
>>                  pr_err("sending cmd response failed\n");
>> -               nvmet_rdma_release_rsp(rsp);
>> +               /*
>> +                * We might be in atomic context, hence release
>> +                * the rsp in async context in case we need to
>> +                * process the wr_wait_list.
>> +                */
>> +               async_schedule(nvmet_rdma_async_release_rsp, rsp);
>>          }
>>   }
> 
> Just FYI, async_schedule() has conditions where it may execute your
> callback synchronously. Your suggestion is probably fine for testing,
> but it sounds like you require something that can guarantee a non-atomic
> context for nvmet_rdma_release_rsp().

OK, it seems that the issue is that we are submitting I/O in atomic
context. This should be more appropriate...

--
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 7d607f435e36..16f2f5a84ae7 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -102,6 +102,7 @@ struct nvmet_rdma_queue {

         struct work_struct      release_work;
         struct list_head        rsp_wait_list;
+       struct work_struct      wr_wait_work;
         struct list_head        rsp_wr_wait_list;
         spinlock_t              rsp_wr_wait_lock;

@@ -517,8 +518,10 @@ static int nvmet_rdma_post_recv(struct 
nvmet_rdma_device *ndev,
         return ret;
  }

-static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue)
+static void nvmet_rdma_process_wr_wait_list(struct work_struct *w)
  {
+       struct nvmet_rdma_queue *queue =
+               container_of(w, struct nvmet_rdma_queue, wr_wait_work);
         spin_lock(&queue->rsp_wr_wait_lock);
         while (!list_empty(&queue->rsp_wr_wait_list)) {
                 struct nvmet_rdma_rsp *rsp;
@@ -677,7 +680,7 @@ static void nvmet_rdma_release_rsp(struct 
nvmet_rdma_rsp *rsp)
                 nvmet_req_free_sgls(&rsp->req);

         if (unlikely(!list_empty_careful(&queue->rsp_wr_wait_list)))
-               nvmet_rdma_process_wr_wait_list(queue);
+               schedule_work(&queue->wr_wait_work);

         nvmet_rdma_put_rsp(rsp);
  }
@@ -1446,6 +1449,7 @@ nvmet_rdma_alloc_queue(struct nvmet_rdma_device *ndev,
          * inside a CM callback would trigger a deadlock. (great API 
design..)
          */
         INIT_WORK(&queue->release_work, nvmet_rdma_release_queue_work);
+       INIT_WORK(&queue->wr_wait_work, nvmet_rdma_process_wr_wait_list);
         queue->dev = ndev;
         queue->cm_id = cm_id;
         queue->port = port->nport;
--

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

  reply	other threads:[~2021-06-09  0:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-30  7:33 BUG: scheduling while atomic when nvmet_rdma_queue_response fails in posting a request Michal Kalderon
2021-06-08 16:50 ` Christoph Hellwig
2021-06-08 17:43 ` Sagi Grimberg
2021-06-08 18:41   ` Keith Busch
2021-06-09  0:03     ` Sagi Grimberg [this message]
2021-06-14 14:44       ` [EXT] " Michal Kalderon
2021-06-14 16:44         ` Sagi Grimberg
2021-06-14 18:14           ` Michal Kalderon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4a031bc0-fba3-12d6-428f-a378aba1897e@grimberg.me \
    --to=sagi@grimberg.me \
    --cc=aelior@marvell.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mkalderon@marvell.com \
    --cc=smalin@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.