| Message ID | 20221002030123.11409-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: call ib_drain_qp when disconnected | expand |
On 10/1/2022 11:01 PM, Namjae Jeon wrote: > When disconnected, call ib_drain_qp to cancel all pending work requests > and prevent ksmbd_conn_handler_loop from waiting for a long time > for those work requests to compelete. > > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/transport_rdma.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c > index 0315bca3d53b..096eda9ef873 100644 > --- a/fs/ksmbd/transport_rdma.c > +++ b/fs/ksmbd/transport_rdma.c > @@ -1527,6 +1527,8 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, > } > case RDMA_CM_EVENT_DEVICE_REMOVAL: > case RDMA_CM_EVENT_DISCONNECTED: { > + ib_drain_qp(t->qp); > + > t->status = SMB_DIRECT_CS_DISCONNECTED; > wake_up_interruptible(&t->wait_status); > wake_up_interruptible(&t->wait_reassembly_queue); Because we're now flushing the cancelled work requests, don't we need to also wake up &t->wait_send_pending, to abort any waiters on the send wq? Tom.
2022-10-03 3:11 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 10/1/2022 11:01 PM, Namjae Jeon wrote: >> When disconnected, call ib_drain_qp to cancel all pending work requests >> and prevent ksmbd_conn_handler_loop from waiting for a long time >> for those work requests to compelete. >> >> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/transport_rdma.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c >> index 0315bca3d53b..096eda9ef873 100644 >> --- a/fs/ksmbd/transport_rdma.c >> +++ b/fs/ksmbd/transport_rdma.c >> @@ -1527,6 +1527,8 @@ static int smb_direct_cm_handler(struct rdma_cm_id >> *cm_id, >> } >> case RDMA_CM_EVENT_DEVICE_REMOVAL: >> case RDMA_CM_EVENT_DISCONNECTED: { >> + ib_drain_qp(t->qp); >> + >> t->status = SMB_DIRECT_CS_DISCONNECTED; >> wake_up_interruptible(&t->wait_status); >> wake_up_interruptible(&t->wait_reassembly_queue); > > Because we're now flushing the cancelled work requests, don't > we need to also wake up &t->wait_send_pending, to abort any > waiters on the send wq? Could you please elaborate more how it could be a problem ? send_pending is decreased and wake_up &t->wait_send_pending if it is zero in send_done(). > > Tom. >
On 10/3/2022 10:31 AM, Namjae Jeon wrote: > 2022-10-03 3:11 GMT+09:00, Tom Talpey <tom@talpey.com>: >> On 10/1/2022 11:01 PM, Namjae Jeon wrote: >>> When disconnected, call ib_drain_qp to cancel all pending work requests >>> and prevent ksmbd_conn_handler_loop from waiting for a long time >>> for those work requests to compelete. >>> >>> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> >>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >>> --- >>> fs/ksmbd/transport_rdma.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c >>> index 0315bca3d53b..096eda9ef873 100644 >>> --- a/fs/ksmbd/transport_rdma.c >>> +++ b/fs/ksmbd/transport_rdma.c >>> @@ -1527,6 +1527,8 @@ static int smb_direct_cm_handler(struct rdma_cm_id >>> *cm_id, >>> } >>> case RDMA_CM_EVENT_DEVICE_REMOVAL: >>> case RDMA_CM_EVENT_DISCONNECTED: { >>> + ib_drain_qp(t->qp); >>> + >>> t->status = SMB_DIRECT_CS_DISCONNECTED; >>> wake_up_interruptible(&t->wait_status); >>> wake_up_interruptible(&t->wait_reassembly_queue); >> >> Because we're now flushing the cancelled work requests, don't >> we need to also wake up &t->wait_send_pending, to abort any >> waiters on the send wq? > Could you please elaborate more how it could be a problem ? > send_pending is decreased and wake_up &t->wait_send_pending if it is > zero in send_done(). I took another look at __ib_drain_sq() and yes it does put the qp into ERR, and only steals the single WR that it itself posted. As long as we're not using IB_POLL_DIRECT (which ksmbd isn't), we're good. Reviewed-by: Tom Talpey <tom@talpey.com>
diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c index 0315bca3d53b..096eda9ef873 100644 --- a/fs/ksmbd/transport_rdma.c +++ b/fs/ksmbd/transport_rdma.c @@ -1527,6 +1527,8 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, } case RDMA_CM_EVENT_DEVICE_REMOVAL: case RDMA_CM_EVENT_DISCONNECTED: { + ib_drain_qp(t->qp); + t->status = SMB_DIRECT_CS_DISCONNECTED; wake_up_interruptible(&t->wait_status); wake_up_interruptible(&t->wait_reassembly_queue);