[SCSI] drivers/scsi/st.c: add reference count and related fixes

I have rediffed the patch against 2.6.13-rc5, done a couple of cosmetic
cleanups, and run some tests.  Brian King has acknowledged that it fixes the
problems he has seen. Seems mature enough for inclusion into 2.6.14 (or
later)?

Nate's explanation of the changes:

I've attached patches against 2.6.13rc2. These are basically identical
to my earlier patches, as I found that all issues I'd seen in earlier
kernels still existed in this kernel.

To summarize, the changes are: (more details in my original email)

- add a kref to the scsi_tape structure, and associate reference
counting stuff

- set sr_request->end_io = blk_end_sync_rq so we get notified when an IO
is rejected when the device goes away

- check rq_status when IOs complete, else we don't know that IOs
rejected for a dead device in fact did not complete

- change last_SRpnt so it's set before an async IO is issued (in case
st_sleep_done is bypassed)

- fix a bogus use of last_SRpnt in st_chk_result

Signed-off-by: Nate Dailey <nate.dailey@stratus.com>
Signed-off-by: Kai Makisara <kai.makisara@kolumbus.fi>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 0291a8f..47a5698 100644 (file)
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -17,7 +17,7 @@
    Last modified: 18-JAN-1998 Richard Gooch <rgooch@atnf.csiro.au> Devfs support
  */
 
-static char *verstr = "20050501";
+static char *verstr = "20050802";
 
 #include <linux/module.h>
 
@@ -219,6 +219,12 @@ static int switch_partition(struct scsi_tape *);
 
 static int st_int_ioctl(struct scsi_tape *, unsigned int, unsigned long);
 
+static void scsi_tape_release(struct kref *);
+
+#define to_scsi_tape(obj) container_of(obj, struct scsi_tape, kref)
+
+static DECLARE_MUTEX(st_ref_sem);
+
 \f
 #include "osst_detect.h"
 #ifndef SIGS_FROM_OSST
@@ -230,6 +236,46 @@ static int st_int_ioctl(struct scsi_tape *, unsigned int, unsigned long);
        {"OnStream", "FW-", "", "osst"}
 #endif
 
