scsi code and jiffy wraps

Andrea Arcangeli (andrea@e-mind.com)
Wed, 2 Dec 1998 00:53:24 +0100 (CET)


Eric, I am trying to fix the last_reset problem in the scsi code. The
problem is that last_reset == 0 can' t be considered a special value
(because jiffy first or before will wrap around). This is my latest code
against 2.1.130. I would like if you could comment or find problems in it.

I am going to hack ppa.c (my only scsi host) to generate custom scsi reset
and so have a way to test the code... (David any hint btw ? ;).

I also seen the new scsi_done() that should fix the scsi_done() stack
recursion issue (because the new command will be sent from a bh handler if
I interpret correctly the mark_bh()...). I seen also the new nice scsi
error handler kernel thread. Unfortunately there are no device that are
using the new inteface yet ;))..

The stack recursion was a really bad thing for device driver no irq driven
as ppa (that would like to use the ppa_command callback). Unfortunately
ppa will still have to use the queuecommand and the ppa_interrupt()
(software) using bh handlers (or better timers) to run "light", because
sending a single scsi command via parallel port is a long task and we can'
t hog the CPU for so much time ;).

Index: linux/drivers/scsi/atari_scsi.c
diff -u linux/drivers/scsi/atari_scsi.c:1.1.1.1 linux/drivers/scsi/atari_scsi.c:1.1.1.1.2.1
--- linux/drivers/scsi/atari_scsi.c:1.1.1.1 Fri Nov 20 00:02:45 1998
+++ linux/drivers/scsi/atari_scsi.c Fri Nov 20 00:15:30 1998
@@ -892,7 +892,7 @@
NCR5380_write( INITIATOR_COMMAND_REG, ICR_BASE );
NCR5380_read( RESET_PARITY_INTERRUPT_REG );

- for( end = jiffies + AFTER_RESET_DELAY; jiffies < end; )
+ for( end = jiffies + AFTER_RESET_DELAY; time_before(jiffies,end); )
barrier();

printk( " done\n" );
Index: linux/drivers/scsi/hosts.c
diff -u linux/drivers/scsi/hosts.c:1.1.1.1 linux/drivers/scsi/hosts.c:1.1.1.1.2.4
--- linux/drivers/scsi/hosts.c:1.1.1.1 Fri Nov 20 00:02:39 1998
+++ linux/drivers/scsi/hosts.c Tue Dec 1 18:31:45 1998
@@ -581,7 +581,8 @@
next_scsi_host++;
retval->host_queue = NULL;
retval->host_wait = NULL;
- retval->last_reset = 0;
+ retval->resetting = 0;
+ /* retval->last_reset don' t need to be set because resetting is 0 -arca */
retval->irq = 0;
retval->dma_channel = 0xff;

Index: linux/drivers/scsi/hosts.h
diff -u linux/drivers/scsi/hosts.h:1.1.1.1 linux/drivers/scsi/hosts.h:1.1.1.1.2.1
--- linux/drivers/scsi/hosts.h:1.1.1.1 Fri Nov 20 00:02:39 1998
+++ linux/drivers/scsi/hosts.h Tue Dec 1 18:31:45 1998
@@ -320,6 +320,7 @@
/* public: */
unsigned short extra_bytes;
unsigned short host_no; /* Used for IOCTL_GET_IDLUN, /proc/scsi et al. */
+ int resetting; /* if set, it means that last_reset is a valid value */
unsigned long last_reset;


