Linux PCI Non-Transparent Bridge framework and drivers
 help / color / mirror / Atom feed
* [PATCH] ntb: no sleep in ntb_async_tx_submit
@ 2017-06-09 22:06 Allen Hubbe
  2017-06-09 22:18 ` Dave Jiang
  2017-06-13 13:55 ` Jon Mason
  0 siblings, 2 replies; 3+ messages in thread
From: Allen Hubbe @ 2017-06-09 22:06 UTC (permalink / raw)
  To: linux-ntb; +Cc: Jon Mason, Dave Jiang, Allen Hubbe

Do not sleep in ntb_async_tx_submit, which could deadlock.
This reverts commit "8c874cc140d667f84ae4642bb5b5e0d6396d2ca4"

Fixes: 8c874cc140d667f84ae4642bb5b5e0d6396d2ca4

Reported-by: Jia-Ju Bai <baijiaju1990@163.com>
Signed-off-by: Allen Hubbe <Allen.Hubbe@dell.com>
---
 drivers/ntb/ntb_transport.c | 50 +++++++--------------------------------------
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 5b6b00ea6ed9..10e5bf460139 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -177,14 +177,12 @@ struct ntb_transport_qp {
 	u64 rx_err_ver;
 	u64 rx_memcpy;
 	u64 rx_async;
-	u64 dma_rx_prep_err;
 	u64 tx_bytes;
 	u64 tx_pkts;
 	u64 tx_ring_full;
 	u64 tx_err_no_buf;
 	u64 tx_memcpy;
 	u64 tx_async;
-	u64 dma_tx_prep_err;
 };
 
 struct ntb_transport_mw {
@@ -254,8 +252,6 @@ enum {
 #define QP_TO_MW(nt, qp)	((qp) % nt->mw_count)
 #define NTB_QP_DEF_NUM_ENTRIES	100
 #define NTB_LINK_DOWN_TIMEOUT	10
-#define DMA_RETRIES		20
-#define DMA_OUT_RESOURCE_TO	msecs_to_jiffies(50)
 
 static void ntb_transport_rxc_db(unsigned long data);
 static const struct ntb_ctx_ops ntb_transport_ops;
@@ -516,12 +512,6 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "free tx - \t%u\n",
 			       ntb_transport_tx_free_entry(qp));
-	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "DMA tx prep err - \t%llu\n",
-			       qp->dma_tx_prep_err);
-	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "DMA rx prep err - \t%llu\n",
-			       qp->dma_rx_prep_err);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "\n");
@@ -768,8 +758,6 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
 	qp->tx_err_no_buf = 0;
 	qp->tx_memcpy = 0;
 	qp->tx_async = 0;
-	qp->dma_tx_prep_err = 0;
-	qp->dma_rx_prep_err = 0;
 }
 
 static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
@@ -1317,7 +1305,6 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
 	struct dmaengine_unmap_data *unmap;
 	dma_cookie_t cookie;
 	void *buf = entry->buf;
-	int retries = 0;
 
 	len = entry->len;
 	device = chan->device;
@@ -1346,22 +1333,11 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
 
 	unmap->from_cnt = 1;
 
-	for (retries = 0; retries < DMA_RETRIES; retries++) {
-		txd = device->device_prep_dma_memcpy(chan,
-						     unmap->addr[1],
-						     unmap->addr[0], len,
-						     DMA_PREP_INTERRUPT);
-		if (txd)
-			break;
-
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(DMA_OUT_RESOURCE_TO);
-	}
-
-	if (!txd) {
-		qp->dma_rx_prep_err++;
+	txd = device->device_prep_dma_memcpy(chan, unmap->addr[1],
+					     unmap->addr[0], len,
+					     DMA_PREP_INTERRUPT);
+	if (!txd)
 		goto err_get_unmap;
-	}
 
 	txd->callback_result = ntb_rx_copy_callback;
 	txd->callback_param = entry;
@@ -1606,7 +1582,6 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
 	struct dmaengine_unmap_data *unmap;
 	dma_addr_t dest;
 	dma_cookie_t cookie;
-	int retries = 0;
 
 	device = chan->device;
 	dest = qp->tx_mw_phys + qp->tx_max_frame * entry->tx_index;
@@ -1628,21 +1603,10 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
 
 	unmap->to_cnt = 1;
 
