| Message ID | 20220722030346.28534-4-linkinjeon@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] ksmbd: replace sessions list in connection with xarray | expand |
2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성: > > ksmbd threads eating masses of cputime when connection is disconnected. > If connection is disconnected, ksmbd thread waits for pending requests > to be processed using schedule_timeout. schedule_timeout() incorrectly > is used, and it is more efficient to use wait_event/wake_up than to check > r_count every time with timeout. > > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/connection.c | 6 +++--- > fs/ksmbd/connection.h | 1 + > fs/ksmbd/oplock.c | 21 ++++++++++----------- > fs/ksmbd/server.c | 1 + > 4 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c > index ce23cc89046e..756ad631c019 100644 > --- a/fs/ksmbd/connection.c > +++ b/fs/ksmbd/connection.c > @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) > conn->outstanding_credits = 0; > > init_waitqueue_head(&conn->req_running_q); > + init_waitqueue_head(&conn->r_count_q); > INIT_LIST_HEAD(&conn->conns_list); > INIT_LIST_HEAD(&conn->requests); > INIT_LIST_HEAD(&conn->async_requests); > @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work) > struct kvec iov[3]; > int iov_idx = 0; > > - ksmbd_conn_try_dequeue_request(work); > if (!work->response_buf) { > pr_err("NULL response header\n"); > return -EINVAL; > @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p) > > out: > /* Wait till all reference dropped to the Server object*/ > - while (atomic_read(&conn->r_count) > 0) > - schedule_timeout(HZ); > + wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0); > + > > unload_nls(conn->local_nls); > if (default_conn_ops.terminate_fn) > diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h > index 5b39f0bdeff8..2e4730457c92 100644 > --- a/fs/ksmbd/connection.h > +++ b/fs/ksmbd/connection.h > @@ -65,6 +65,7 @@ struct ksmbd_conn { > unsigned int outstanding_credits; > spinlock_t credits_lock; > wait_queue_head_t req_running_q; > + wait_queue_head_t r_count_q; > /* Lock to protect requests list*/ > spinlock_t request_lock; > struct list_head requests; > diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c > index 8b5560574d4c..9bb4fb8b80de 100644 > --- a/fs/ksmbd/oplock.c > +++ b/fs/ksmbd/oplock.c > @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) > struct ksmbd_file *fp; > > fp = ksmbd_lookup_durable_fd(br_info->fid); > - if (!fp) { > - atomic_dec(&conn->r_count); > - ksmbd_free_work_struct(work); > - return; > - } > + if (!fp) > + goto out; > > if (allocate_oplock_break_buf(work)) { > pr_err("smb2_allocate_rsp_buf failed! "); > - atomic_dec(&conn->r_count); > ksmbd_fd_put(work, fp); > - ksmbd_free_work_struct(work); > - return; > + goto out; > } > > rsp_hdr = smb2_get_msg(work->response_buf); > @@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) > > ksmbd_fd_put(work, fp); > ksmbd_conn_write(work); > + > +out: > ksmbd_free_work_struct(work); > atomic_dec(&conn->r_count); > + wake_up_all(&conn->r_count_q); I think calling wake_up_all is better if atomic_dec_return(&conn->r_count) == 0. Otherwise, Looks good to me. > } > > /** > @@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk) > > if (allocate_oplock_break_buf(work)) { > ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! "); > - ksmbd_free_work_struct(work); > - atomic_dec(&conn->r_count); > - return; > + goto out; > } > > rsp_hdr = smb2_get_msg(work->response_buf); > @@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct work_struct *wk) > inc_rfc1001_len(work->response_buf, 44); > > ksmbd_conn_write(work); > + > +out: > ksmbd_free_work_struct(work); > atomic_dec(&conn->r_count); > + wake_up_all(&conn->r_count_q); > } > > /** > diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c > index 4cd03d661df0..dfddc8dc1919 100644 > --- a/fs/ksmbd/server.c > +++ b/fs/ksmbd/server.c > @@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk) > ksmbd_conn_try_dequeue_request(work); > ksmbd_free_work_struct(work); > atomic_dec(&conn->r_count); > + wake_up_all(&conn->r_count_q); > } > > /** > -- > 2.25.1 >
2022-07-25 9:48 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > 2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성: >> >> ksmbd threads eating masses of cputime when connection is disconnected. >> If connection is disconnected, ksmbd thread waits for pending requests >> to be processed using schedule_timeout. schedule_timeout() incorrectly >> is used, and it is more efficient to use wait_event/wake_up than to check >> r_count every time with timeout. >> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/connection.c | 6 +++--- >> fs/ksmbd/connection.h | 1 + >> fs/ksmbd/oplock.c | 21 ++++++++++----------- >> fs/ksmbd/server.c | 1 + >> 4 files changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c >> index ce23cc89046e..756ad631c019 100644 >> --- a/fs/ksmbd/connection.c >> +++ b/fs/ksmbd/connection.c >> @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) >> conn->outstanding_credits = 0; >> >> init_waitqueue_head(&conn->req_running_q); >> + init_waitqueue_head(&conn->r_count_q); >> INIT_LIST_HEAD(&conn->conns_list); >> INIT_LIST_HEAD(&conn->requests); >> INIT_LIST_HEAD(&conn->async_requests); >> @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work) >> struct kvec iov[3]; >> int iov_idx = 0; >> >> - ksmbd_conn_try_dequeue_request(work); >> if (!work->response_buf) { >> pr_err("NULL response header\n"); >> return -EINVAL; >> @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p) >> >> out: >> /* Wait till all reference dropped to the Server object*/ >> - while (atomic_read(&conn->r_count) > 0) >> - schedule_timeout(HZ); >> + wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0); >> + >> >> unload_nls(conn->local_nls); >> if (default_conn_ops.terminate_fn) >> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h >> index 5b39f0bdeff8..2e4730457c92 100644 >> --- a/fs/ksmbd/connection.h >> +++ b/fs/ksmbd/connection.h >> @@ -65,6 +65,7 @@ struct ksmbd_conn { >> unsigned int outstanding_credits; >> spinlock_t credits_lock; >> wait_queue_head_t req_running_q; >> + wait_queue_head_t r_count_q; >> /* Lock to protect requests list*/ >> spinlock_t request_lock; >> struct list_head requests; >> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c >> index 8b5560574d4c..9bb4fb8b80de 100644 >> --- a/fs/ksmbd/oplock.c >> +++ b/fs/ksmbd/oplock.c >> @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct >> work_struct *wk) >> struct ksmbd_file *fp; >> >> fp = ksmbd_lookup_durable_fd(br_info->fid); >> - if (!fp) { >> - atomic_dec(&conn->r_count); >> - ksmbd_free_work_struct(work); >> - return; >> - } >> + if (!fp) >> + goto out; >> >> if (allocate_oplock_break_buf(work)) { >> pr_err("smb2_allocate_rsp_buf failed! "); >> - atomic_dec(&conn->r_count); >> ksmbd_fd_put(work, fp); >> - ksmbd_free_work_struct(work); >> - return; >> + goto out; >> } >> >> rsp_hdr = smb2_get_msg(work->response_buf); >> @@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct >> work_struct *wk) >> >> ksmbd_fd_put(work, fp); >> ksmbd_conn_write(work); >> + >> +out: >> ksmbd_free_work_struct(work); >> atomic_dec(&conn->r_count); >> + wake_up_all(&conn->r_count_q); > > I think calling wake_up_all is better if atomic_dec_return(&conn->r_count) > == 0. Are there cases where r_count inc/dec doesn't pair? > Otherwise, Looks good to me. > >> } >> >> /** >> @@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct >> *wk) >> >> if (allocate_oplock_break_buf(work)) { >> ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! "); >> - ksmbd_free_work_struct(work); >> - atomic_dec(&conn->r_count); >> - return; >> + goto out; >> } >> >> rsp_hdr = smb2_get_msg(work->response_buf); >> @@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct >> work_struct *wk) >> inc_rfc1001_len(work->response_buf, 44); >> >> ksmbd_conn_write(work); >> + >> +out: >> ksmbd_free_work_struct(work); >> atomic_dec(&conn->r_count); >> + wake_up_all(&conn->r_count_q); >> } >> >> /** >> diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c >> index 4cd03d661df0..dfddc8dc1919 100644 >> --- a/fs/ksmbd/server.c >> +++ b/fs/ksmbd/server.c >> @@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk) >> ksmbd_conn_try_dequeue_request(work); >> ksmbd_free_work_struct(work); >> atomic_dec(&conn->r_count); >> + wake_up_all(&conn->r_count_q); >> } >> >> /** >> -- >> 2.25.1 >> > > > -- > Thanks, > Hyunchul >
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index ce23cc89046e..756ad631c019 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) conn->outstanding_credits = 0; init_waitqueue_head(&conn->req_running_q); + init_waitqueue_head(&conn->r_count_q); INIT_LIST_HEAD(&conn->conns_list); INIT_LIST_HEAD(&conn->requests); INIT_LIST_HEAD(&conn->async_requests); @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work) struct kvec iov[3]; int iov_idx = 0; - ksmbd_conn_try_dequeue_request(work); if (!work->response_buf) { pr_err("NULL response header\n"); return -EINVAL; @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p) out: /* Wait till all reference dropped to the Server object*/ - while (atomic_read(&conn->r_count) > 0) - schedule_timeout(HZ); + wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0); + unload_nls(conn->local_nls); if (default_conn_ops.terminate_fn) diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 5b39f0bdeff8..2e4730457c92 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -65,6 +65,7 @@ struct ksmbd_conn { unsigned int outstanding_credits; spinlock_t credits_lock; wait_queue_head_t req_running_q; + wait_queue_head_t r_count_q; /* Lock to protect requests list*/ spinlock_t request_lock; struct list_head requests; diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 8b5560574d4c..9bb4fb8b80de 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) struct ksmbd_file *fp; fp = ksmbd_lookup_durable_fd(br_info->fid); - if (!fp) { - atomic_dec(&conn->r_count); - ksmbd_free_work_struct(work); - return; - } + if (!fp) + goto out; if (allocate_oplock_break_buf(work)) { pr_err("smb2_allocate_rsp_buf failed! "); - atomic_dec(&conn->r_count); ksmbd_fd_put(work, fp); - ksmbd_free_work_struct(work); - return; + goto out; } rsp_hdr = smb2_get_msg(work->response_buf); @@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) ksmbd_fd_put(work, fp); ksmbd_conn_write(work); + +out: ksmbd_free_work_struct(work); atomic_dec(&conn->r_count); + wake_up_all(&conn->r_count_q); } /** @@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk) if (allocate_oplock_break_buf(work)) { ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! "); - ksmbd_free_work_struct(work); - atomic_dec(&conn->r_count); - return; + goto out; } rsp_hdr = smb2_get_msg(work->response_buf); @@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct work_struct *wk) inc_rfc1001_len(work->response_buf, 44); ksmbd_conn_write(work); + +out: ksmbd_free_work_struct(work); atomic_dec(&conn->r_count); + wake_up_all(&conn->r_count_q); } /** diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c index 4cd03d661df0..dfddc8dc1919 100644 --- a/fs/ksmbd/server.c +++ b/fs/ksmbd/server.c @@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk) ksmbd_conn_try_dequeue_request(work); ksmbd_free_work_struct(work); atomic_dec(&conn->r_count); + wake_up_all(&conn->r_count_q); } /**
ksmbd threads eating masses of cputime when connection is disconnected. If connection is disconnected, ksmbd thread waits for pending requests to be processed using schedule_timeout. schedule_timeout() incorrectly is used, and it is more efficient to use wait_event/wake_up than to check r_count every time with timeout. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/connection.c | 6 +++--- fs/ksmbd/connection.h | 1 + fs/ksmbd/oplock.c | 21 ++++++++++----------- fs/ksmbd/server.c | 1 + 4 files changed, 15 insertions(+), 14 deletions(-)