| Message ID | 20220805183523.32044-1-rpearsonhpe@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [v5,for-next,1/2] RDMA/rxe: Set pd early in mr alloc routines | expand |
Bob, Sorry for the late reply. Just back to office from a vacation :) > > Move setting of pd in mr objects ahead of any possible errors so that it will > always be set in rxe_mr_complete() to avoid seg faults when rxe_put(mr_pd(mr)) > is called. > > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> Looks good to me Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
BTW: this patch should be for-rc instead. On 22/08/2022 13:46, lizhijian@fujitsu.com wrote: > Bob, > > Sorry for the late reply. Just back to office from a vacation :) > >> Move setting of pd in mr objects ahead of any possible errors so that it will >> always be set in rxe_mr_complete() to avoid seg faults when rxe_put(mr_pd(mr)) >> is called. >> >> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > Looks good to me > Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> >
On Wed, Aug 31, 2022 at 01:48:08PM +0800, Li Zhijian wrote:
> BTW: this patch should be for-rc instead.
It can't be in -rc without Fixes line and more clear description.
Thanks
On Fri, Aug 05, 2022 at 01:35:24PM -0500, Bob Pearson wrote: > Move setting of pd in mr objects ahead of any possible errors > so that it will always be set in rxe_mr_complete() to avoid > seg faults when rxe_put(mr_pd(mr)) is called. > > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_loc.h | 6 +++--- > drivers/infiniband/sw/rxe/rxe_mr.c | 11 ++++------- > drivers/infiniband/sw/rxe/rxe_verbs.c | 10 +++++++--- > 3 files changed, 14 insertions(+), 13 deletions(-) I see only one patch out of two "v5 for-next 1/2". Where is the second one? Thanks
Leon, On 31/08/2022 14:26, Leon Romanovsky wrote: > On Wed, Aug 31, 2022 at 01:48:08PM +0800, Li Zhijian wrote: >> BTW: this patch should be for-rc instead. > It can't be in -rc without Fixes line and more clear description. Fixes: cf40367961d8 ("RDMA/rxe: Move mr cleanup code to rxe_mr_cleanup()") ? IIRC, This patch fixes a kernel crash newly introduced by v6.0 merge window it's an alternative for my former patch: https://lore.kernel.org/all/42ec06bb-9e97-3b43-5fb9-9801407d301e@fujitsu.com/ So if you agree it should go to for-rc, does Bob need to repost a new one with more details ? > > Thanks
On Wed, Aug 31, 2022 at 04:05:57PM +0800, Li Zhijian wrote: > Leon, > > > On 31/08/2022 14:26, Leon Romanovsky wrote: > > On Wed, Aug 31, 2022 at 01:48:08PM +0800, Li Zhijian wrote: > > > BTW: this patch should be for-rc instead. > > It can't be in -rc without Fixes line and more clear description. > > Fixes: cf40367961d8 ("RDMA/rxe: Move mr cleanup code to rxe_mr_cleanup()") ? > > IIRC, This patch fixes a kernel crash newly introduced by v6.0 merge window > it's an alternative for my former patch: https://lore.kernel.org/all/42ec06bb-9e97-3b43-5fb9-9801407d301e@fujitsu.com/ > So if you agree it should go to for-rc, does Bob need to repost a new one with more details ? Yes, please, especially kernel panic. > > > > > > Thanks >
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h index 22f6cc31d1d6..c2a5c8814a48 100644 --- a/drivers/infiniband/sw/rxe/rxe_loc.h +++ b/drivers/infiniband/sw/rxe/rxe_loc.h @@ -64,10 +64,10 @@ int rxe_mmap(struct ib_ucontext *context, struct vm_area_struct *vma); /* rxe_mr.c */ u8 rxe_get_next_key(u32 last_key); -void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr); -int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova, +void rxe_mr_init_dma(int access, struct rxe_mr *mr); +int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova, int access, struct rxe_mr *mr); -int rxe_mr_init_fast(struct rxe_pd *pd, int max_pages, struct rxe_mr *mr); +int rxe_mr_init_fast(int max_pages, struct rxe_mr *mr); int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length, enum rxe_mr_copy_dir dir); int copy_data(struct rxe_pd *pd, int access, struct rxe_dma_info *dma, diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c index 850b80f5ad8b..af34f198e645 100644 --- a/drivers/infiniband/sw/rxe/rxe_mr.c +++ b/drivers/infiniband/sw/rxe/rxe_mr.c @@ -103,17 +103,16 @@ static int rxe_mr_alloc(struct rxe_mr *mr, int num_buf) return -ENOMEM; } -void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr) +void rxe_mr_init_dma(int access, struct rxe_mr *mr) { rxe_mr_init(access, mr); - mr->ibmr.pd = &pd->ibpd; mr->access = access; mr->state = RXE_MR_STATE_VALID; mr->type = IB_MR_TYPE_DMA; } -int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova, +int rxe_mr_init_user(struct rxe_dev *rxe, u64 start, u64 length, u64 iova, int access, struct rxe_mr *mr) { struct rxe_map **map; @@ -125,7 +124,7 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova, int err; int i; - umem = ib_umem_get(pd->ibpd.device, start, length, access); + umem = ib_umem_get(&rxe->ib_dev, start, length, access); if (IS_ERR(umem)) { pr_warn("%s: Unable to pin memory region err = %d\n", __func__, (int)PTR_ERR(umem)); @@ -175,7 +174,6 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova, } } - mr->ibmr.pd = &pd->ibpd; mr->umem = umem; mr->access = access; mr->length = length; @@ -197,7 +195,7 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova, return err; } -int rxe_mr_init_fast(struct rxe_pd *pd, int max_pages, struct rxe_mr *mr) +int rxe_mr_init_fast(int max_pages, struct rxe_mr *mr) { int err; @@ -208,7 +206,6 @@ int rxe_mr_init_fast(struct rxe_pd *pd, int max_pages, struct rxe_mr *mr) if (err) goto err1; - mr->ibmr.pd = &pd->ibpd; mr->max_buf = max_pages; mr->state = RXE_MR_STATE_FREE; mr->type = IB_MR_TYPE_MEM_REG; diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index e264cf69bf55..6c13be14d723 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -903,7 +903,9 @@ static struct ib_mr *rxe_get_dma_mr(struct ib_pd *ibpd, int access) return ERR_PTR(-ENOMEM); rxe_get(pd); - rxe_mr_init_dma(pd, access, mr); + mr->ibmr.pd = ibpd; + + rxe_mr_init_dma(access, mr); rxe_finalize(mr); return &mr->ibmr; @@ -928,8 +930,9 @@ static struct ib_mr *rxe_reg_user_mr(struct ib_pd *ibpd, rxe_get(pd); + mr->ibmr.pd = ibpd; - err = rxe_mr_init_user(pd, start, length, iova, access, mr); + err = rxe_mr_init_user(rxe, start, length, iova, access, mr); if (err) goto err3; @@ -962,8 +965,9 @@ static struct ib_mr *rxe_alloc_mr(struct ib_pd *ibpd, enum ib_mr_type mr_type, } rxe_get(pd); + mr->ibmr.pd = ibpd; - err = rxe_mr_init_fast(pd, max_num_sg, mr); + err = rxe_mr_init_fast(max_num_sg, mr); if (err) goto err2;
Move setting of pd in mr objects ahead of any possible errors so that it will always be set in rxe_mr_complete() to avoid seg faults when rxe_put(mr_pd(mr)) is called. Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> --- drivers/infiniband/sw/rxe/rxe_loc.h | 6 +++--- drivers/infiniband/sw/rxe/rxe_mr.c | 11 ++++------- drivers/infiniband/sw/rxe/rxe_verbs.c | 10 +++++++--- 3 files changed, 14 insertions(+), 13 deletions(-)