cifs: No need to send SIGKILL to demux_thread during umount

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 <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/cifs/connect.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)

fs/cifs/connect.c +0 -15

index 03ed8a0..b4b6d10 100644
@@ -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);

Quoted 3 lines

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.

Quoted 92 lines

---------- Forwarded message ----------
From: Jeff Layton <[email protected]>
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 <[email protected]>
Cc: Steve French <[email protected]>, Shirish Pargaonkar <[email protected]>, Pavel Shilovsky <[email protected]>, [email protected], Ashish Sangwan <[email protected]>

Quoted 54 lines · On Wed, 20 Aug 2014 19:39:17 +0900 Namjae Jeon…

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 <[email protected]>

Signed-off-by Ashish Sangwan <[email protected]>

---
 fs/cifs/connect.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)

fs/cifs/connect.c +0 -15

index 03ed8a0..b4b6d10 100644
@@ -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);

fs/cifs/connect.c cifs_put_tcp_session(struct TCP_Server_Info *server) · line 2076

Quoted 10 lines

fs/cifs/connect.c +0 -4

@@ -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 <[email protected]>

--
Thanks,

Steve