-	for (retries = 0; retries < DMA_RETRIES; retries++) {
-		txd = device->device_prep_dma_memcpy(chan, dest,
-						     unmap->addr[0], len,
-						     DMA_PREP_INTERRUPT);
-		if (txd)
-			break;
-
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule_timeout(DMA_OUT_RESOURCE_TO);
-	}
-
-	if (!txd) {
-		qp->dma_tx_prep_err++;
+	txd = device->device_prep_dma_memcpy(chan, dest, unmap->addr[0], len,
+					     DMA_PREP_INTERRUPT);
+	if (!txd)
 		goto err_get_unmap;
-	}
 
 	txd->callback_result = ntb_tx_copy_callback;
 	txd->callback_param = entry;
-- 
2.13.0


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

* Re: [PATCH] ntb: no sleep in ntb_async_tx_submit
  2017-06-09 22:06 [PATCH] ntb: no sleep in ntb_async_tx_submit Allen Hubbe
@ 2017-06-09 22:18 ` Dave Jiang
  2017-06-13 13:55 ` Jon Mason
  1 sibling, 0 replies; 3+ messages in thread
From: Dave Jiang @ 2017-06-09 22:18 UTC (permalink / raw)
  To: Allen Hubbe, linux-ntb; +Cc: Jon Mason



