Subject: Re: Race in scsipi_execute_xs()? (Was: Re: More on se0)
To: Julian Coleman <J.D.Coleman@newcastle.ac.uk>
From: Leo Weppelman <leo@wau.mis.ah.nl>
List: tech-kern
Date: 08/16/1998 22:07:02
I thought out a somewhat different approach. It doesn't seem to break
anything, a kernel using it is running now ;-)
What I did is giving each scsipi_xfer structure it's own unique generation
number (extra member: u_int gen_number ). The changes to scsipi_base.c
are globally:
   static int gen_counter;

scsipi_get_xs() _and_ scsipi_free_xs()
{
    .....
    xs->gen_number = gen_counter++;
    .....
}

scsipi_execute_xs()
{
     ......
     xs_gen = xs->gen_number;
     switch (scsipi_command_direct(xs)) {
        case SUCCESSFULLY_QUEUED:
              if (xs_gen != xs->gen_number) {
                   /*
                    * Re-used before we got here. Was NOSLEEP case...
                    */
                   return (EJUSTRETURN);
              }
    .......
}

I attached a diff to scsipi_base.c, but this is the scsipi_base.c from the
bouyer-branch and is not yet released AFAIK. I hope you have enough info
to try something out.

Leo.

--- scsipi_base.c.org	Sun Aug 16 21:39:17 1998
+++ scsipi_base.c	Sun Aug 16 21:44:47 1998
@@ -82,6 +82,7 @@
  * a NULL pointer, signifying that no slots were available
  * Note in the link structure, that we are waiting on it.
  */
+static int gen_counter = 0;
 
 struct scsipi_xfer *
 scsipi_get_xs(sc_link, flags)
@@ -106,8 +107,10 @@
 	SC_DEBUG(sc_link, SDEV_DB3, ("calling pool_get\n"));
 	xs = pool_get(&scsipi_xfer_pool,
 	    ((flags & SCSI_NOSLEEP) != 0 ? PR_NOWAIT : PR_WAITOK));
-	if (xs != NULL)
+	if (xs != NULL) {
+		xs->gen_number = gen_counter++;
 		sc_link->openings--;
+	}
 	else {
 		(*sc_link->sc_print_addr)(sc_link);
 		printf("cannot allocate scsipi xs\n");
@@ -139,6 +142,7 @@
 {
 	struct scsipi_link *sc_link = xs->sc_link;
 
+	xs->gen_number = gen_counter++;
 	xs->flags &= ~INUSE;
 	pool_put(&scsipi_xfer_pool, xs);
 
@@ -367,7 +371,7 @@
 scsipi_execute_xs(xs)
 	struct scsipi_xfer *xs;
 {
-	int error;
+	int error, xs_gen;
 	int s;
 
 	xs->flags &= ~ITSDONE;
@@ -398,8 +402,15 @@
 		printf("\n");
 	}
 #endif
+	xs_gen = xs->gen_number;
 	switch (scsipi_command_direct(xs)) {
 	case SUCCESSFULLY_QUEUED:
+		if (xs_gen != xs->gen_number) {
+			/*
+			 * Re-used before we got here. Was NOSLEEP case...
+			 */
+			return (EJUSTRETURN);
+		}
 		if ((xs->flags & (SCSI_NOSLEEP | SCSI_POLL)) == SCSI_NOSLEEP)
 			return (EJUSTRETURN);
 #ifdef DIAGNOSTIC