[SCSI] Fix st oops with new scsi_execute infrastructure
authorKai Makisara <Kai.Makisara@kolumbus.fi>
Sun, 13 Nov 2005 08:04:44 +0000 (10:04 +0200)
committerJames Bottomley <jejb@mulgrave.(none)>
Thu, 15 Dec 2005 23:17:03 +0000 (15:17 -0800)
Patch from Kai minus last sg_segs clearing which was merged already.

> > Was there a oops or lockup or any debug output you can send me? I will try
> > some more large request tests with scsi_debug. You also have to compile your
> > kernel with SCSI_MAX_PHYS_SEGMENTS == 255 to get larger requests now.
>
It was an oops in sgl_unmap_user_pages(). The reason is this:

/* XXX: just for debug. Remove when PageReserved is removed */
BUG_ON(PageReserved(page));

I was using /dev/zero as input and it triggers this. When I used a file as
input, this did not trigger. Should this BUG_ON be removed?

In the same log I noticed that there was another ->sg_segs inconsistency.
Also, the field ->last_SRpnt was not reset when scsi_execute_async()
failed. This caused the error message "Async command already active"
later and prevented proper close.

While doing the changes, I noticed that the current code (since
2.6.0-test4) does not set the pages dirty when reading with direct i/o.

All of these st problems (including the one I sent earlier) are fixed in
the patch at the end of this message. These fixes should probably be
included already in 2.6.15.

After these fixes, the tape seems to operate as expected. Without other
changes, the largest block size with sym53c896 SCSI adapter is 384 kB. The
maximum number of sg segments is set to 96 and clustering is disabled in
the driver. 96 x 4 kB = 384 kB. OK.

I enabled clustering and set max_sectors to 10000 in the SCSI HBA driver.
Now the block size limit is 5000 kB as expected.

Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
drivers/scsi/st.c

index 26e13dc7bcdb14090a7b3a4370609a002d353dae..2096d137f5eeb70ec88d95a1621c45bc5f6bcb38 100644 (file)
@@ -511,9 +511,11 @@ st_do_scsi(struct st_request * SRpnt, struct scsi_tape * STp, unsigned char *cmd
 
        if (scsi_execute_async(STp->device, cmd, direction,
                        &((STp->buffer)->sg[0]), bytes, (STp->buffer)->sg_segs,
-                       timeout, retries, SRpnt, st_sleep_done, GFP_KERNEL))
+                              timeout, retries, SRpnt, st_sleep_done, GFP_KERNEL)) {
                /* could not allocate the buffer or request was too large */
                (STp->buffer)->syscall_result = (-EBUSY);
+               (STp->buffer)->last_SRpnt = NULL;
+       }
        else if (do_wait) {
                wait_for_completion(waiting);
                SRpnt->waiting = NULL;
@@ -1449,14 +1451,15 @@ static int setup_buffering(struct scsi_tape *STp, const char __user *buf,
 
 
 /* Can be called more than once after each setup_buffer() */
-static void release_buffering(struct scsi_tape *STp)
+static void release_buffering(struct scsi_tape *STp, int is_read)
 {
        struct st_buffer *STbp;
 
        STbp = STp->buffer;
        if (STbp->do_dio) {
-               sgl_unmap_user_pages(&(STbp->sg[0]), STbp->do_dio, 0);
+               sgl_unmap_user_pages(&(STbp->sg[0]), STbp->do_dio, is_read);
                STbp->do_dio = 0;
+               STbp->sg_segs = 0;
        }
 }
 
@@ -1729,7 +1732,7 @@ st_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
  out:
        if (SRpnt != NULL)
                st_release_request(SRpnt);
-       release_buffering(STp);
+       release_buffering(STp, 0);
        up(&STp->lock);
 
        return retval;
@@ -1787,7 +1790,7 @@ static long read_tape(struct scsi_tape *STp, long count,
        SRpnt = *aSRpnt;
        SRpnt = st_do_scsi(SRpnt, STp, cmd, bytes, DMA_FROM_DEVICE,
                           STp->device->timeout, MAX_RETRIES, 1);
-       release_buffering(STp);
+       release_buffering(STp, 1);
        *aSRpnt = SRpnt;
        if (!SRpnt)
                return STbp->syscall_result;
@@ -2058,7 +2061,7 @@ st_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
                SRpnt = NULL;
        }
        if (do_dio) {
-               release_buffering(STp);
+               release_buffering(STp, 1);
                STbp->buffer_bytes = 0;
        }
        up(&STp->lock);