* [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.