All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] rds: ib: drop unnecessary rdma_reject
@ 2017-03-09  7:26 Zhu Yanjun
       [not found] ` <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  7:26 UTC (permalink / raw)
  To: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

When rdma_accept fails, rdma_reject is called in it. As such, it is
not necessary to execute rdma_reject again.

Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 net/rds/ib_cm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index ce3775a..eca3d5f 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -677,8 +677,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 		event->param.conn.initiator_depth);
 
 	/* rdma_accept() calls rdma_reject() internally if it fails */
-	err = rdma_accept(cm_id, &conn_param);
-	if (err)
+	if (rdma_accept(cm_id, &conn_param))
 		rds_ib_conn_error(conn, "rdma_accept failed (%d)\n", err);
 
 out:
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave
       [not found] ` <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-03-09  7:26   ` Zhu Yanjun
       [not found]     ` <1489044405-26150-2-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  7:26 UTC (permalink / raw)
  To: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

It is difficult to make sure the state of the interrupt when this
function is called. As such, it is safer to use spin_lock_irqsave
than spin_lock_irq.

Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 net/rds/ib_cm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index eca3d5f..87ec4dd 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -963,6 +963,7 @@ void rds_ib_conn_free(void *arg)
 {
 	struct rds_ib_connection *ic = arg;
 	spinlock_t	*lock_ptr;
+	unsigned long	flags;
 
 	rdsdebug("ic %p\n", ic);
 
@@ -973,9 +974,9 @@ void rds_ib_conn_free(void *arg)
 	 */
 	lock_ptr = ic->rds_ibdev ? &ic->rds_ibdev->spinlock : &ib_nodev_conns_lock;
 
-	spin_lock_irq(lock_ptr);
+	spin_lock_irqsave(lock_ptr, flags);
 	list_del(&ic->ib_node);
-	spin_unlock_irq(lock_ptr);
+	spin_unlock_irqrestore(lock_ptr, flags);
 
 	rds_ib_recv_free_caches(ic);
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr
  2017-03-09  7:26 [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Zhu Yanjun
       [not found] ` <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-03-09  7:26 ` Zhu Yanjun
  2017-03-09  8:16   ` Johannes Thumshirn
  2017-03-09  7:26 ` [PATCH 4/5] rds: ib: add the static type to the function Zhu Yanjun
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  7:26 UTC (permalink / raw)
  To: santosh.shilimkar, netdev, linux-rdma, rds-devel

The function ib_dealloc_fmr will never be called. As such, it should
be removed.

Cc: Joe Jin <joe.jin@oracle.com>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 net/rds/ib_fmr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
index 4fe8f4f..b807df6 100644
--- a/net/rds/ib_fmr.c
+++ b/net/rds/ib_fmr.c
@@ -78,12 +78,10 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
 	return ibmr;
 
 out_no_cigar:
-	if (ibmr) {
-		if (fmr->fmr)
-			ib_dealloc_fmr(fmr->fmr);
+	if (ibmr)
 		kfree(ibmr);
-	}
 	atomic_dec(&pool->item_count);
+
 	return ERR_PTR(err);
 }
 
-- 
2.7.4

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

* [PATCH 4/5] rds: ib: add the static type to the function
  2017-03-09  7:26 [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Zhu Yanjun
       [not found] ` <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  2017-03-09  7:26 ` [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr Zhu Yanjun
@ 2017-03-09  7:26 ` Zhu Yanjun
  2017-03-09 16:52   ` Santosh Shilimkar
  2017-03-09  7:26 ` [PATCH 5/5] rds: ib: unmap the scatter/gather list when error Zhu Yanjun
  2017-03-09 16:47 ` [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Santosh Shilimkar
  4 siblings, 1 reply; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  7:26 UTC (permalink / raw)
  To: santosh.shilimkar, netdev, linux-rdma, rds-devel

The function rds_ib_map_fmr is used only in the ib_fmr.c
file. As such, the static type is added to limit it in this file.

Cc: Joe Jin <joe.jin@oracle.com>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 net/rds/ib_fmr.c | 5 +++--
 net/rds/ib_mr.h  | 2 --
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
index b807df6..1e00371 100644
--- a/net/rds/ib_fmr.c
+++ b/net/rds/ib_fmr.c
@@ -85,8 +85,9 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
 	return ERR_PTR(err);
 }
 
-int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev, struct rds_ib_mr *ibmr,
-		   struct scatterlist *sg, unsigned int nents)
+static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev,
+			  struct rds_ib_mr *ibmr, struct scatterlist *sg,
+			  unsigned int nents)
 {
 	struct ib_device *dev = rds_ibdev->dev;
 	struct rds_ib_fmr *fmr = &ibmr->u.fmr;
diff --git a/net/rds/ib_mr.h b/net/rds/ib_mr.h
index 5d6e98a..0ea4ab0 100644
--- a/net/rds/ib_mr.h
+++ b/net/rds/ib_mr.h
@@ -125,8 +125,6 @@ void rds_ib_mr_exit(void);
 void __rds_ib_teardown_mr(struct rds_ib_mr *);
 void rds_ib_teardown_mr(struct rds_ib_mr *);
 struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *, int);
-int rds_ib_map_fmr(struct rds_ib_device *, struct rds_ib_mr *,
-		   struct scatterlist *, unsigned int);
 struct rds_ib_mr *rds_ib_reuse_mr(struct rds_ib_mr_pool *);
 int rds_ib_flush_mr_pool(struct rds_ib_mr_pool *, int, struct rds_ib_mr **);
 struct rds_ib_mr *rds_ib_reg_fmr(struct rds_ib_device *, struct scatterlist *,
-- 
2.7.4

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

* [PATCH 5/5] rds: ib: unmap the scatter/gather list when error
  2017-03-09  7:26 [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Zhu Yanjun
                   ` (2 preceding siblings ...)
  2017-03-09  7:26 ` [PATCH 4/5] rds: ib: add the static type to the function Zhu Yanjun
@ 2017-03-09  7:26 ` Zhu Yanjun
       [not found]   ` <1489044405-26150-5-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  2017-03-09 16:47 ` [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Santosh Shilimkar
  4 siblings, 1 reply; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  7:26 UTC (permalink / raw)
  To: santosh.shilimkar, netdev, linux-rdma, rds-devel

When some errors occur, the scatter/gather list mapped to DMA addresses
should be handled.

Cc: Joe Jin <joe.jin@oracle.com>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 net/rds/ib_fmr.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
index 1e00371..c60247f 100644
--- a/net/rds/ib_fmr.c
+++ b/net/rds/ib_fmr.c
@@ -113,29 +113,39 @@ static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev,
 		u64 dma_addr = ib_sg_dma_address(dev, &scat[i]);
 
 		if (dma_addr & ~PAGE_MASK) {
-			if (i > 0)
+			if (i > 0) {
+				ib_dma_unmap_sg(dev, sg, nents,
+						DMA_BIDIRECTIONAL);
 				return -EINVAL;
-			else
+			} else {
 				++page_cnt;
+			}
 		}
 		if ((dma_addr + dma_len) & ~PAGE_MASK) {
-			if (i < sg_dma_len - 1)
+			if (i < sg_dma_len - 1) {
+				ib_dma_unmap_sg(dev, sg, nents,
+						DMA_BIDIRECTIONAL);
 				return -EINVAL;
-			else
+			} else {
 				++page_cnt;
+			}
 		}
 
 		len += dma_len;
 	}
 
 	page_cnt += len >> PAGE_SHIFT;
-	if (page_cnt > ibmr->pool->fmr_attr.max_pages)
+	if (page_cnt > ibmr->pool->fmr_attr.max_pages) {
+		ib_dma_unmap_sg(dev, sg, nents, DMA_BIDIRECTIONAL);
 		return -EINVAL;
+	}
 
 	dma_pages = kmalloc_node(sizeof(u64) * page_cnt, GFP_ATOMIC,
 				 rdsibdev_to_node(rds_ibdev));
-	if (!dma_pages)
+	if (!dma_pages) {
+		ib_dma_unmap_sg(dev, sg, nents, DMA_BIDIRECTIONAL);
 		return -ENOMEM;
+	}
 
 	page_cnt = 0;
 	for (i = 0; i < sg_dma_len; ++i) {
@@ -148,8 +158,10 @@ static int rds_ib_map_fmr(struct rds_ib_device *rds_ibdev,
 	}
 
 	ret = ib_map_phys_fmr(fmr->fmr, dma_pages, page_cnt, io_addr);
-	if (ret)
+	if (ret) {
+		ib_dma_unmap_sg(dev, sg, nents, DMA_BIDIRECTIONAL);
 		goto out;
+	}
 
 	/* Success - we successfully remapped the MR, so we can
 	 * safely tear down the old mapping.
-- 
2.7.4

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

* Re: [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr
  2017-03-09  7:26 ` [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr Zhu Yanjun
@ 2017-03-09  8:16   ` Johannes Thumshirn
       [not found]     ` <cffc93fa-f4e6-e61e-54c6-67d6501dd78c-l3A5Bk7waGM@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Thumshirn @ 2017-03-09  8:16 UTC (permalink / raw)
  To: Zhu Yanjun, santosh.shilimkar, netdev, linux-rdma, rds-devel

On 03/09/2017 08:26 AM, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
>  net/rds/ib_fmr.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
> index 4fe8f4f..b807df6 100644
> --- a/net/rds/ib_fmr.c
> +++ b/net/rds/ib_fmr.c
> @@ -78,12 +78,10 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
>  	return ibmr;
>  
>  out_no_cigar:
> -	if (ibmr) {
> -		if (fmr->fmr)
> -			ib_dealloc_fmr(fmr->fmr);
> +	if (ibmr)
>  		kfree(ibmr);

kfree() already does a NULL check (see
http://lxr.free-electrons.com/source/mm/slab.c#L3811) so no need to do
it here.



-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr
       [not found]     ` <cffc93fa-f4e6-e61e-54c6-67d6501dd78c-l3A5Bk7waGM@public.gmane.org>
@ 2017-03-09  9:20       ` Zhu Yanjun
  2017-03-09  9:42         ` Johannes Thumshirn
                           ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Zhu Yanjun @ 2017-03-09  9:20 UTC (permalink / raw)
  To: jthumshirn-l3A5Bk7waGM, santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

The function ib_dealloc_fmr will never be called. As such, it should
be removed.

Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
Change from v1 to v2:
  remove ibmr NULL test.

 net/rds/ib_fmr.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
index 4fe8f4f..249ae1c 100644
--- a/net/rds/ib_fmr.c
+++ b/net/rds/ib_fmr.c
@@ -78,12 +78,9 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
 	return ibmr;
 
 out_no_cigar:
-	if (ibmr) {
-		if (fmr->fmr)
-			ib_dealloc_fmr(fmr->fmr);
-		kfree(ibmr);
-	}
+	kfree(ibmr);
 	atomic_dec(&pool->item_count);
+
 	return ERR_PTR(err);
 }
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr
  2017-03-09  9:20       ` [PATCHv2 " Zhu Yanjun
@ 2017-03-09  9:42         ` Johannes Thumshirn
  2017-03-09 12:09         ` Yuval Shaia
  2017-03-09 16:51         ` Santosh Shilimkar
  2 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2017-03-09  9:42 UTC (permalink / raw)
  To: Zhu Yanjun, santosh.shilimkar, netdev, linux-rdma, rds-devel

On 03/09/2017 10:20 AM, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>


-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr
  2017-03-09  9:20       ` [PATCHv2 " Zhu Yanjun
  2017-03-09  9:42         ` Johannes Thumshirn
@ 2017-03-09 12:09         ` Yuval Shaia
  2017-03-09 16:51         ` Santosh Shilimkar
  2 siblings, 0 replies; 17+ messages in thread
From: Yuval Shaia @ 2017-03-09 12:09 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: jthumshirn, santosh.shilimkar, netdev, linux-rdma, rds-devel

On Thu, Mar 09, 2017 at 04:20:43AM -0500, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
> Change from v1 to v2:
>   remove ibmr NULL test.
> 
>  net/rds/ib_fmr.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
> index 4fe8f4f..249ae1c 100644
> --- a/net/rds/ib_fmr.c
> +++ b/net/rds/ib_fmr.c
> @@ -78,12 +78,9 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
>  	return ibmr;
>  
>  out_no_cigar:
> -	if (ibmr) {
> -		if (fmr->fmr)
> -			ib_dealloc_fmr(fmr->fmr);
> -		kfree(ibmr);
> -	}
> +	kfree(ibmr);
>  	atomic_dec(&pool->item_count);
> +

Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>

>  	return ERR_PTR(err);
>  }
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/5] rds: ib: drop unnecessary rdma_reject
  2017-03-09  7:26 [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Zhu Yanjun
                   ` (3 preceding siblings ...)
  2017-03-09  7:26 ` [PATCH 5/5] rds: ib: unmap the scatter/gather list when error Zhu Yanjun
@ 2017-03-09 16:47 ` Santosh Shilimkar
  4 siblings, 0 replies; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 16:47 UTC (permalink / raw)
  To: Zhu Yanjun, netdev, linux-rdma, rds-devel

On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
> When rdma_accept fails, rdma_reject is called in it. As such, it is
> not necessary to execute rdma_reject again.
>
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
>  net/rds/ib_cm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index ce3775a..eca3d5f 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -677,8 +677,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
>  		event->param.conn.initiator_depth);
>
>  	/* rdma_accept() calls rdma_reject() internally if it fails */
> -	err = rdma_accept(cm_id, &conn_param);
> -	if (err)
> +	if (rdma_accept(cm_id, &conn_param))
>  		rds_ib_conn_error(conn, "rdma_accept failed (%d)\n", err);
>
There are couple of consumer reject reasons needs to be conveyed.
Current code don't pass them, but downstream code has them and
hence I kept the code as is. Its good to avoid the reject on
current code so am fine with the change.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

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

* Re: [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave
       [not found]     ` <1489044405-26150-2-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-03-09 16:50       ` Santosh Shilimkar
       [not found]         ` <65ac3de8-9fdd-b5c5-b7aa-aa4393626551-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 16:50 UTC (permalink / raw)
  To: Zhu Yanjun, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
> It is difficult to make sure the state of the interrupt when this
> function is called. As such, it is safer to use spin_lock_irqsave
> than spin_lock_irq.
>
There is no reason to hold irqs  and as such the code path is
safe from irq context. I don't see need of this change unless
you have test case which showed some issue.

Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr
  2017-03-09  9:20       ` [PATCHv2 " Zhu Yanjun
  2017-03-09  9:42         ` Johannes Thumshirn
  2017-03-09 12:09         ` Yuval Shaia
@ 2017-03-09 16:51         ` Santosh Shilimkar
  2 siblings, 0 replies; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 16:51 UTC (permalink / raw)
  To: Zhu Yanjun, jthumshirn, netdev, linux-rdma, rds-devel

On 3/9/2017 1:20 AM, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
>
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
> Change from v1 to v2:
>   remove ibmr NULL test.
>
While posting updates, please post entire series and also
give some time between two posts.


>  net/rds/ib_fmr.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
> index 4fe8f4f..249ae1c 100644
> --- a/net/rds/ib_fmr.c
> +++ b/net/rds/ib_fmr.c
> @@ -78,12 +78,9 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device *rds_ibdev, int npages)
>  	return ibmr;
>
>  out_no_cigar:
> -	if (ibmr) {
> -		if (fmr->fmr)
> -			ib_dealloc_fmr(fmr->fmr);
> -		kfree(ibmr);
> -	}
> +	kfree(ibmr);
>  	atomic_dec(&pool->item_count);
> +
No need of the extra line here.

>  	return ERR_PTR(err);
>  }
>
>

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

* Re: [PATCH 4/5] rds: ib: add the static type to the function
  2017-03-09  7:26 ` [PATCH 4/5] rds: ib: add the static type to the function Zhu Yanjun
@ 2017-03-09 16:52   ` Santosh Shilimkar
  0 siblings, 0 replies; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 16:52 UTC (permalink / raw)
  To: Zhu Yanjun, netdev, linux-rdma, rds-devel

On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
> The function rds_ib_map_fmr is used only in the ib_fmr.c
> file. As such, the static type is added to limit it in this file.
>
> Cc: Joe Jin <joe.jin@oracle.com>
> Cc: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
>  net/rds/ib_fmr.c | 5 +++--
>  net/rds/ib_mr.h  | 2 --
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

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

* Re: [PATCH 5/5] rds: ib: unmap the scatter/gather list when error
       [not found]   ` <1489044405-26150-5-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-03-09 17:00     ` Santosh Shilimkar
  2017-03-09 17:01     ` Santosh Shilimkar
  1 sibling, 0 replies; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 17:00 UTC (permalink / raw)
  To: Zhu Yanjun, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
> When some errors occur, the scatter/gather list mapped to DMA addresses
> should be handled.
>
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 5/5] rds: ib: unmap the scatter/gather list when error
       [not found]   ` <1489044405-26150-5-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
  2017-03-09 17:00     ` Santosh Shilimkar
@ 2017-03-09 17:01     ` Santosh Shilimkar
  1 sibling, 0 replies; 17+ messages in thread
From: Santosh Shilimkar @ 2017-03-09 17:01 UTC (permalink / raw)
  To: Zhu Yanjun, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g



On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
> When some errors occur, the scatter/gather list mapped to DMA addresses
> should be handled.
>
> Cc: Joe Jin <joe.jin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Junxiao Bi <junxiao.bi-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Zhu Yanjun <yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
Looks good.
Acked-by: Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave
       [not found]         ` <65ac3de8-9fdd-b5c5-b7aa-aa4393626551-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
@ 2017-03-12  2:33           ` Yanjun Zhu
  2017-03-12  6:37             ` santosh.shilimkar
  0 siblings, 1 reply; 17+ messages in thread
From: Yanjun Zhu @ 2017-03-12  2:33 UTC (permalink / raw)
  To: Santosh Shilimkar, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	rds-devel-N0ozoZBvEnrZJqsBc5GL+g

Sorry. I have no test case to show some issue.
But from Linux Kernel Development Second Edition by Robert Love.

Use spin_lock_irq is dangerous since spin_unlock_irq unconditionally 
enables interrupts.

We can assume the following scenario:

--->the interrupt is disabled.

         spin_lock_irq(lock_ptr);   <---this will disable interrupt again
         list_del(&ic->ib_node);
         spin_unlock_irq(lock_ptr); <---this will enable interrupt

---->the interrupt is enabled.

our code change the state of interrupt. This will make potential risk.
But spin_lock_irqsave/spin_unlock_irqrestore will not make potential risk.

Zhu Yanjun
On 2017/3/10 0:50, Santosh Shilimkar wrote:
> On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
>> It is difficult to make sure the state of the interrupt when this
>> function is called. As such, it is safer to use spin_lock_irqsave
>> than spin_lock_irq.
>>
> There is no reason to hold irqs  and as such the code path is
> safe from irq context. I don't see need of this change unless
> you have test case which showed some issue.
>
> Regards,
> Santosh
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave
  2017-03-12  2:33           ` Yanjun Zhu
@ 2017-03-12  6:37             ` santosh.shilimkar
  0 siblings, 0 replies; 17+ messages in thread
From: santosh.shilimkar @ 2017-03-12  6:37 UTC (permalink / raw)
  To: Yanjun Zhu, netdev, linux-rdma, rds-devel

On 3/11/17 6:33 PM, Yanjun Zhu wrote:
> Sorry. I have no test case to show some issue.
> But from Linux Kernel Development Second Edition by Robert Love.
>
Yes I know the book and what the API does :D

> Use spin_lock_irq is dangerous since spin_unlock_irq unconditionally
> enables interrupts.
>
> We can assume the following scenario:
>
> --->the interrupt is disabled.
>
>         spin_lock_irq(lock_ptr);   <---this will disable interrupt again
>         list_del(&ic->ib_node);
>         spin_unlock_irq(lock_ptr); <---this will enable interrupt
>
> ---->the interrupt is enabled.
>
> our code change the state of interrupt. This will make potential risk.
> But spin_lock_irqsave/spin_unlock_irqrestore will not make potential risk.
>
ic is well protected for any possible race and hence I asked
if you had any test case. Please re-post the series again with
the subject patch dropper for Dave to pick it up.

Regards,
Santosh

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

end of thread, other threads:[~2017-03-12  6:37 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09  7:26 [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Zhu Yanjun
     [not found] ` <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-03-09  7:26   ` [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave Zhu Yanjun
     [not found]     ` <1489044405-26150-2-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-03-09 16:50       ` Santosh Shilimkar
     [not found]         ` <65ac3de8-9fdd-b5c5-b7aa-aa4393626551-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-03-12  2:33           ` Yanjun Zhu
2017-03-12  6:37             ` santosh.shilimkar
2017-03-09  7:26 ` [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr Zhu Yanjun
2017-03-09  8:16   ` Johannes Thumshirn
     [not found]     ` <cffc93fa-f4e6-e61e-54c6-67d6501dd78c-l3A5Bk7waGM@public.gmane.org>
2017-03-09  9:20       ` [PATCHv2 " Zhu Yanjun
2017-03-09  9:42         ` Johannes Thumshirn
2017-03-09 12:09         ` Yuval Shaia
2017-03-09 16:51         ` Santosh Shilimkar
2017-03-09  7:26 ` [PATCH 4/5] rds: ib: add the static type to the function Zhu Yanjun
2017-03-09 16:52   ` Santosh Shilimkar
2017-03-09  7:26 ` [PATCH 5/5] rds: ib: unmap the scatter/gather list when error Zhu Yanjun
     [not found]   ` <1489044405-26150-5-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
2017-03-09 17:00     ` Santosh Shilimkar
2017-03-09 17:01     ` Santosh Shilimkar
2017-03-09 16:47 ` [PATCH 1/5] rds: ib: drop unnecessary rdma_reject Santosh Shilimkar

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.