Messages in this thread | | | Date | Sat, 22 Oct 2022 09:09:39 +0800 | Subject | Re: [for-next PATCH v2 1/2] RDMA/rxe: Remove unnecessary mr testing | From | Li Zhijian <> |
| |
On 21/10/2022 22:39, Zhu Yanjun wrote: > On Fri, Oct 21, 2022 at 3:53 PM Li Zhijian <lizhijian@fujitsu.com> wrote: >> Before the testing, we already passed it to rxe_mr_copy() where mr could >> be dereferenced. so this checking is not exactly correct. >> >> I tried to figure out the details how/when mr could be NULL, but failed >> at last. Add a WARN_ON(!mr) to that path to tell us more when it >> happends. > If I get you correctly, you confronted a problem, Not exactly, I removed the mr checking since i think this checking is not correct. the newly added WARN_ON(!mr) is the only once place where the mr can be NULL but not handled correctly. At least with/without this patch, once WARN_ON(!mr) is triggered, kernel will go something wrong.
so i want to place this WARN_ON(!mr) to point to the problem.
Thanks Zhijian
> but you can not figure it out. > So you send it upstream as a patch? > > I am not sure if it is a good idea. > > Zhu Yanjun > >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> drivers/infiniband/sw/rxe/rxe_resp.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c >> index ed5a09e86417..218c14fb07c6 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_resp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c >> @@ -778,6 +778,7 @@ static enum resp_states read_reply(struct rxe_qp *qp, >> if (res->state == rdatm_res_state_new) { >> if (!res->replay) { >> mr = qp->resp.mr; >> + WARN_ON(!mr); >> qp->resp.mr = NULL; >> } else { >> mr = rxe_recheck_mr(qp, res->read.rkey); >> @@ -811,8 +812,7 @@ static enum resp_states read_reply(struct rxe_qp *qp, >> >> rxe_mr_copy(mr, res->read.va, payload_addr(&ack_pkt), >> payload, RXE_FROM_MR_OBJ); >> - if (mr) >> - rxe_put(mr); >> + rxe_put(mr); >> >> if (bth_pad(&ack_pkt)) { >> u8 *pad = payload_addr(&ack_pkt) + payload; >> -- >> 2.31.1 >>
| |