| Message ID | 20230515201056.1591140-1-rpearsonhpe@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [for-next] RDMA/rxe: Fix double free in rxe_qp.c | expand |
Hello, On 5/16/23 04:10, Bob Pearson wrote: > A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr() > at line 715 in rxe_qp.c. This patch corrects that behavior. > > A newer patch from Guoqing Jiang recommends replacing all spin_lock > calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since > apparently the blktests test suite can call the kernel verbs APIs > while in hard interrupt state. This patch needs to be applied first > and Guoqing's patch modified to accommodate this small change. If you don't mind, I will send a patch set with your patch as first one, then refresh mine. Which means we don't need to keep the second paragraph in commit message, what do you think? > Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/ > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > index c5451a4488ca..245dd36638c7 100644 > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > @@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask) > if (qp->attr.sq_draining) { > spin_unlock_bh(&qp->state_lock); > cond_resched(); > + } else { > + spin_unlock_bh(&qp->state_lock); > } > - spin_unlock_bh(&qp->state_lock); > > return 0; > } Looks good, Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev> Thanks, Guoqing
On 5/15/23 21:10, Guoqing Jiang wrote: > Hello, > > On 5/16/23 04:10, Bob Pearson wrote: >> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr() >> at line 715 in rxe_qp.c. This patch corrects that behavior. >> >> A newer patch from Guoqing Jiang recommends replacing all spin_lock >> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since >> apparently the blktests test suite can call the kernel verbs APIs >> while in hard interrupt state. This patch needs to be applied first >> and Guoqing's patch modified to accommodate this small change. > > If you don't mind, I will send a patch set with your patch as first one, then > refresh mine. Which means we don't need to keep the second paragraph > in commit message, what do you think? > >> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock") >> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/ >> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> >> --- >> drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c >> index c5451a4488ca..245dd36638c7 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_qp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c >> @@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask) >> if (qp->attr.sq_draining) { >> spin_unlock_bh(&qp->state_lock); >> cond_resched(); >> + } else { >> + spin_unlock_bh(&qp->state_lock); >> } >> - spin_unlock_bh(&qp->state_lock); >> return 0; >> } > > Looks good, Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev> > > Thanks, > Guoqing Guoqing, I don't care how we do it. Perhaps we should leave it up to the maintainers. Just needs to be done. Bob
On Mon, May 15, 2023 at 03:10:57PM -0500, Bob Pearson wrote: > A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr() > at line 715 in rxe_qp.c. This patch corrects that behavior. > > A newer patch from Guoqing Jiang recommends replacing all spin_lock > calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since > apparently the blktests test suite can call the kernel verbs APIs > while in hard interrupt state. This patch needs to be applied first > and Guoqing's patch modified to accommodate this small change. > > Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/ > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Applied to for-rc, thanks Jason
On 5/16/23 19:11, Jason Gunthorpe wrote: > On Mon, May 15, 2023 at 03:10:57PM -0500, Bob Pearson wrote: >> A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr() >> at line 715 in rxe_qp.c. This patch corrects that behavior. >> >> A newer patch from Guoqing Jiang recommends replacing all spin_lock >> calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since >> apparently the blktests test suite can call the kernel verbs APIs >> while in hard interrupt state. This patch needs to be applied first >> and Guoqing's patch modified to accommodate this small change. >> >> Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock") >> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/ >> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> >> --- >> drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > Applied to for-rc, thanks > > Jason Thanks
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c index c5451a4488ca..245dd36638c7 100644 --- a/drivers/infiniband/sw/rxe/rxe_qp.c +++ b/drivers/infiniband/sw/rxe/rxe_qp.c @@ -712,8 +712,9 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct ib_qp_attr *attr, int mask) if (qp->attr.sq_draining) { spin_unlock_bh(&qp->state_lock); cond_resched(); + } else { + spin_unlock_bh(&qp->state_lock); } - spin_unlock_bh(&qp->state_lock); return 0; }
A recent patch can cause a double spin_unlock_bh() in rxe_qp_to_attr() at line 715 in rxe_qp.c. This patch corrects that behavior. A newer patch from Guoqing Jiang recommends replacing all spin_lock calls for qp->state_lock to spin_(un)lock_irqsave(restore)() since apparently the blktests test suite can call the kernel verbs APIs while in hard interrupt state. This patch needs to be applied first and Guoqing's patch modified to accommodate this small change. Fixes: f605f26ea196 ("RDMA/rxe: Protect QP state with qp->state_lock") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/linux-rdma/27773078-40ce-414f-8b97-781954da9f25@kili.mountain/ Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> --- drivers/infiniband/sw/rxe/rxe_qp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)