Index: linux/drivers/scsi/scsi_obsolete.c
diff -u linux/drivers/scsi/scsi_obsolete.c:1.1.1.1 linux/drivers/scsi/scsi_obsolete.c:1.1.1.1.2.3
--- linux/drivers/scsi/scsi_obsolete.c:1.1.1.1 Fri Nov 20 00:02:54 1998
+++ linux/drivers/scsi/scsi_obsolete.c Tue Dec 1 18:31:46 1998
@@ -607,10 +607,9 @@
if ((++SCpnt->retries) < SCpnt->allowed)
{
if ((SCpnt->retries >= (SCpnt->allowed >> 1))
- /* FIXME: last_reset == 0 is allowed
- * && !(SCpnt->host->last_reset > 0 */ &&
- time_before(jiffies, SCpnt->host->last_reset
- + MIN_RESET_PERIOD)
+ && (SCpnt->host->resetting &&
+ time_after(jiffies, SCpnt->host->last_reset
+ + MIN_RESET_PERIOD))
&& !(SCpnt->flags & WAS_RESET))
{
printk("scsi%d channel %d : resetting for second half of retries.\n",
@@ -936,6 +935,12 @@
}

host->last_reset = jiffies;
+ host->resetting = 1;
+ /*
+ * I suppose that the host reset callback will not play
+ * with the resetting field. We have just set the resetting
+ * flag here. -arca
+ */
temp = host->hostt->reset(SCpnt, reset_flags);
/*
This test allows the driver to introduce an additional bus
@@ -954,7 +959,13 @@
{
if (!host->block) host->host_busy++;
host->last_reset = jiffies;
+ host->resetting = 1;
SCpnt->flags |= (WAS_RESET | IS_RESETTING);
+ /*
+ * I suppose that the host reset callback will not play
+ * with the resetting field. We have just set the resetting
+ * flag here. -arca
+ */
temp = host->hostt->reset(SCpnt, reset_flags);
if (time_before(host->last_reset, jiffies) ||
(time_after(host->last_reset, jiffies + 20 * HZ)))
Index: linux/drivers/scsi/sd.c
diff -u linux/drivers/scsi/sd.c:1.1.1.1 linux/drivers/scsi/sd.c:1.1.1.1.2.1
--- linux/drivers/scsi/sd.c:1.1.1.1 Fri Nov 20 00:02:40 1998
+++ linux/drivers/scsi/sd.c Fri Nov 20 00:15:32 1998
@@ -1126,8 +1126,8 @@
unsigned char cmd[10];
char nbuff[6];
unsigned char *buffer;
- unsigned long spintime;
- int the_result, retries;
+ unsigned long spintime_value = 0;
+ int the_result, retries, spintime;
Scsi_Cmnd * SCpnt;

/*
@@ -1222,16 +1222,18 @@
SCpnt->request.sem = NULL;
}

- spintime = jiffies;
+ spintime = 1;
+ spintime_value = jiffies;
}

time1 = jiffies + HZ;
spin_unlock_irq(&io_request_lock);
- while(jiffies < time1); /* Wait 1 second for next try */
+ while(time_before(jiffies, time1)); /* Wait 1 second for next try */
printk( "." );
spin_lock_irq(&io_request_lock);
}
- } while(the_result && spintime && spintime+100*HZ > jiffies);
+ } while(the_result && spintime &&
+ time_after(spintime_value+100*HZ, jiffies));
if (spintime) {
if (the_result)
printk( "not responding...\n" );
Index: linux/drivers/scsi/scsi_error.c
diff -u linux/drivers/scsi/scsi_error.c:1.1.1.1 linux/drivers/scsi/scsi_error.c:1.1.1.1.2.1
--- linux/drivers/scsi/scsi_error.c:1.1.1.1 Fri Nov 20 00:02:54 1998
+++ linux/drivers/scsi/scsi_error.c Fri Nov 20 00:15:31 1998
@@ -142,7 +142,6 @@
SCSI_LOG_ERROR_RECOVERY(5,printk("Clearing timer for command %p\n", SCset));

SCset->eh_timeout.data = (unsigned long) NULL;
- SCset->eh_timeout.expires = 0;
SCset->eh_timeout.function = NULL;

return rtn;

Comments and bug discovery are (as always) welcome.

Andrea Arcangeli

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/