| Message ID | 1-v1-caa70ba3d1ab+1436e-ucmd_mask_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [01/11] RDMA/cxgb4: Remove MW support | expand |
On Sat, Oct 03, 2020 at 08:20:01PM -0300, Jason Gunthorpe wrote: > This driver never enabled IB_USER_VERBS_CMD_ALLOC_MW so memory windows > were not usable from userspace. The kernel side was removed long ago. Drop > this dead code. > > Fixes: feb7c1e38bcc ("IB: remove in-kernel support for memory windows") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 2 - > drivers/infiniband/hw/cxgb4/mem.c | 84 -------------------------- > drivers/infiniband/hw/cxgb4/provider.c | 2 - > 3 files changed, 88 deletions(-) > > diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > index a27899402f59a5..f85477f3b037d2 100644 > --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h > @@ -983,9 +983,7 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type, > u32 max_num_sg); > int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents, > unsigned int *sg_offset); > -int c4iw_dealloc_mw(struct ib_mw *mw); > void c4iw_dealloc(struct uld_ctx *ctx); > -int c4iw_alloc_mw(struct ib_mw *mw, struct ib_udata *udata); > struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, > u64 length, u64 virt, int acc, > struct ib_udata *udata); > diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c > index 42234df896fb2c..a2c71a1d93d5a8 100644 > --- a/drivers/infiniband/hw/cxgb4/mem.c > +++ b/drivers/infiniband/hw/cxgb4/mem.c > @@ -365,22 +365,6 @@ static int dereg_mem(struct c4iw_rdev *rdev, u32 stag, u32 pbl_size, > pbl_size, pbl_addr, skb, wr_waitp); > } > > -static int allocate_window(struct c4iw_rdev *rdev, u32 *stag, u32 pdid, > - struct c4iw_wr_wait *wr_waitp) > -{ > - *stag = T4_STAG_UNSET; > - return write_tpt_entry(rdev, 0, stag, 0, pdid, FW_RI_STAG_MW, 0, 0, 0, > - 0UL, 0, 0, 0, 0, NULL, wr_waitp); > -} > - > -static int deallocate_window(struct c4iw_rdev *rdev, u32 stag, > - struct sk_buff *skb, > - struct c4iw_wr_wait *wr_waitp) > -{ > - return write_tpt_entry(rdev, 1, &stag, 0, 0, 0, 0, 0, 0, 0UL, 0, 0, 0, > - 0, skb, wr_waitp); > -} > - > static int allocate_stag(struct c4iw_rdev *rdev, u32 *stag, u32 pdid, > u32 pbl_size, u32 pbl_addr, > struct c4iw_wr_wait *wr_waitp) > @@ -611,74 +595,6 @@ struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, > return ERR_PTR(err); > } > > -int c4iw_alloc_mw(struct ib_mw *ibmw, struct ib_udata *udata) > -{ > - struct c4iw_mw *mhp = to_c4iw_mw(ibmw); > - struct c4iw_dev *rhp; > - struct c4iw_pd *php; > - u32 mmid; > - u32 stag = 0; > - int ret; > - > - if (ibmw->type != IB_MW_TYPE_1) > - return -EINVAL; > - > - php = to_c4iw_pd(ibmw->pd); > - rhp = php->rhp; > - mhp->wr_waitp = c4iw_alloc_wr_wait(GFP_KERNEL); > - if (!mhp->wr_waitp) > - return -ENOMEM; > - > - mhp->dereg_skb = alloc_skb(SGE_MAX_WR_LEN, GFP_KERNEL); > - if (!mhp->dereg_skb) { > - ret = -ENOMEM; > - goto free_wr_wait; > - } > - > - ret = allocate_window(&rhp->rdev, &stag, php->pdid, mhp->wr_waitp); > - if (ret) > - goto free_skb; > - > - mhp->rhp = rhp; > - mhp->attr.pdid = php->pdid; > - mhp->attr.type = FW_RI_STAG_MW; 75% of "enum fw_ri_stag_type" can be removed too. Thanks
On Mon, Oct 05, 2020 at 08:56:52AM +0300, Leon Romanovsky wrote: > > - mhp->rhp = rhp; > > - mhp->attr.pdid = php->pdid; > > - mhp->attr.type = FW_RI_STAG_MW; > > 75% of "enum fw_ri_stag_type" can be removed too. I think that is the code-gen'd HW API for this driver, I don't mind leaving it. It seems the HW supports MW, just nobody plumbed it through to rdma-core Jason
On Monday, October 10/05/20, 2020 at 21:47:26 +0530, Jason Gunthorpe wrote: > On Mon, Oct 05, 2020 at 08:56:52AM +0300, Leon Romanovsky wrote: > > > > - mhp->rhp = rhp; > > > - mhp->attr.pdid = php->pdid; > > > - mhp->attr.type = FW_RI_STAG_MW; > > > > 75% of "enum fw_ri_stag_type" can be removed too. > > I think that is the code-gen'd HW API for this driver, I don't mind > leaving it. It seems the HW supports MW, just nobody plumbed it > through to rdma-core > Hi Jason, Agreed its dead code as is but Chelsio HW suports MW and we are yet to decide on requirements, we may probably add userspace support for MW in future. Thanks, Bharat
On Fri, Oct 09, 2020 at 10:10:06PM +0530, Potnuri Bharat Teja wrote: > On Monday, October 10/05/20, 2020 at 21:47:26 +0530, Jason Gunthorpe wrote: > > On Mon, Oct 05, 2020 at 08:56:52AM +0300, Leon Romanovsky wrote: > > > > > > - mhp->rhp = rhp; > > > > - mhp->attr.pdid = php->pdid; > > > > - mhp->attr.type = FW_RI_STAG_MW; > > > > > > 75% of "enum fw_ri_stag_type" can be removed too. > > > > I think that is the code-gen'd HW API for this driver, I don't mind > > leaving it. It seems the HW supports MW, just nobody plumbed it > > through to rdma-core > > > Hi Jason, > Agreed its dead code as is but Chelsio HW suports MW and we are yet to decide on > requirements, we may probably add userspace support for MW in future. You can't add userspace support without modifying the kernel since the ucmd_mask was never, set. So when/if you get everything working send a kernel series bringing these functions back. Jason
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h index a27899402f59a5..f85477f3b037d2 100644 --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h @@ -983,9 +983,7 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type, u32 max_num_sg); int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents, unsigned int *sg_offset); -int c4iw_dealloc_mw(struct ib_mw *mw); void c4iw_dealloc(struct uld_ctx *ctx); -int c4iw_alloc_mw(struct ib_mw *mw, struct ib_udata *udata); struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, u64 virt, int acc, struct ib_udata *udata); diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c index 42234df896fb2c..a2c71a1d93d5a8 100644 --- a/drivers/infiniband/hw/cxgb4/mem.c +++ b/drivers/infiniband/hw/cxgb4/mem.c @@ -365,22 +365,6 @@ static int dereg_mem(struct c4iw_rdev *rdev, u32 stag, u32 pbl_size, pbl_size, pbl_addr, skb, wr_waitp); } -static int allocate_window(struct c4iw_rdev *rdev, u32 *stag, u32 pdid, - struct c4iw_wr_wait *wr_waitp) -{ - *stag = T4_STAG_UNSET; - return write_tpt_entry(rdev, 0, stag, 0, pdid, FW_RI_STAG_MW, 0, 0, 0, - 0UL, 0, 0, 0, 0, NULL, wr_waitp); -} - -static int deallocate_window(struct c4iw_rdev *rdev, u32 stag, - struct sk_buff *skb, - struct c4iw_wr_wait *wr_waitp) -{ - return write_tpt_entry(rdev, 1, &stag, 0, 0, 0, 0, 0, 0, 0UL, 0, 0, 0, - 0, skb, wr_waitp); -} - static int allocate_stag(struct c4iw_rdev *rdev, u32 *stag, u32 pdid, u32 pbl_size, u32 pbl_addr, struct c4iw_wr_wait *wr_waitp) @@ -611,74 +595,6 @@ struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start, u64 length, return ERR_PTR(err); } -int c4iw_alloc_mw(struct ib_mw *ibmw, struct ib_udata *udata) -{ - struct c4iw_mw *mhp = to_c4iw_mw(ibmw); - struct c4iw_dev *rhp; - struct c4iw_pd *php; - u32 mmid; - u32 stag = 0; - int ret; - - if (ibmw->type != IB_MW_TYPE_1) - return -EINVAL; - - php = to_c4iw_pd(ibmw->pd); - rhp = php->rhp; - mhp->wr_waitp = c4iw_alloc_wr_wait(GFP_KERNEL); - if (!mhp->wr_waitp) - return -ENOMEM; - - mhp->dereg_skb = alloc_skb(SGE_MAX_WR_LEN, GFP_KERNEL); - if (!mhp->dereg_skb) { - ret = -ENOMEM; - goto free_wr_wait; - } - - ret = allocate_window(&rhp->rdev, &stag, php->pdid, mhp->wr_waitp); - if (ret) - goto free_skb; - - mhp->rhp = rhp; - mhp->attr.pdid = php->pdid; - mhp->attr.type = FW_RI_STAG_MW; - mhp->attr.stag = stag; - mmid = (stag) >> 8; - ibmw->rkey = stag; - if (xa_insert_irq(&rhp->mrs, mmid, mhp, GFP_KERNEL)) { - ret = -ENOMEM; - goto dealloc_win; - } - pr_debug("mmid 0x%x mhp %p stag 0x%x\n", mmid, mhp, stag); - return 0; - -dealloc_win: - deallocate_window(&rhp->rdev, mhp->attr.stag, mhp->dereg_skb, - mhp->wr_waitp); -free_skb: - kfree_skb(mhp->dereg_skb); -free_wr_wait: - c4iw_put_wr_wait(mhp->wr_waitp); - return ret; -} - -int c4iw_dealloc_mw(struct ib_mw *mw) -{ - struct c4iw_dev *rhp; - struct c4iw_mw *mhp; - u32 mmid; - - mhp = to_c4iw_mw(mw); - rhp = mhp->rhp; - mmid = (mw->rkey) >> 8; - xa_erase_irq(&rhp->mrs, mmid); - deallocate_window(&rhp->rdev, mhp->attr.stag, mhp->dereg_skb, - mhp->wr_waitp); - kfree_skb(mhp->dereg_skb); - c4iw_put_wr_wait(mhp->wr_waitp); - return 0; -} - struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type, u32 max_num_sg) { diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c index 4b76f2f3f4e483..3bdcdce9def2da 100644 --- a/drivers/infiniband/hw/cxgb4/provider.c +++ b/drivers/infiniband/hw/cxgb4/provider.c @@ -456,13 +456,11 @@ static const struct ib_device_ops c4iw_dev_ops = { .alloc_hw_stats = c4iw_alloc_stats, .alloc_mr = c4iw_alloc_mr, - .alloc_mw = c4iw_alloc_mw, .alloc_pd = c4iw_allocate_pd, .alloc_ucontext = c4iw_alloc_ucontext, .create_cq = c4iw_create_cq, .create_qp = c4iw_create_qp, .create_srq = c4iw_create_srq, - .dealloc_mw = c4iw_dealloc_mw, .dealloc_pd = c4iw_deallocate_pd, .dealloc_ucontext = c4iw_dealloc_ucontext, .dereg_mr = c4iw_dereg_mr,
This driver never enabled IB_USER_VERBS_CMD_ALLOC_MW so memory windows were not usable from userspace. The kernel side was removed long ago. Drop this dead code. Fixes: feb7c1e38bcc ("IB: remove in-kernel support for memory windows") Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 2 - drivers/infiniband/hw/cxgb4/mem.c | 84 -------------------------- drivers/infiniband/hw/cxgb4/provider.c | 2 - 3 files changed, 88 deletions(-)