| Message ID | 20140422112202.GA7440@shamir-pc (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
> I was out of work so I tested the patch "librdmacm: lazy initialization > for ib devices" today. The patch works fine for my test case. The only > concern I have is that I think the patch might leak verbs context. I > think that any call to ucma_init_device must be done with mutex locked. > Please correct me if I am wrong. I think you're correct; the lock needs to move up. I'll apply your fix. Thanks! -- 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
On Tue, Apr 22, 2014 at 06:35:45PM +0000, Hefty, Sean wrote: > > I was out of work so I tested the patch "librdmacm: lazy initialization > > for ib devices" today. The patch works fine for my test case. The only > > concern I have is that I think the patch might leak verbs context. I > > think that any call to ucma_init_device must be done with mutex locked. > > Please correct me if I am wrong. > > I think you're correct; the lock needs to move up. I'll apply your fix. Thanks! librdmacm git (git://git.openfabrics.org/~shefty/librdmacm.git) currently only has the lazy initialization patch (23ffef0 librdmacm: Support lazy initialization) but this patch is missing. can you please add this fix to the git if you approve it? thanks. -- 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
> librdmacm git (git://git.openfabrics.org/~shefty/librdmacm.git) > currently only has the lazy initialization patch (23ffef0 librdmacm: > Support lazy initialization) but this patch is missing. can you please > add this fix to the git if you approve it? thanks. I pushed the fix upstream. Thanks! -- 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
diff --git a/src/cma.c b/src/cma.c index 0dc229e..42769cf 100644 --- a/src/cma.c +++ b/src/cma.c @@ -416,10 +416,13 @@ static int ucma_get_device(struct cma_id_private *id_priv, uint64_t guid) return ERR(ENODEV); match: - if ((ret = ucma_init_device(cma_dev))) + pthread_mutex_lock(&mut); + + if ((ret = ucma_init_device(cma_dev))) { + pthread_mutex_unlock(&mut); return ret; + } - pthread_mutex_lock(&mut); if (!cma_dev->refcnt++) { cma_dev->pd = ibv_alloc_pd(cma_dev->verbs); if (!cma_dev->pd) {
Sean, I was out of work so I tested the patch "librdmacm: lazy initialization for ib devices" today. The patch works fine for my test case. The only concern I have is that I think the patch might leak verbs context. I think that any call to ucma_init_device must be done with mutex locked. Please correct me if I am wrong. Thanks, Shamir Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com> --- src/cma.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)