+static struct scsi_tape *scsi_tape_get(int dev)
+{
+       struct scsi_tape *STp = NULL;
+
+       down(&st_ref_sem);
+       write_lock(&st_dev_arr_lock);
+
+       if (dev < st_dev_max && scsi_tapes != NULL)
+               STp = scsi_tapes[dev];
+       if (!STp) goto out;
+
+       kref_get(&STp->kref);
+
+       if (!STp->device)
+               goto out_put;
+
+       if (scsi_device_get(STp->device))
+               goto out_put;
+
+       goto out;
+
+out_put:
+       kref_put(&STp->kref, scsi_tape_release);
+       STp = NULL;
+out:
+       write_unlock(&st_dev_arr_lock);
+       up(&st_ref_sem);
+       return STp;
+}
+
+static void scsi_tape_put(struct scsi_tape *STp)
+{
+       struct scsi_device *sdev = STp->device;
+
+       down(&st_ref_sem);
+       kref_put(&STp->kref, scsi_tape_release);
+       scsi_device_put(sdev);
+       up(&st_ref_sem);
+}
+
 struct st_reject_data {
        char *vendor;
        char *model;
@@ -311,7 +357,7 @@ static int st_chk_result(struct scsi_tape *STp, struct scsi_request * SRpnt)
                return 0;
 
        cmdstatp = &STp->buffer->cmdstat;
-       st_analyze_sense(STp->buffer->last_SRpnt, cmdstatp);
+       st_analyze_sense(SRpnt, cmdstatp);
 
        if (cmdstatp->have_sense)
                scode = STp->buffer->cmdstat.sense_hdr.sense_key;
@@ -399,10 +445,10 @@ static void st_sleep_done(struct scsi_cmnd * SCpnt)
 
        (STp->buffer)->cmdstat.midlevel_result = SCpnt->result;
        SCpnt->request->rq_status = RQ_SCSI_DONE;
-       (STp->buffer)->last_SRpnt = SCpnt->sc_request;
        DEB( STp->write_pending = 0; )
 
-       complete(SCpnt->request->waiting);
+       if (SCpnt->request->waiting)
+               complete(SCpnt->request->waiting);
 }
 
 /* Do the scsi command. Waits until command performed if do_wait is true.
@@ -412,8 +458,20 @@ static struct scsi_request *
 st_do_scsi(struct scsi_request * SRpnt, struct scsi_tape * STp, unsigned char *cmd,
           int bytes, int direction, int timeout, int retries, int do_wait)
 {
+       struct completion *waiting;
        unsigned char *bp;
 
+       /* if async, make sure there's no command outstanding */
+       if (!do_wait && ((STp->buffer)->last_SRpnt)) {
+               printk(KERN_ERR "%s: Async command already active.\n",
+                      tape_name(STp));
+               if (signal_pending(current))
+                       (STp->buffer)->syscall_result = (-EINTR);
+               else
+                       (STp->buffer)->syscall_result = (-EBUSY);
+               return NULL;
+       }
+
        if (SRpnt == NULL) {
                SRpnt = scsi_allocate_request(STp->device, GFP_ATOMIC);
                if (SRpnt == NULL) {
@@ -427,7 +485,13 @@ st_do_scsi(struct scsi_request * SRpnt, struct scsi_tape * STp, unsigned char *c
                }
        }
 
-       init_completion(&STp->wait);
+       /* If async IO, set last_SRpnt. This ptr tells write_behind_check
+          which IO is outstanding. It's nulled out when the IO completes. */
+       if (!do_wait)
+               (STp->buffer)->last_SRpnt = SRpnt;
+
+       waiting = &STp->wait;
+       init_completion(waiting);
        SRpnt->sr_use_sg = STp->buffer->do_dio || (bytes > (STp->buffer)->frp[0].length);
        if (SRpnt->sr_use_sg) {
                if (!STp->buffer->do_dio)
@@ -438,17 +502,20 @@ st_do_scsi(struct scsi_request * SRpnt, struct scsi_tape * STp, unsigned char *c
                bp = (STp->buffer)->b_data;
        SRpnt->sr_data_direction = direction;
        SRpnt->sr_cmd_len = 0;
-       SRpnt->sr_request->waiting = &(STp->wait);
+       SRpnt->sr_request->waiting = waiting;
        SRpnt->sr_request->rq_status = RQ_SCSI_BUSY;
        SRpnt->sr_request->rq_disk = STp->disk;
+       SRpnt->sr_request->end_io = blk_end_sync_rq;
        STp->buffer->cmdstat.have_sense = 0;
 
        scsi_do_req(SRpnt, (void *) cmd, bp, bytes,
                    st_sleep_done, timeout, retries);
 
        if (do_wait) {
-               wait_for_completion(SRpnt->sr_request->waiting);
+               wait_for_completion(waiting);
                SRpnt->sr_request->waiting = NULL;
+               if (SRpnt->sr_request->rq_status != RQ_SCSI_DONE)
+                       SRpnt->sr_result |= (DRIVER_ERROR << 24);
                (STp->buffer)->syscall_result = st_chk_result(STp, SRpnt);
        }
        return SRpnt;
@@ -465,6 +532,7 @@ static int write_behind_check(struct scsi_tape * STp)
        struct st_buffer *STbuffer;
        struct st_partstat *STps;
        struct st_cmdstatus *cmdstatp;
+       struct scsi_request *SRpnt;
 
        STbuffer = STp->buffer;
        if (!STbuffer->writing)
@@ -478,10 +546,14 @@ static int write_behind_check(struct scsi_tape * STp)
         ) /* end DEB */
 
        wait_for_completion(&(STp->wait));
-       (STp->buffer)->last_SRpnt->sr_request->waiting = NULL;
+       SRpnt = STbuffer->last_SRpnt;
+       STbuffer->last_SRpnt = NULL;
+       SRpnt->sr_request->waiting = NULL;
+       if (SRpnt->sr_request->rq_status != RQ_SCSI_DONE)
+               SRpnt->sr_result |= (DRIVER_ERROR << 24);
 
-       (STp->buffer)->syscall_result = st_chk_result(STp, (STp->buffer)->last_SRpnt);
-       scsi_release_request((STp->buffer)->last_SRpnt);
+       (STp->buffer)->syscall_result = st_chk_result(STp, SRpnt);
+       scsi_release_request(SRpnt);
 
        STbuffer->buffer_bytes -= STbuffer->writing;
        STps = &(STp->ps[STp->partition]);
@@ -1055,25 +1127,20 @@ static int st_open(struct inode *inode, struct file *filp)
         */
        filp->f_mode &= ~(FMODE_PREAD | FMODE_PWRITE);
 
+       if (!(STp = scsi_tape_get(dev)))
+               return -ENXIO;
+
        write_lock(&st_dev_arr_lock);
-       if (dev >= st_dev_max || scsi_tapes == NULL ||
-           ((STp = scsi_tapes[dev]) == NULL)) {
-               write_unlock(&st_dev_arr_lock);
-               return (-ENXIO);
-       }
        filp->private_data = STp;
        name = tape_name(STp);
 
        if (STp->in_use) {
                write_unlock(&st_dev_arr_lock);
+               scsi_tape_put(STp);
                DEB( printk(ST_DEB_MSG "%s: Device already in use.\n", name); )
                return (-EBUSY);
        }
 
-       if(scsi_device_get(STp->device)) {
-               write_unlock(&st_dev_arr_lock);
-               return (-ENXIO);
-       }
        STp->in_use = 1;
        write_unlock(&st_dev_arr_lock);
        STp->rew_at_close = STp->autorew_dev = (iminor(inode) & 0x80) == 0;
@@ -1118,7 +1185,7 @@ static int st_open(struct inode *inode, struct file *filp)
  err_out:
        normalize_buffer(STp->buffer);
        STp->in_use = 0;
-       scsi_device_put(STp->device);
+       scsi_tape_put(STp);
        return retval;
 
 }
@@ -1250,7 +1317,7 @@ static int st_release(struct inode *inode, struct file *filp)
        write_lock(&st_dev_arr_lock);
        STp->in_use = 0;
        write_unlock(&st_dev_arr_lock);
-       scsi_device_put(STp->device);
+       scsi_tape_put(STp);
 
        return result;
 }
