| Message ID | 1438241568-3685-12-git-send-email-sagig@mellanox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Sagi Grimberg > Sent: Thursday, July 30, 2015 2:33 AM > To: linux-rdma@vger.kernel.org; linux-nfs@vger.kernel.org; target-devel@vger.kernel.org > Subject: [PATCH for-4.3 11/15] iw_cxgb4: Support ib_alloc_mr verb > > Signed-off-by: Sagi Grimberg <sagig@mellanox.com> > --- > drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +++- > drivers/infiniband/hw/cxgb4/mem.c | 12 +++++++++--- > drivers/infiniband/hw/cxgb4/provider.c | 2 +- > 3 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > index cc77844..c7bb38c 100644 > --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > @@ -970,7 +970,9 @@ void c4iw_free_fastreg_pbl(struct ib_fast_reg_page_list *page_list); > struct ib_fast_reg_page_list *c4iw_alloc_fastreg_pbl( > struct ib_device *device, > int page_list_len); > -struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth); > +struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, > + enum ib_mr_type mr_type, > + u32 max_num_sg); > int c4iw_dealloc_mw(struct ib_mw *mw); > struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type); > struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, > diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c > index cff815b..026b91e 100644 > --- a/drivers/infiniband/hw/cxgb4/mem.c > +++ b/drivers/infiniband/hw/cxgb4/mem.c > @@ -853,7 +853,9 @@ int c4iw_dealloc_mw(struct ib_mw *mw) > return 0; > } > > -struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth) > +struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, > + enum ib_mr_type mr_type, > + u32 max_num_sg) > { > struct c4iw_dev *rhp; > struct c4iw_pd *php; > @@ -862,6 +864,10 @@ struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth) > u32 stag = 0; > int ret = 0; > > + if (mr_type != IB_MR_TYPE_MEM_REG || > + max_num_sg > t4_max_fr_depth(use_dsgl)) > + return ERR_PTR(-EINVAL); > + > php = to_c4iw_pd(pd); > rhp = php->rhp; > mhp = kzalloc(sizeof(*mhp), GFP_KERNEL); Hey Sagi/Doug, The above change introduces a regression with NFSRDMA over cxgb4. Prior to this commit, cxgb4 allowed frmr and page list allocations that exceeded the device max. It does enforce the max when processing a fastreg WR though. This is definitely a bug in cxgb4, but was benign. Further, the NFSRDMA server currently allocates frmrs and page_lists using RPCSVC_MAXPAGES for max_num_sg which exceeds the max supported by cxgb4. Thus with the above patch, NFSRDMA doesn't work at all over cxgb4. :( So I need to fix NFSRDMA for sure. But adding a fix on top of the above patch will leave a bisectable window where NFSRDMA is broken on cxgb4. Is it too late to change the above patch so the regression is avoided? Then I can provide a new series of patches to fix the NFS server to mind the device max, and fix cxgb4 to enforce the device max. If it is too much of a pain to alter this patch, then I'll just submit the NFSRDMA fix and live with the bisect issue... Thoughts? Steve. -- 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 Fri, Aug 07, 2015 at 10:06:26AM -0500, Steve Wise wrote: > If it is too much of a pain to alter this patch, then I'll just > submit the NFSRDMA fix and live with the bisect issue... Doug's tree is still to be rebased. So please submit your NFS fix now as ask Doug to merge it before Sagi's series in the final tree. -- 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
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Friday, August 07, 2015 10:13 AM > To: Steve Wise > Cc: 'Sagi Grimberg'; 'Doug Ledford'; linux-rdma@vger.kernel.org; linux-nfs@vger.kernel.org; target-devel@vger.kernel.org > Subject: Re: [PATCH for-4.3 11/15] iw_cxgb4: Support ib_alloc_mr verb > > On Fri, Aug 07, 2015 at 10:06:26AM -0500, Steve Wise wrote: > > If it is too much of a pain to alter this patch, then I'll just > > submit the NFSRDMA fix and live with the bisect issue... > > Doug's tree is still to be rebased. So please submit your NFS > fix now as ask Doug to merge it before Sagi's series in the final > tree. My new NFS fix needs to land before these two: af78181 cxgb3: Support ib_alloc_mr verb b7e06cd iw_cxgb4: Support ib_alloc_mr verb But it will cause the following patch, which is after the above two, to need rework because it hits the same lines: e20684a xprtrdma, svcrdma: Convert to ib_alloc_mr I guess I'll post two patches, the NFS fix that preceeds af78181/ b7e06cd, and a reworked patch to replace e20684a. Is that the way to go in your opinion? Steve. -- 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 8/7/2015 11:19 AM, Steve Wise wrote: > >> -----Original Message----- >> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Christoph Hellwig >> Sent: Friday, August 07, 2015 10:13 AM >> To: Steve Wise >> Cc: 'Sagi Grimberg'; 'Doug Ledford'; linux-rdma@vger.kernel.org; linux-nfs@vger.kernel.org; target-devel@vger.kernel.org >> Subject: Re: [PATCH for-4.3 11/15] iw_cxgb4: Support ib_alloc_mr verb >> >> On Fri, Aug 07, 2015 at 10:06:26AM -0500, Steve Wise wrote: >>> If it is too much of a pain to alter this patch, then I'll just >>> submit the NFSRDMA fix and live with the bisect issue... >> Doug's tree is still to be rebased. So please submit your NFS >> fix now as ask Doug to merge it before Sagi's series in the final >> tree. > My new NFS fix needs to land before these two: > > af78181 cxgb3: Support ib_alloc_mr verb > b7e06cd iw_cxgb4: Support ib_alloc_mr verb > > But it will cause the following patch, which is after the above two, to need rework because it hits the same lines: > > e20684a xprtrdma, svcrdma: Convert to ib_alloc_mr > > I guess I'll post two patches, the NFS fix that preceeds af78181/ b7e06cd, and a reworked patch to replace e20684a. > > Is that the way to go in your opinion? > > Steve. Ignore this. I think I was looking at the wrong staging branch. -- 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 Fri, Aug 07, 2015 at 11:19:59AM -0500, Steve Wise wrote: > I guess I'll post two patches, the NFS fix that preceeds af78181/ b7e06cd, and a reworked patch to replace e20684a. > > Is that the way to go in your opinion? To me this sounds good. We have a couple patches from Jason's series that already need to be replaced, so the tree will need a rebase anyway, so I don't see a problem with replacing ones in Sagi's series either. If Sagi needs to do a repost for some reason he can just include your NFS patch in front of the series. -- 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
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of 'Christoph Hellwig' > Sent: Friday, August 07, 2015 11:26 AM > To: Steve Wise > Cc: 'Christoph Hellwig'; 'Sagi Grimberg'; 'Doug Ledford'; linux-rdma@vger.kernel.org; linux-nfs@vger.kernel.org; target- > devel@vger.kernel.org > Subject: Re: [PATCH for-4.3 11/15] iw_cxgb4: Support ib_alloc_mr verb > > On Fri, Aug 07, 2015 at 11:19:59AM -0500, Steve Wise wrote: > > I guess I'll post two patches, the NFS fix that preceeds af78181/ b7e06cd, and a reworked patch to replace e20684a. > > > > Is that the way to go in your opinion? > > To me this sounds good. We have a couple patches from Jason's series > that already need to be replaced, so the tree will need a rebase anyway, > so I don't see a problem with replacing ones in Sagi's series either. > If Sagi needs to do a repost for some reason he can just include your > NFS patch in front of the series. I misspoke. I had the order reversed. The order is such that we can add my new NFS patch after: e20684a xprtrdma, svcrdma: Convert to ib_alloc_mr and before these: af78181 cxgb3: Support ib_alloc_mr verb b7e06cd iw_cxgb4: Support ib_alloc_mr verb Steve -- 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 Fri, Aug 07, 2015 at 11:29:12AM -0500, Steve Wise wrote: > I misspoke. I had the order reversed. The order is such that we can add my new NFS patch after: > > e20684a xprtrdma, svcrdma: Convert to ib_alloc_mr > > and before these: > > af78181 cxgb3: Support ib_alloc_mr verb > b7e06cd iw_cxgb4: Support ib_alloc_mr verb In general it would be preferable to have it before the ib_alloc_mr conversion, but if it causes more work I doubt it's really worth it. -- 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/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h index cc77844..c7bb38c 100644 --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h @@ -970,7 +970,9 @@ void c4iw_free_fastreg_pbl(struct ib_fast_reg_page_list *page_list); struct ib_fast_reg_page_list *c4iw_alloc_fastreg_pbl( struct ib_device *device, int page_list_len); -struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth); +struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, + enum ib_mr_type mr_type, + u32 max_num_sg); int c4iw_dealloc_mw(struct ib_mw *mw); struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type); struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c index cff815b..026b91e 100644 --- a/drivers/infiniband/hw/cxgb4/mem.c +++ b/drivers/infiniband/hw/cxgb4/mem.c @@ -853,7 +853,9 @@ int c4iw_dealloc_mw(struct ib_mw *mw) return 0; } -struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth) +struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, + enum ib_mr_type mr_type, + u32 max_num_sg) { struct c4iw_dev *rhp; struct c4iw_pd *php; @@ -862,6 +864,10 @@ struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth) u32 stag = 0; int ret = 0; + if (mr_type != IB_MR_TYPE_MEM_REG || + max_num_sg > t4_max_fr_depth(use_dsgl)) + return ERR_PTR(-EINVAL); + php = to_c4iw_pd(pd); rhp = php->rhp; mhp = kzalloc(sizeof(*mhp), GFP_KERNEL); @@ -871,10 +877,10 @@ struct ib_mr *c4iw_alloc_fast_reg_mr(struct ib_pd *pd, int pbl_depth) } mhp->rhp = rhp; - ret = alloc_pbl(mhp, pbl_depth); + ret = alloc_pbl(mhp, max_num_sg); if (ret) goto err1; - mhp->attr.pbl_size = pbl_depth; + mhp->attr.pbl_size = max_num_sg; ret = allocate_stag(&rhp->rdev, &stag, php->pdid, mhp->attr.pbl_size, mhp->attr.pbl_addr); if (ret) diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c index 6eee3d3..7746113 100644 --- a/drivers/infiniband/hw/cxgb4/provider.c +++ b/drivers/infiniband/hw/cxgb4/provider.c @@ -556,7 +556,7 @@ int c4iw_register_device(struct c4iw_dev *dev) dev->ibdev.alloc_mw = c4iw_alloc_mw; dev->ibdev.bind_mw = c4iw_bind_mw; dev->ibdev.dealloc_mw = c4iw_dealloc_mw; - dev->ibdev.alloc_fast_reg_mr = c4iw_alloc_fast_reg_mr; + dev->ibdev.alloc_mr = c4iw_alloc_mr; dev->ibdev.alloc_fast_reg_page_list = c4iw_alloc_fastreg_pbl; dev->ibdev.free_fast_reg_page_list = c4iw_free_fastreg_pbl; dev->ibdev.attach_mcast = c4iw_multicast_attach;
Signed-off-by: Sagi Grimberg <sagig@mellanox.com> --- drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +++- drivers/infiniband/hw/cxgb4/mem.c | 12 +++++++++--- drivers/infiniband/hw/cxgb4/provider.c | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-)