[PATCH] Eliminate signals entirely from the mid-layer

[PATCH] Eliminate signals entirely from the mid-layer

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

the attached patch removes SCSI's use of signals for killing the error handler 
thread.  It also:

- changes the eh_notify semaphore to a completion
- adds complete_and_exit() to the thread
- uses a host structure flag to signal thread death instead of the signal

I'm currently travelling, so could someone test it out for me?  Thanks,

James



[-- Attachment #2: tmp.diff --]
[-- Type: text/plain , Size: 5782 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.947   -> 1.948  
#	drivers/scsi/scsi_error.c	1.26    -> 1.27   
#	drivers/scsi/hosts.c	1.42    -> 1.43   
#	drivers/scsi/hosts.h	1.47    -> 1.48   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/01/10	jejb@mulgrave.(none)	1.948
# remove SCSI's use of signals for killing the error handler thread
# 
# - change the eh_notify semaphore to a completion
# - add complete_and_exit() to the thread
# - use a host structure flag to signal thread death instead of the signal
# --------------------------------------------
#
diff -Nru a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
--- a/drivers/scsi/hosts.c	Fri Jan 10 16:14:02 2003
+++ b/drivers/scsi/hosts.c	Fri Jan 10 16:14:02 2003
@@ -36,6 +36,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
+#include <linux/completion.h>
 
 #define __KERNEL_SYSCALLS__
 
@@ -335,10 +336,10 @@
 	 * Next, kill the kernel error recovery thread for this host.
 	 */
 	if (shost->ehandler) {
-		DECLARE_MUTEX_LOCKED(sem);
+		DECLARE_COMPLETION(sem);
 		shost->eh_notify = &sem;
-		send_sig(SIGPWR, shost->ehandler, 1);
-		down(&sem);
+		up(shost->eh_wait);
+		wait_for_completion(&sem);
 		shost->eh_notify = NULL;
 	}
 
@@ -368,7 +369,7 @@
 {
 	struct Scsi_Host *shost, *shost_scr;
 	int gfp_mask;
-	DECLARE_MUTEX_LOCKED(sem);
+	DECLARE_COMPLETION(sem);
 
         /* Check to see if this host has any error handling facilities */
         if(shost_tp->eh_strategy_handler == NULL &&
@@ -464,7 +465,7 @@
 	 * Now wait for the kernel error thread to initialize itself
 	 * as it might be needed when we scan the bus.
 	 */
-	down(&sem);
+	wait_for_completion(&sem);
 	shost->eh_notify = NULL;
 
 	shost->hostt->present++;
diff -Nru a/drivers/scsi/hosts.h b/drivers/scsi/hosts.h
--- a/drivers/scsi/hosts.h	Fri Jan 10 16:14:02 2003
+++ b/drivers/scsi/hosts.h	Fri Jan 10 16:14:02 2003
@@ -381,11 +381,12 @@
     struct task_struct    * ehandler;  /* Error recovery thread. */
     struct semaphore      * eh_wait;   /* The error recovery thread waits on
                                           this. */
-    struct semaphore      * eh_notify; /* wait for eh to begin */
+    struct completion     * eh_notify; /* wait for eh to begin or end */
     struct semaphore      * eh_action; /* Wait for specific actions on the
                                           host. */
     unsigned int            eh_active:1; /* Indicates the eh thread is awake and active if
                                           this is true. */
+    unsigned int            eh_kill:1; /* set when killing the eh thread */
     wait_queue_head_t       host_wait;
     Scsi_Host_Template    * hostt;
     atomic_t                host_active; /* commands checked out */
diff -Nru a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
--- a/drivers/scsi/scsi_error.c	Fri Jan 10 16:14:02 2003
+++ b/drivers/scsi/scsi_error.c	Fri Jan 10 16:14:02 2003
@@ -27,6 +27,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/smp_lock.h>
+#include <linux/completion.h>
 
 #define __KERNEL_SYSCALLS__
 
@@ -41,24 +42,6 @@
 
 #include <scsi/scsi_ioctl.h> /* grr */
 
-/*
- * We must always allow SHUTDOWN_SIGS.  Even if we are not a module,
- * the host drivers that we are using may be loaded as modules, and
- * when we unload these,  we need to ensure that the error handler thread
- * can be shut down.
- *
- * Note - when we unload a module, we send a SIGHUP.  We mustn't
- * enable SIGTERM, as this is how the init shuts things down when you
- * go to single-user mode.  For that matter, init also sends SIGKILL,
- * so we mustn't enable that one either.  We use SIGHUP instead.  Other
- * options would be SIGPWR, I suppose.
- *
- * Changed behavior 1/1/2003 - it turns out, that SIGHUP can get sent
- * to error handlers from a process responsible for their creation.
- * To sidestep that issue, we now use SIGPWR as suggested above.
- */
-#define SHUTDOWN_SIGS	(sigmask(SIGPWR))
-
 #ifdef DEBUG
 #define SENSE_TIMEOUT SCSI_TIMEOUT
 #else
@@ -1593,12 +1576,10 @@
 	int rtn;
 	DECLARE_MUTEX_LOCKED(sem);
 
-	/*
-	 * We only listen to signals if the HA was loaded as a module.
-	 * If the HA was compiled into the kernel, then we don't listen
-	 * to any signals.
-	 */
-	siginitsetinv(&current->blocked, SHUTDOWN_SIGS);
+	spin_lock_irq(&current->sig->siglock);
+	sigfillset(&current->blocked);
+	recalc_sigpending();
+	spin_unlock_irq(&current->sig->siglock);
 
 	lock_kernel();
 
@@ -1624,7 +1605,7 @@
 	 */
 	SCSI_LOG_ERROR_RECOVERY(3, printk("Wake up parent of scsi_eh_%d\n",shost->host_no));
 
-	up(shost->eh_notify);
+	complete(shost->eh_notify);
 
 	while (1) {
 		/*
@@ -1644,7 +1625,7 @@
 		 * semaphores isn't unreasonable.
 		 */
 		down_interruptible(&sem);
-		if (signal_pending(current))
+		if (shost->eh_kill)
 			break;
 
 		SCSI_LOG_ERROR_RECOVERY(1, printk("Error handler scsi_eh_%d waking up\n",shost->host_no));
@@ -1695,13 +1676,9 @@
 	/*
 	 * If anyone is waiting for us to exit (i.e. someone trying to unload
 	 * a driver), then wake up that process to let them know we are on
-	 * the way out the door.  This may be overkill - I *think* that we
-	 * could probably just unload the driver and send the signal, and when
-	 * the error handling thread wakes up that it would just exit without
-	 * needing to touch any memory associated with the driver itself.
+	 * the way out the door.
 	 */
-	if (shost->eh_notify != NULL)
-		up(shost->eh_notify);
+	complete_and_exit(shost->eh_notify, 0);
 }
 
 /**

Re: [PATCH] Eliminate signals entirely from the mid-layer

[Mike Anderson]

James Bottomley [James.Bottomley@steeleye.com] wrote:
> the attached patch removes SCSI's use of signals for killing the error handler 
> thread.  It also:
> 
> - changes the eh_notify semaphore to a completion
> - adds complete_and_exit() to the thread
> - uses a host structure flag to signal thread death instead of the signal
> 
> I'm currently travelling, so could someone test it out for me?  Thanks,

I will try to test it today or tomorrow. I am already setup for some
error handler fault tests for a qlogicisp error handling update.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] Eliminate signals entirely from the mid-layer

[James Bottomley]

andmike@us.ibm.com said:
> I will try to test it today or tomorrow. I am already setup for some
> error handler fault tests for a qlogicisp error handling update. 

Thanks.  I forgot to say that it applies on top of Willem Riede's signal 
changes patch.

I've pushed it up to scsi-misc-2.5 to make the dependencies easier to manage.

James

Re: [PATCH] Eliminate signals entirely from the mid-layer

[Willem Riede]

On 2003.01.10 16:19 James Bottomley wrote:
> the attached patch removes SCSI's use of signals for killing the error handler 
> thread.  It also:
> 
> - changes the eh_notify semaphore to a completion
> - adds complete_and_exit() to the thread
> - uses a host structure flag to signal thread death instead of the signal
> 
> I'm currently travelling, so could someone test it out for me?  Thanks,
> 
Sorry, James,

When I apply your patch, rmmod ide-scsi hangs. This is my log:

Jan 11 09:08:06 fallguy kernel: Error handler scsi_eh_2 waking up
Jan 11 09:08:06 fallguy kernel: Total of 0 commands on 0 devices require eh work
Jan 11 09:08:06 fallguy kernel: scsi_eh_get_sense: checking to see if we need to request sense
Jan 11 09:08:06 fallguy kernel: scsi_restart_operations: waking up host to restart
Jan 11 09:08:06 fallguy kernel: Error handler scsi_eh_2 sleeping

and ps shows:

...
040     0   236     1  15   0     0    0 down_i SW   ?          0:00 [scsi_eh_2]
...
100     0   946   887  16   0  1324  380 scsi_u D    pts/0      0:00 rmmod ide-scsi
044     0   947   946  16   0     0    0 do_exi Z    pts/0      0:00 [kmodule0? <defunct>]
...

Regards, Willem Riede.

Re: [PATCH] Eliminate signals entirely from the mid-layer

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

wrlk@riede.org said:
> When I apply your patch, rmmod ide-scsi hangs. This is my log: 

Well, we all have these embarassing things we forget from time to time (like 
remembering actually to tell the daemon to die...).  Can you try the attached 
as an additional patch?

Thanks,

James


[-- Attachment #2: tmp.diff --]
[-- Type: text/plain , Size: 365 bytes --]

===== drivers/scsi/hosts.c 1.43 vs edited =====
--- 1.43/drivers/scsi/hosts.c	Fri Jan 10 16:13:38 2003
+++ edited/drivers/scsi/hosts.c	Sat Jan 11 10:39:20 2003
@@ -338,6 +338,7 @@
 	if (shost->ehandler) {
 		DECLARE_COMPLETION(sem);
 		shost->eh_notify = &sem;
+		shost->eh_kill = 1;
 		up(shost->eh_wait);
 		wait_for_completion(&sem);
 		shost->eh_notify = NULL;

Re: [PATCH] Eliminate signals entirely from the mid-layer

[Mike Anderson]

James Bottomley [James.Bottomley@steeleye.com] wrote:
> wrlk@riede.org said:
> > When I apply your patch, rmmod ide-scsi hangs. This is my log: 
> 
> Well, we all have these embarassing things we forget from time to time (like 
> remembering actually to tell the daemon to die...).  Can you try the attached 
> as an additional patch?

I applied your patch plus this last change to my 2.5.55 view and the error
handler is exiting on rmmod of scsi_debug.

I did some drive pulls and the error handler woke up and returned to
sleep. (The errors where not cleared, but that is not related to your
patch)

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] Eliminate signals entirely from the mid-layer

[Willem Riede]

On 2003.01.11 10:44 James Bottomley wrote:
> wrlk@riede.org said:
> > When I apply your patch, rmmod ide-scsi hangs. This is my log: 
> 
> Well, we all have these embarassing things we forget from time to time (like 
> remembering actually to tell the daemon to die...).  Can you try the attached 
> as an additional patch?
> 
Success!

With this change rmmod completes, the error handler exits when it is supposed to,
I can rmmod / modprobe ide-scsi repeatedly until I tire of it, and signals sent
deliberately to the error handler process with kill have no effect.

Regards, Willem Riede.