* [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread during umount
@ 2014-08-20 10:39 Namjae Jeon
2014-08-21 11:38 ` Jeff Layton
0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2014-08-20 10:39 UTC (permalink / raw)
To: 'Steve French'
Cc: 'Shirish Pargaonkar', 'Pavel Shilovsky',
linux-cifs-u79uwXL29TY76Z2rM5mHXA, Ashish Sangwan
There is no need to explicitly send SIGKILL to cifs_demultiplex_thread
as it is calling module_put_and_exit to exit cleanly.
socket sk_rcvtimeo is set to 7 HZ so the thread will wake up in 7 seconds and
clean itself.
Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
fs/cifs/connect.c | 19 -------------------
1 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 03ed8a0..b4b6d10 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -837,7 +837,6 @@ cifs_demultiplex_thread(void *p)
struct TCP_Server_Info *server = p;
unsigned int pdu_length;
char *buf = NULL;
- struct task_struct *task_to_wake = NULL;
struct mid_q_entry *mid_entry;
current->flags |= PF_MEMALLOC;
@@ -928,19 +927,7 @@ cifs_demultiplex_thread(void *p)
if (server->smallbuf) /* no sense logging a debug message if NULL */
cifs_small_buf_release(server->smallbuf);
- task_to_wake = xchg(&server->tsk, NULL);
clean_demultiplex_info(server);
-
- /* if server->tsk was NULL then wait for a signal before exiting */
- if (!task_to_wake) {
- set_current_state(TASK_INTERRUPTIBLE);
- while (!signal_pending(current)) {
- schedule();
- set_current_state(TASK_INTERRUPTIBLE);
- }
- set_current_state(TASK_RUNNING);
- }
-
module_put_and_exit(0);
}
@@ -2061,8 +2048,6 @@ cifs_find_tcp_session(struct smb_vol *vol)
static void
cifs_put_tcp_session(struct TCP_Server_Info *server)
{
- struct task_struct *task;
-
spin_lock(&cifs_tcp_ses_lock);
if (--server->srv_count > 0) {
spin_unlock(&cifs_tcp_ses_lock);
@@ -2086,10 +2071,6 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
kfree(server->session_key.response);
server->session_key.response = NULL;
server->session_key.len = 0;
-
- task = xchg(&server->tsk, NULL);
- if (task)
- force_sig(SIGKILL, task);
}
static struct TCP_Server_Info *
--
1.7.7
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread during umount
2014-08-20 10:39 [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread during umount Namjae Jeon
@ 2014-08-21 11:38 ` Jeff Layton
[not found] ` <CAH2r5mviz=Js_rOQ0LJ2eB5fK=D8RB3hyEcyg7MAMukz07y2BQ@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2014-08-21 11:38 UTC (permalink / raw)
To: Namjae Jeon
Cc: 'Steve French', 'Shirish Pargaonkar',
'Pavel Shilovsky',
linux-cifs-u79uwXL29TY76Z2rM5mHXA, Ashish Sangwan
On Wed, 20 Aug 2014 19:39:17 +0900
Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> There is no need to explicitly send SIGKILL to cifs_demultiplex_thread
> as it is calling module_put_and_exit to exit cleanly.
>
> socket sk_rcvtimeo is set to 7 HZ so the thread will wake up in 7 seconds and
> clean itself.
>
> Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> fs/cifs/connect.c | 19 -------------------
> 1 files changed, 0 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 03ed8a0..b4b6d10 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -837,7 +837,6 @@ cifs_demultiplex_thread(void *p)
> struct TCP_Server_Info *server = p;
> unsigned int pdu_length;
> char *buf = NULL;
> - struct task_struct *task_to_wake = NULL;
> struct mid_q_entry *mid_entry;
>
> current->flags |= PF_MEMALLOC;
> @@ -928,19 +927,7 @@ cifs_demultiplex_thread(void *p)
> if (server->smallbuf) /* no sense logging a debug message if NULL */
> cifs_small_buf_release(server->smallbuf);
>
> - task_to_wake = xchg(&server->tsk, NULL);
> clean_demultiplex_info(server);
> -
> - /* if server->tsk was NULL then wait for a signal before exiting */
> - if (!task_to_wake) {
> - set_current_state(TASK_INTERRUPTIBLE);
> - while (!signal_pending(current)) {
> - schedule();
> - set_current_state(TASK_INTERRUPTIBLE);
> - }
> - set_current_state(TASK_RUNNING);
> - }
> -
> module_put_and_exit(0);
> }
>
> @@ -2061,8 +2048,6 @@ cifs_find_tcp_session(struct smb_vol *vol)
> static void
> cifs_put_tcp_session(struct TCP_Server_Info *server)
> {
> - struct task_struct *task;
> -
> spin_lock(&cifs_tcp_ses_lock);
> if (--server->srv_count > 0) {
> spin_unlock(&cifs_tcp_ses_lock);
> @@ -2086,10 +2071,6 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
> kfree(server->session_key.response);
> server->session_key.response = NULL;
> server->session_key.len = 0;
> -
> - task = xchg(&server->tsk, NULL);
> - if (task)
> - force_sig(SIGKILL, task);
> }
>
> static struct TCP_Server_Info *
Looks fine, I think. That code is a leftover from when we used a
different scheme to take down the kthread and I don't think it's
necessary any longer.
Acked-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Fwd: [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread during umount
[not found] ` <CAH2r5mtt0Y8nDSxsv+DXaG8WdtzOT==-fH_fnNTSqEp8HJ8TyQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-09-16 18:47 ` Steve French
0 siblings, 0 replies; 3+ messages in thread
From: Steve French @ 2014-09-16 18:47 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
cc list
---------- Forwarded message ----------
From: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue, Sep 16, 2014 at 1:46 PM
Subject: Re: [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread
during umount
To: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>, Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
reverted in cifs-2.6.git
On Tue, Sep 16, 2014 at 3:14 AM, Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> I think this patch may be slowing down the ability to do rmmod after
>> cifs umount (have to wait about 10-20 seconds) since it thinks the
>> cifs module is in use longer now
> Hi Steve,
>
> your concern is correct, rmmod time is increased from 1 second to 7 seconds.
> The reason is why thread is sleeping interruptible in interuptibble state,
> signal delivering wakes up the thread early otherwise it is wake up after
> 7 seconds after timeout expires..
>
> Sorry, Could you revert this patch ?
>
> Thanks.
>
>>
>>
>> ---------- Forwarded message ----------
>> From: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> Date: Thu, Aug 21, 2014 at 4:38 AM
>> Subject: Re: [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread
>> during umount
>> To: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Cc: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Shirish Pargaonkar
>> <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>,
>> linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>
>>
>> On Wed, 20 Aug 2014 19:39:17 +0900
>> Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>
>> > There is no need to explicitly send SIGKILL to cifs_demultiplex_thread
>> > as it is calling module_put_and_exit to exit cleanly.
>> >
>> > socket sk_rcvtimeo is set to 7 HZ so the thread will wake up in 7 seconds and
>> > clean itself.
>> >
>> > Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> > Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> > ---
>> > fs/cifs/connect.c | 19 -------------------
>> > 1 files changed, 0 insertions(+), 19 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 03ed8a0..b4b6d10 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -837,7 +837,6 @@ cifs_demultiplex_thread(void *p)
>> > struct TCP_Server_Info *server = p;
>> > unsigned int pdu_length;
>> > char *buf = NULL;
>> > - struct task_struct *task_to_wake = NULL;
>> > struct mid_q_entry *mid_entry;
>> >
>> > current->flags |= PF_MEMALLOC;
>> > @@ -928,19 +927,7 @@ cifs_demultiplex_thread(void *p)
>> > if (server->smallbuf) /* no sense logging a debug message if NULL */
>> > cifs_small_buf_release(server->smallbuf);
>> >
>> > - task_to_wake = xchg(&server->tsk, NULL);
>> > clean_demultiplex_info(server);
>> > -
>> > - /* if server->tsk was NULL then wait for a signal before exiting */
>> > - if (!task_to_wake) {
>> > - set_current_state(TASK_INTERRUPTIBLE);
>> > - while (!signal_pending(current)) {
>> > - schedule();
>> > - set_current_state(TASK_INTERRUPTIBLE);
>> > - }
>> > - set_current_state(TASK_RUNNING);
>> > - }
>> > -
>> > module_put_and_exit(0);
>> > }
>> >
>> > @@ -2061,8 +2048,6 @@ cifs_find_tcp_session(struct smb_vol *vol)
>> > static void
>> > cifs_put_tcp_session(struct TCP_Server_Info *server)
>> > {
>> > - struct task_struct *task;
>> > -
>> > spin_lock(&cifs_tcp_ses_lock);
>> > if (--server->srv_count > 0) {
>> > spin_unlock(&cifs_tcp_ses_lock);
>> > @@ -2086,10 +2071,6 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
>> > kfree(server->session_key.response);
>> > server->session_key.response = NULL;
>> > server->session_key.len = 0;
>> > -
>> > - task = xchg(&server->tsk, NULL);
>> > - if (task)
>> > - force_sig(SIGKILL, task);
>> > }
>> >
>> > static struct TCP_Server_Info *
>>
>> Looks fine, I think. That code is a leftover from when we used a
>> different scheme to take down the kthread and I don't think it's
>> necessary any longer.
>>
>> Acked-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>>
>>
>> --
>> Thanks,
>>
>> Steve
>
--
Thanks,
Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-16 18:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-20 10:39 [PATCH 3/7] cifs: No need to send SIGKILL to demux_thread during umount Namjae Jeon
2014-08-21 11:38 ` Jeff Layton
[not found] ` <CAH2r5mviz=Js_rOQ0LJ2eB5fK=D8RB3hyEcyg7MAMukz07y2BQ@mail.gmail.com>
[not found] ` <004101cfd186$33aa37d0$9afea770$@samsung.com>
[not found] ` <CAH2r5mtt0Y8nDSxsv+DXaG8WdtzOT==-fH_fnNTSqEp8HJ8TyQ@mail.gmail.com>
[not found] ` <CAH2r5mtt0Y8nDSxsv+DXaG8WdtzOT==-fH_fnNTSqEp8HJ8TyQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-16 18:47 ` Fwd: " Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.