[ CC-ed to tech-kern, since I think this is no longer atari-specific. Please
  strip port-atari when sending replies ]


On Tue 11 Aug 1998, Julian Coleman wrote:
> > Uhm, before the call to tsleep in the SUCCESSFULLY_QUEUED case, there are
> > the lines:
> > 	if ((xs->flags & (SCSI_NOSLEEP | SCSI_POLL)) == SCSI_NOSLEEP)
> > 		return (EJUSTRETURN);
> > 
> > In other words, it should never get to the point of calling tsleep...
> 
> Hmm, I'm a little confused that scsipi_done() (i.e. wakeup()) gets called 
> before tsleep(), but ...
> 
> Anyway, the problem is that the SCSI_NOSLEEP flag is set in scsipi_done()
> but is not set by the time the test in SUCCESSFULLY_QUEUED is made.  In
> scsipi_free_xs(), there is the call :
> 
> 	pool_put(&scsipi_pool_xfer_pool, xs);
> 
> Before this call, SCSI_NOSLEEP is set, but afterward it is not.

I think you've just found a race-condition in the scsi code... I think
what happens is this:
    scsipi_execute_xs()
       scsipi_command_direct() = SUCCESSFULLY_QUEUED
               ..... Some interrupts happen along the way causing the
               scsi-command to have finished in the mean time. NOSLEEP
               is set, so no wakeup/return is done, the xs-structure
               is freed instead (argh!).
    -> now the resuming sections of scsipi_execute_xs() use an already
       freed xs....

One way that might work to get around this is the addition of a FREEWHENDONE
flag. So the code in scsipi_execute_xs() looks like:
        ....
       switch (scsipi_command_direct(xs)) {
       case SUCCESSFULLY_QUEUED:
                if ((xs->flags & (SCSI_NOSLEEP | SCSI_POLL)) == SCSI_NOSLEEP) {
                     xs->flags |= SCSI_FREEWHENDONE;
                     return (EJUSTRETURN);
                }
       .....

The tail of scsipi_done() will look like:

       .....
       if (xs->flags & SCSI_FREEWHENDONE)
              scsipi_free_xs(xs, SCSI_NOSLEEP);
       if (bp)
       .....

Leo.