On 06/09/2017 03:06 PM, Allen Hubbe wrote:
> Do not sleep in ntb_async_tx_submit, which could deadlock.
> This reverts commit "8c874cc140d667f84ae4642bb5b5e0d6396d2ca4"
> 
> Fixes: 8c874cc140d667f84ae4642bb5b5e0d6396d2ca4
> 
> Reported-by: Jia-Ju Bai <baijiaju1990@163.com>
> Signed-off-by: Allen Hubbe <Allen.Hubbe@dell.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/ntb_transport.c | 50 +++++++--------------------------------------
>  1 file changed, 7 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 5b6b00ea6ed9..10e5bf460139 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -177,14 +177,12 @@ struct ntb_transport_qp {
>  	u64 rx_err_ver;
>  	u64 rx_memcpy;
>  	u64 rx_async;
> -	u64 dma_rx_prep_err;
>  	u64 tx_bytes;
>  	u64 tx_pkts;
>  	u64 tx_ring_full;
>  	u64 tx_err_no_buf;
>  	u64 tx_memcpy;
>  	u64 tx_async;
> -	u64 dma_tx_prep_err;
>  };
>  
>  struct ntb_transport_mw {
> @@ -254,8 +252,6 @@ enum {
>  #define QP_TO_MW(nt, qp)	((qp) % nt->mw_count)
>  #define NTB_QP_DEF_NUM_ENTRIES	100
>  #define NTB_LINK_DOWN_TIMEOUT	10
> -#define DMA_RETRIES		20
> -#define DMA_OUT_RESOURCE_TO	msecs_to_jiffies(50)
>  
>  static void ntb_transport_rxc_db(unsigned long data);
>  static const struct ntb_ctx_ops ntb_transport_ops;
> @@ -516,12 +512,6 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
>  	out_offset += snprintf(buf + out_offset, out_count - out_offset,
>  			       "free tx - \t%u\n",
>  			       ntb_transport_tx_free_entry(qp));
> -	out_offset += snprintf(buf + out_offset, out_count - out_offset,
> -			       "DMA tx prep err - \t%llu\n",
> -			       qp->dma_tx_prep_err);
> -	out_offset += snprintf(buf + out_offset, out_count - out_offset,
> -			       "DMA rx prep err - \t%llu\n",
> -			       qp->dma_rx_prep_err);
>  
>  	out_offset += snprintf(buf + out_offset, out_count - out_offset,
>  			       "\n");
> @@ -768,8 +758,6 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
>  	qp->tx_err_no_buf = 0;
>  	qp->tx_memcpy = 0;
>  	qp->tx_async = 0;
> -	qp->dma_tx_prep_err = 0;
> -	qp->dma_rx_prep_err = 0;
>  }
>  
>  static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
> @@ -1317,7 +1305,6 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>  	struct dmaengine_unmap_data *unmap;
>  	dma_cookie_t cookie;
>  	void *buf = entry->buf;
> -	int retries = 0;
>  
>  	len = entry->len;
>  	device = chan->device;
> @@ -1346,22 +1333,11 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>  
>  	unmap->from_cnt = 1;
>  
> -	for (retries = 0; retries < DMA_RETRIES; retries++) {
> -		txd = device->device_prep_dma_memcpy(chan,
> -						     unmap->addr[1],
> -						     unmap->addr[0], len,
> -						     DMA_PREP_INTERRUPT);
> -		if (txd)
> -			break;
> -
> -		set_current_state(TASK_INTERRUPTIBLE);
> -		schedule_timeout(DMA_OUT_RESOURCE_TO);
> -	}
> -
> -	if (!txd) {
> -		qp->dma_rx_prep_err++;
> +	txd = device->device_prep_dma_memcpy(chan, unmap->addr[1],
> +					     unmap->addr[0], len,
> +					     DMA_PREP_INTERRUPT);
> +	if (!txd)
>  		goto err_get_unmap;
> -	}
>  
>  	txd->callback_result = ntb_rx_copy_callback;
>  	txd->callback_param = entry;
> @@ -1606,7 +1582,6 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>  	struct dmaengine_unmap_data *unmap;
>  	dma_addr_t dest;
>  	dma_cookie_t cookie;
> -	int retries = 0;
>  
>  	device = chan->device;
>  	dest = qp->tx_mw_phys + qp->tx_max_frame * entry->tx_index;
> @@ -1628,21 +1603,10 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>  
>  	unmap->to_cnt = 1;
>  
> -	for (retries = 0; retries < DMA_RETRIES; retries++) {
> -		txd = device->device_prep_dma_memcpy(chan, dest,
> -						     unmap->addr[0], len,
> -						     DMA_PREP_INTERRUPT);
> -		if (txd)
> -			break;
> -
> -		set_current_state(TASK_INTERRUPTIBLE);
> -		schedule_timeout(DMA_OUT_RESOURCE_TO);
> -	}
> -
> -	if (!txd) {
> -		qp->dma_tx_prep_err++;
> +	txd = device->device_prep_dma_memcpy(chan, dest, unmap->addr[0], len,
> +					     DMA_PREP_INTERRUPT);
> +	if (!txd)
>  		goto err_get_unmap;
> -	}
>  
>  	txd->callback_result = ntb_tx_copy_callback;
>  	txd->callback_param = entry;
> 

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

* Re: [PATCH] ntb: no sleep in ntb_async_tx_submit
  2017-06-09 22:06 [PATCH] ntb: no sleep in ntb_async_tx_submit Allen Hubbe
  2017-06-09 22:18 ` Dave Jiang
@ 2017-06-13 13:55 ` Jon Mason
  1 sibling, 0 replies; 3+ messages in thread
From: Jon Mason @ 2017-06-13 13:55 UTC (permalink / raw)
  To: Allen Hubbe; +Cc: linux-ntb, Dave Jiang

On Fri, Jun 9, 2017 at 6:06 PM, Allen Hubbe <Allen.Hubbe@dell.com> wrote:
> Do not sleep in ntb_async_tx_submit, which could deadlock.
> This reverts commit "8c874cc140d667f84ae4642bb5b5e0d6396d2ca4"
>
> Fixes: 8c874cc140d667f84ae4642bb5b5e0d6396d2ca4

Nit, this should be
Fixes: 8c874cc140d6 ("NTB: Address out of DMA descriptor issue with NTB")

12 characters and the description

I fixed it up, added Dave's ack, and put it in my ntb branch.

Thanks,
Jon

> Reported-by: Jia-Ju Bai <baijiaju1990@163.com>
> Signed-off-by: Allen Hubbe <Allen.Hubbe@dell.com>
> ---
>  drivers/ntb/ntb_transport.c | 50 +++++++--------------------------------------
>  1 file changed, 7 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 5b6b00ea6ed9..10e5bf460139 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -177,14 +177,12 @@ struct ntb_transport_qp {
>         u64 rx_err_ver;
>         u64 rx_memcpy;
>         u64 rx_async;
> -       u64 dma_rx_prep_err;
>         u64 tx_bytes;
>         u64 tx_pkts;
>         u64 tx_ring_full;
>         u64 tx_err_no_buf;
>         u64 tx_memcpy;
>         u64 tx_async;
> -       u64 dma_tx_prep_err;
>  };
>
>  struct ntb_transport_mw {
> @@ -254,8 +252,6 @@ enum {
>  #define QP_TO_MW(nt, qp)       ((qp) % nt->mw_count)
>  #define NTB_QP_DEF_NUM_ENTRIES 100
>  #define NTB_LINK_DOWN_TIMEOUT  10
> -#define DMA_RETRIES            20
> -#define DMA_OUT_RESOURCE_TO    msecs_to_jiffies(50)
>
>  static void ntb_transport_rxc_db(unsigned long data);
>  static const struct ntb_ctx_ops ntb_transport_ops;
> @@ -516,12 +512,6 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
>         out_offset += snprintf(buf + out_offset, out_count - out_offset,
>                                "free tx - \t%u\n",
>                                ntb_transport_tx_free_entry(qp));
> -       out_offset += snprintf(buf + out_offset, out_count - out_offset,
> -                              "DMA tx prep err - \t%llu\n",
> -                              qp->dma_tx_prep_err);
> -       out_offset += snprintf(buf + out_offset, out_count - out_offset,
> -                              "DMA rx prep err - \t%llu\n",
> -                              qp->dma_rx_prep_err);
>
>         out_offset += snprintf(buf + out_offset, out_count - out_offset,
>                                "\n");
> @@ -768,8 +758,6 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
>         qp->tx_err_no_buf = 0;
>         qp->tx_memcpy = 0;
>         qp->tx_async = 0;
> -       qp->dma_tx_prep_err = 0;
> -       qp->dma_rx_prep_err = 0;
>  }
>
>  static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
> @@ -1317,7 +1305,6 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>         struct dmaengine_unmap_data *unmap;
>         dma_cookie_t cookie;
>         void *buf = entry->buf;
> -       int retries = 0;
>
>         len = entry->len;
>         device = chan->device;
> @@ -1346,22 +1333,11 @@ static int ntb_async_rx_submit(struct ntb_queue_entry *entry, void *offset)
>
>         unmap->from_cnt = 1;
>
> -       for (retries = 0; retries < DMA_RETRIES; retries++) {
> -               txd = device->device_prep_dma_memcpy(chan,
> -                                                    unmap->addr[1],
> -                                                    unmap->addr[0], len,
> -                                                    DMA_PREP_INTERRUPT);
> -               if (txd)
> -                       break;
> -
> -               set_current_state(TASK_INTERRUPTIBLE);
> -               schedule_timeout(DMA_OUT_RESOURCE_TO);
> -       }
> -
> -       if (!txd) {
> -               qp->dma_rx_prep_err++;
> +       txd = device->device_prep_dma_memcpy(chan, unmap->addr[1],
> +                                            unmap->addr[0], len,
> +                                            DMA_PREP_INTERRUPT);
> +       if (!txd)
>                 goto err_get_unmap;
> -       }
>
>         txd->callback_result = ntb_rx_copy_callback;
>         txd->callback_param = entry;
> @@ -1606,7 +1582,6 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>         struct dmaengine_unmap_data *unmap;
>         dma_addr_t dest;
>         dma_cookie_t cookie;
> -       int retries = 0;
>
>         device = chan->device;
>         dest = qp->tx_mw_phys + qp->tx_max_frame * entry->tx_index;
> @@ -1628,21 +1603,10 @@ static int ntb_async_tx_submit(struct ntb_transport_qp *qp,
>
>         unmap->to_cnt = 1;
>
> -       for (retries = 0; retries < DMA_RETRIES; retries++) {
> -               txd = device->device_prep_dma_memcpy(chan, dest,
> -                                                    unmap->addr[0], len,
> -                                                    DMA_PREP_INTERRUPT);
> -               if (txd)
> -                       break;
> -
> -               set_current_state(TASK_INTERRUPTIBLE);
> -               schedule_timeout(DMA_OUT_RESOURCE_TO);
> -       }
> -
> -       if (!txd) {
> -               qp->dma_tx_prep_err++;
> +       txd = device->device_prep_dma_memcpy(chan, dest, unmap->addr[0], len,
> +                                            DMA_PREP_INTERRUPT);
> +       if (!txd)
>                 goto err_get_unmap;
> -       }
>
>         txd->callback_result = ntb_tx_copy_callback;
>         txd->callback_param = entry;
> --
> 2.13.0
>

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

end of thread, other threads:[~2017-06-13 13:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-09 22:06 [PATCH] ntb: no sleep in ntb_async_tx_submit Allen Hubbe
2017-06-09 22:18 ` Dave Jiang
2017-06-13 13:55 ` Jon Mason

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox