* [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
[parent not found: <1489044405-26150-1-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* [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
[parent not found: <1489044405-26150-2-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* 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
[parent not found: <65ac3de8-9fdd-b5c5-b7aa-aa4393626551-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* 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
* [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
* 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
[parent not found: <cffc93fa-f4e6-e61e-54c6-67d6501dd78c-l3A5Bk7waGM@public.gmane.org>]
* [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: [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
* [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
* 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
* [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
[parent not found: <1489044405-26150-5-git-send-email-yanjun.zhu-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* 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 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
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.