2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
> 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@xxxxxxxxxx>
> ---
> 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
>
--
Thanks,
Hyunchul