@@ -3887,6 +3954,7 @@ static int st_probe(struct device *dev)
                goto out_put_disk;
        }
        memset(tpnt, 0, sizeof(struct scsi_tape));
+       kref_init(&tpnt->kref);
        tpnt->disk = disk;
        sprintf(disk->disk_name, "st%d", i);
        disk->private_data = &tpnt->driver;
@@ -3902,6 +3970,7 @@ static int st_probe(struct device *dev)
                tpnt->tape_type = MT_ISSCSI2;
 
        tpnt->buffer = buffer;
+       tpnt->buffer->last_SRpnt = NULL;
 
        tpnt->inited = 0;
        tpnt->dirty = 0;
@@ -4076,15 +4145,10 @@ static int st_remove(struct device *dev)
                                        tpnt->modes[mode].cdevs[j] = NULL;
                                }
                        }
-                       tpnt->device = NULL;
 
-                       if (tpnt->buffer) {
-                               tpnt->buffer->orig_frp_segs = 0;
-                               normalize_buffer(tpnt->buffer);
-                               kfree(tpnt->buffer);
-                       }
-                       put_disk(tpnt->disk);
-                       kfree(tpnt);
+                       down(&st_ref_sem);
+                       kref_put(&tpnt->kref, scsi_tape_release);
+                       up(&st_ref_sem);
                        return 0;
                }
        }
@@ -4093,6 +4157,34 @@ static int st_remove(struct device *dev)
        return 0;
 }
 
+/**
+ *      scsi_tape_release - Called to free the Scsi_Tape structure
+ *      @kref: pointer to embedded kref
+ *
+ *      st_ref_sem must be held entering this routine.  Because it is
+ *      called on last put, you should always use the scsi_tape_get()
+ *      scsi_tape_put() helpers which manipulate the semaphore directly
+ *      and never do a direct kref_put().
+ **/
+static void scsi_tape_release(struct kref *kref)
+{
+       struct scsi_tape *tpnt = to_scsi_tape(kref);
+       struct gendisk *disk = tpnt->disk;
+
+       tpnt->device = NULL;
+
+       if (tpnt->buffer) {
+               tpnt->buffer->orig_frp_segs = 0;
+               normalize_buffer(tpnt->buffer);
+               kfree(tpnt->buffer);
+       }
+
+       disk->private_data = NULL;
+       put_disk(disk);
+       kfree(tpnt);
+       return;
+}
+
 static void st_intr(struct scsi_cmnd *SCpnt)
 {
        scsi_io_completion(SCpnt, (SCpnt->result ? 0: SCpnt->bufflen), 1);

diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
index 061da11..790acac 100644 (file)
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -3,7 +3,7 @@
 #define _ST_H
 
 #include <linux/completion.h>
-
+#include <linux/kref.h>
 
 /* Descriptor for analyzed sense data */
 struct st_cmdstatus {
@@ -156,6 +156,7 @@ struct scsi_tape {
        unsigned char last_sense[16];
 #endif
        struct gendisk *disk;
+       struct kref     kref;
 };
 
 /* Bit masks for use_pf */