| Message ID | 20210604230558.4812-1-rpearsonhpe@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [for-next] RDMA/rxe: Fix qp reference counting for atomic ops | expand |
On 6/4/21 6:05 PM, Bob Pearson wrote: > Currently the rdma_rxe driver attempts to protect atomic responder > resources by taking a reference to the qp which is only freed when the > resource is recycled for a new read or atomic operation. This means that > in normal circumstances there is almost always an extra qp reference > once an atomic operation has been executed which prevents cleaning up > the qp and associated pd and cqs when the qp is destroyed. > > This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > destroyed while a peer is retrying an atomic op it will cause the > operation to fail which is acceptable. > > Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com> > Fixes: 86af61764151 ("IB/rxe: remove unnecessary skb_clone") > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_qp.c | 1 - > drivers/infiniband/sw/rxe/rxe_resp.c | 2 -- > 2 files changed, 3 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > index 34ae957a315c..b6d83d82e4f9 100644 > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > @@ -136,7 +136,6 @@ static void free_rd_atomic_resources(struct rxe_qp *qp) > void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res) > { > if (res->type == RXE_ATOMIC_MASK) { > - rxe_drop_ref(qp); > kfree_skb(res->atomic.skb); > } else if (res->type == RXE_READ_MASK) { > if (res->read.mr) > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index 2b220659bddb..39dc39be586e 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -966,8 +966,6 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt, > goto out; > } > > - rxe_add_ref(qp); > - > res = &qp->resp.resources[qp->resp.res_head]; > free_rd_atomic_resource(qp, res); > rxe_advance_resp_resource(qp); > There is another way to fix this which is to add cleanup_rd_atomic_resources to rxe_qp_destroy() but it has the exact same behavior because it removes the responder resource causing a retried operation to fail. You can't add it to the qp cleanup routines because you never get there until the ref count goes to zero. I think the above is the cleanest solution. Bob
On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: > > Currently the rdma_rxe driver attempts to protect atomic responder > resources by taking a reference to the qp which is only freed when the > resource is recycled for a new read or atomic operation. This means that > in normal circumstances there is almost always an extra qp reference > once an atomic operation has been executed which prevents cleaning up > the qp and associated pd and cqs when the qp is destroyed. > > This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is Not sure if it is a good way to fix this problem by removing the call to rxe_add_ref. Because taking a reference to the qp is to protect atomic responder resources. Removing rxe_add_ref is to decrease the protection of the atomic responder resources. Zhu Yanjun > destroyed while a peer is retrying an atomic op it will cause the > operation to fail which is acceptable. > > Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com> > Fixes: 86af61764151 ("IB/rxe: remove unnecessary skb_clone") > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_qp.c | 1 - > drivers/infiniband/sw/rxe/rxe_resp.c | 2 -- > 2 files changed, 3 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c > index 34ae957a315c..b6d83d82e4f9 100644 > --- a/drivers/infiniband/sw/rxe/rxe_qp.c > +++ b/drivers/infiniband/sw/rxe/rxe_qp.c > @@ -136,7 +136,6 @@ static void free_rd_atomic_resources(struct rxe_qp *qp) > void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res) > { > if (res->type == RXE_ATOMIC_MASK) { > - rxe_drop_ref(qp); > kfree_skb(res->atomic.skb); > } else if (res->type == RXE_READ_MASK) { > if (res->read.mr) > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index 2b220659bddb..39dc39be586e 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -966,8 +966,6 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt, > goto out; > } > > - rxe_add_ref(qp); > - > res = &qp->resp.resources[qp->resp.res_head]; > free_rd_atomic_resource(qp, res); > rxe_advance_resp_resource(qp); > -- > 2.30.2 >
On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: > On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: > > > > Currently the rdma_rxe driver attempts to protect atomic responder > > resources by taking a reference to the qp which is only freed when the > > resource is recycled for a new read or atomic operation. This means that > > in normal circumstances there is almost always an extra qp reference > > once an atomic operation has been executed which prevents cleaning up > > the qp and associated pd and cqs when the qp is destroyed. > > > > This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > > call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > > Not sure if it is a good way to fix this problem by removing the call > to rxe_add_ref. > Because taking a reference to the qp is to protect atomic responder resources. > > Removing rxe_add_ref is to decrease the protection of the atomic > responder resources. All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. Thanks
On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: > > On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: > > > > > > Currently the rdma_rxe driver attempts to protect atomic responder > > > resources by taking a reference to the qp which is only freed when the > > > resource is recycled for a new read or atomic operation. This means that > > > in normal circumstances there is almost always an extra qp reference > > > once an atomic operation has been executed which prevents cleaning up > > > the qp and associated pd and cqs when the qp is destroyed. > > > > > > This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > > > call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > > > > Not sure if it is a good way to fix this problem by removing the call > > to rxe_add_ref. > > Because taking a reference to the qp is to protect atomic responder resources. > > > > Removing rxe_add_ref is to decrease the protection of the atomic > > responder resources. > > All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. > I made tests with this commit. After this commit is applied, this problem disappeared. Zhu Yanjun > Thanks
On 6/7/2021 3:16 AM, Zhu Yanjun wrote: > On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: >> Currently the rdma_rxe driver attempts to protect atomic responder >> resources by taking a reference to the qp which is only freed when the >> resource is recycled for a new read or atomic operation. This means that >> in normal circumstances there is almost always an extra qp reference >> once an atomic operation has been executed which prevents cleaning up >> the qp and associated pd and cqs when the qp is destroyed. >> >> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the >> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > Not sure if it is a good way to fix this problem by removing the call > to rxe_add_ref. > Because taking a reference to the qp is to protect atomic responder resources. There is no good way to 'protect' responder resources. They are there to recover from a lost response packet which can occur multiple times. The peer can retry the atomic request an unpredictable number of times. There are no acks for response packets. So the QP just has to leave it in place until the max number of read/atomic requests has been received and then it re-uses the responder resource. So to 'protect' the responder resource means you have to leave the QP around potentially forever which is the root cause of the bug. In fact you should treat a retry of a read/atomic the same as a new request and when the QP is destroyed it stops responding to new requests. There is a reference to the QP taken by a received packet which lasts until the packet is freed to prevent kernel seg faults if the QP is destroyed while a request is in process of being responded to. > > Removing rxe_add_ref is to decrease the protection of the atomic > responder resources. > > Zhu Yanjun > >> destroyed while a peer is retrying an atomic op it will cause the >> operation to fail which is acceptable. >> >> Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com> >> Fixes: 86af61764151 ("IB/rxe: remove unnecessary skb_clone") >> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> >> --- >> drivers/infiniband/sw/rxe/rxe_qp.c | 1 - >> drivers/infiniband/sw/rxe/rxe_resp.c | 2 -- >> 2 files changed, 3 deletions(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c >> index 34ae957a315c..b6d83d82e4f9 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_qp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c >> @@ -136,7 +136,6 @@ static void free_rd_atomic_resources(struct rxe_qp *qp) >> void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res) >> { >> if (res->type == RXE_ATOMIC_MASK) { >> - rxe_drop_ref(qp); >> kfree_skb(res->atomic.skb); >> } else if (res->type == RXE_READ_MASK) { >> if (res->read.mr) >> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c >> index 2b220659bddb..39dc39be586e 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_resp.c >> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c >> @@ -966,8 +966,6 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt, >> goto out; >> } >> >> - rxe_add_ref(qp); >> - >> res = &qp->resp.resources[qp->resp.res_head]; >> free_rd_atomic_resource(qp, res); >> rxe_advance_resp_resource(qp); >> -- >> 2.30.2 >>
On 6/7/2021 6:12 AM, Zhu Yanjun wrote: > On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: >> On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: >>> On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: >>>> Currently the rdma_rxe driver attempts to protect atomic responder >>>> resources by taking a reference to the qp which is only freed when the >>>> resource is recycled for a new read or atomic operation. This means that >>>> in normal circumstances there is almost always an extra qp reference >>>> once an atomic operation has been executed which prevents cleaning up >>>> the qp and associated pd and cqs when the qp is destroyed. >>>> >>>> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the >>>> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is >>> Not sure if it is a good way to fix this problem by removing the call >>> to rxe_add_ref. >>> Because taking a reference to the qp is to protect atomic responder resources. >>> >>> Removing rxe_add_ref is to decrease the protection of the atomic >>> responder resources. >> All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. >> > I made tests with this commit. After this commit is applied, this > problem disappeared. You were testing MW when you saw this bug. Does that mean that now MW is working for you? > > Zhu Yanjun > >> Thanks
On Tue, Jun 8, 2021 at 12:14 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: > > > On 6/7/2021 6:12 AM, Zhu Yanjun wrote: > > On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: > >> On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: > >>> On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: > >>>> Currently the rdma_rxe driver attempts to protect atomic responder > >>>> resources by taking a reference to the qp which is only freed when the > >>>> resource is recycled for a new read or atomic operation. This means that > >>>> in normal circumstances there is almost always an extra qp reference > >>>> once an atomic operation has been executed which prevents cleaning up > >>>> the qp and associated pd and cqs when the qp is destroyed. > >>>> > >>>> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > >>>> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > >>> Not sure if it is a good way to fix this problem by removing the call > >>> to rxe_add_ref. > >>> Because taking a reference to the qp is to protect atomic responder resources. > >>> > >>> Removing rxe_add_ref is to decrease the protection of the atomic > >>> responder resources. > >> All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. > >> > > I made tests with this commit. After this commit is applied, this > > problem disappeared. > You were testing MW when you saw this bug. Does that mean that now MW is > working for you? Your MW patches are huge. After these patches are applied, I found 2 problems in my test environment. So IMO, can you send the test cases about MW to rdma-core? So we can verify these MW patches with them. In previous mails, you mentioned these MW test cases. Thanks a lot. Zhu Yanjun > > > > Zhu Yanjun > > > >> Thanks
On 6/7/2021 8:39 PM, Zhu Yanjun wrote: > On Tue, Jun 8, 2021 at 12:14 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: >> >> On 6/7/2021 6:12 AM, Zhu Yanjun wrote: >>> On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: >>>> On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: >>>>> On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: >>>>>> Currently the rdma_rxe driver attempts to protect atomic responder >>>>>> resources by taking a reference to the qp which is only freed when the >>>>>> resource is recycled for a new read or atomic operation. This means that >>>>>> in normal circumstances there is almost always an extra qp reference >>>>>> once an atomic operation has been executed which prevents cleaning up >>>>>> the qp and associated pd and cqs when the qp is destroyed. >>>>>> >>>>>> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the >>>>>> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is >>>>> Not sure if it is a good way to fix this problem by removing the call >>>>> to rxe_add_ref. >>>>> Because taking a reference to the qp is to protect atomic responder resources. >>>>> >>>>> Removing rxe_add_ref is to decrease the protection of the atomic >>>>> responder resources. >>>> All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. >>>> >>> I made tests with this commit. After this commit is applied, this >>> problem disappeared. >> You were testing MW when you saw this bug. Does that mean that now MW is >> working for you? > Your MW patches are huge. After these patches are applied, I found 2 > problems in my test environment. The trace you showed looked like the pyverbs tests all passed and then there were leaked QP/PD/CQ. I also saw those. After fixing the QP reference count bug (not in MW) I did not see any errors from the pyverbs tests of MW. Or any other errors for that matter. What was the other problem? Was that the memory barrier one (also not in MW)? Mostly I want to know if you currently see any errors in the kernel related to MW. The test case bug (in test_qpex.py) is a separate issue that is not a rxe bug at all. Bob > So IMO, can you send the test cases about MW to rdma-core? So we can > verify these MW patches with them. > > In previous mails, you mentioned these MW test cases. > > Thanks a lot. > Zhu Yanjun > >>> Zhu Yanjun >>> >>>> Thanks
On Tue, Jun 8, 2021 at 10:01 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: > > > On 6/7/2021 8:39 PM, Zhu Yanjun wrote: > > On Tue, Jun 8, 2021 at 12:14 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: > >> > >> On 6/7/2021 6:12 AM, Zhu Yanjun wrote: > >>> On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: > >>>> On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: > >>>>> On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: > >>>>>> Currently the rdma_rxe driver attempts to protect atomic responder > >>>>>> resources by taking a reference to the qp which is only freed when the > >>>>>> resource is recycled for a new read or atomic operation. This means that > >>>>>> in normal circumstances there is almost always an extra qp reference > >>>>>> once an atomic operation has been executed which prevents cleaning up > >>>>>> the qp and associated pd and cqs when the qp is destroyed. > >>>>>> > >>>>>> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > >>>>>> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > >>>>> Not sure if it is a good way to fix this problem by removing the call > >>>>> to rxe_add_ref. > >>>>> Because taking a reference to the qp is to protect atomic responder resources. > >>>>> > >>>>> Removing rxe_add_ref is to decrease the protection of the atomic > >>>>> responder resources. > >>>> All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. > >>>> > >>> I made tests with this commit. After this commit is applied, this > >>> problem disappeared. > >> You were testing MW when you saw this bug. Does that mean that now MW is > >> working for you? > > Your MW patches are huge. After these patches are applied, I found 2 > > problems in my test environment. > > The trace you showed looked like the pyverbs tests all passed and then > there were leaked QP/PD/CQ. I also saw those. After fixing the QP > reference count bug (not in MW) I did not see any errors from the > pyverbs tests of MW. Or any other errors for that matter. What was the > other problem? Was that the memory barrier one (also not in MW)? > > Mostly I want to know if you currently see any errors in the kernel > related to MW. The test case bug (in test_qpex.py) is a separate issue The current test cases in rdma-core just confirm a regression in RXE. Zhu Yanjun > that is not a rxe bug at all. > > Bob > > > So IMO, can you send the test cases about MW to rdma-core? So we can > > verify these MW patches with them. > > > > In previous mails, you mentioned these MW test cases. > > > > Thanks a lot. > > Zhu Yanjun > > > >>> Zhu Yanjun > >>> > >>>> Thanks
On 6/7/2021 10:48 PM, Zhu Yanjun wrote: > On Tue, Jun 8, 2021 at 10:01 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: >> >> On 6/7/2021 8:39 PM, Zhu Yanjun wrote: >>> On Tue, Jun 8, 2021 at 12:14 AM Pearson, Robert B <rpearsonhpe@gmail.com> wrote: >>>> On 6/7/2021 6:12 AM, Zhu Yanjun wrote: >>>>> On Mon, Jun 7, 2021 at 7:03 PM Leon Romanovsky <leon@kernel.org> wrote: >>>>>> On Mon, Jun 07, 2021 at 04:16:37PM +0800, Zhu Yanjun wrote: >>>>>>> On Sat, Jun 5, 2021 at 7:07 AM Bob Pearson <rpearsonhpe@gmail.com> wrote: >>>>>>>> Currently the rdma_rxe driver attempts to protect atomic responder >>>>>>>> resources by taking a reference to the qp which is only freed when the >>>>>>>> resource is recycled for a new read or atomic operation. This means that >>>>>>>> in normal circumstances there is almost always an extra qp reference >>>>>>>> once an atomic operation has been executed which prevents cleaning up >>>>>>>> the qp and associated pd and cqs when the qp is destroyed. >>>>>>>> >>>>>>>> This patch removes the call to rxe_add_ref() in send_atomic_ack() and the >>>>>>>> call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is >>>>>>> Not sure if it is a good way to fix this problem by removing the call >>>>>>> to rxe_add_ref. >>>>>>> Because taking a reference to the qp is to protect atomic responder resources. >>>>>>> >>>>>>> Removing rxe_add_ref is to decrease the protection of the atomic >>>>>>> responder resources. >>>>>> All those rxe_add_ref/rxe_drop_ref in RXE are horrid. It will be good to delete them all. >>>>>> >>>>> I made tests with this commit. After this commit is applied, this >>>>> problem disappeared. >>>> You were testing MW when you saw this bug. Does that mean that now MW is >>>> working for you? >>> Your MW patches are huge. After these patches are applied, I found 2 >>> problems in my test environment. >> The trace you showed looked like the pyverbs tests all passed and then >> there were leaked QP/PD/CQ. I also saw those. After fixing the QP >> reference count bug (not in MW) I did not see any errors from the >> pyverbs tests of MW. Or any other errors for that matter. What was the >> other problem? Was that the memory barrier one (also not in MW)? >> >> Mostly I want to know if you currently see any errors in the kernel >> related to MW. The test case bug (in test_qpex.py) is a separate issue > The current test cases in rdma-core just confirm a regression in RXE. > > Zhu Yanjun Which test cases are you referring to. Currently all test cases either pass or are skipped because they are not supported with one single exception. That test in test_qpex.py is *not* a regression. It used to skip until I added support for the extended MW bind operation to the user code today. It now fails because the test is actually wrong. It didn't set the access flags for the MR to support bind MW so the driver fails the WR with a bind MW error which is the correct behavior. The traditional QP WR API (ibv_post_send) exercises the same exact functionality of the driver and they all set the MR access correctly and pass. There are *no* actual errors being reported by the rdma-core tests. Bob > >> that is not a rxe bug at all. >> >> Bob >> >>> So IMO, can you send the test cases about MW to rdma-core? So we can >>> verify these MW patches with them. >>> >>> In previous mails, you mentioned these MW test cases. >>> >>> Thanks a lot. >>> Zhu Yanjun >>> >>>>> Zhu Yanjun >>>>> >>>>>> Thanks
On Fri, Jun 04, 2021 at 06:05:59PM -0500, Bob Pearson wrote: > Currently the rdma_rxe driver attempts to protect atomic responder > resources by taking a reference to the qp which is only freed when the > resource is recycled for a new read or atomic operation. This means that > in normal circumstances there is almost always an extra qp reference > once an atomic operation has been executed which prevents cleaning up > the qp and associated pd and cqs when the qp is destroyed. > > This patch removes the call to rxe_add_ref() in send_atomic_ack() and the > call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is > destroyed while a peer is retrying an atomic op it will cause the > operation to fail which is acceptable. > > Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com> > Fixes: 86af61764151 ("IB/rxe: remove unnecessary skb_clone") > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> > --- > drivers/infiniband/sw/rxe/rxe_qp.c | 1 - > drivers/infiniband/sw/rxe/rxe_resp.c | 2 -- > 2 files changed, 3 deletions(-) Applied to for-next, thanks Jason
diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c index 34ae957a315c..b6d83d82e4f9 100644 --- a/drivers/infiniband/sw/rxe/rxe_qp.c +++ b/drivers/infiniband/sw/rxe/rxe_qp.c @@ -136,7 +136,6 @@ static void free_rd_atomic_resources(struct rxe_qp *qp) void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res) { if (res->type == RXE_ATOMIC_MASK) { - rxe_drop_ref(qp); kfree_skb(res->atomic.skb); } else if (res->type == RXE_READ_MASK) { if (res->read.mr) diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index 2b220659bddb..39dc39be586e 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -966,8 +966,6 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt, goto out; } - rxe_add_ref(qp); - res = &qp->resp.resources[qp->resp.res_head]; free_rd_atomic_resource(qp, res); rxe_advance_resp_resource(qp);
Currently the rdma_rxe driver attempts to protect atomic responder resources by taking a reference to the qp which is only freed when the resource is recycled for a new read or atomic operation. This means that in normal circumstances there is almost always an extra qp reference once an atomic operation has been executed which prevents cleaning up the qp and associated pd and cqs when the qp is destroyed. This patch removes the call to rxe_add_ref() in send_atomic_ack() and the call to rxe_drop_ref() in free_rd_atomic_resource(). If the qp is destroyed while a peer is retrying an atomic op it will cause the operation to fail which is acceptable. Reported-by: Zhu Yanjun <zyjzyj2000@gmail.com> Fixes: 86af61764151 ("IB/rxe: remove unnecessary skb_clone") Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> --- drivers/infiniband/sw/rxe/rxe_qp.c | 1 - drivers/infiniband/sw/rxe/rxe_resp.c | 2 -- 2 files changed, 3 deletions(-)