[PATCH] fix 2.5 scsi queue depth setting

This patch fixes queue depth setting of scsi devices.

This is done by pairing shost->slave_attach() calls with
a scsi_build_commandblocks in the new scsi_slave_attach.

This is a patch aginst linux-scsi.bkbits.net/scsi-for-linus-2.5 after
applying the last posted hch version of the "Eliminate scsi_host_tmpl_list"
patch, it still applies with offset to the current scsi-for-linus-2.5.

It also:

Will properly call shost->slave_attach after a scsi_unregister_device()
followed by a scsi_register_device() - as could happen if you were able to
rmmod all upper level drivers and then insmod any of them back (only
possible when not booted on scsi).

Checks for scsi_build_commandblocks() allocation failures.

Sets queue depth even if shost->slave_attach() does not call
scsi_adjust_queue_depth.

Removes the use of revoke (no drivers are setting it, it was only
call via the proc scsi remove-single-device interface).

There are at least two problems with sysfs and scsi (one in sysfs, one in
scsi, I'll try and post more soon ...) so I could not completey test rmmod
of an adapter or upper level driver without leading to an oops or shutdown
hang.

 hosts.c              |    5 --
 hosts.h              |    6 --
 osst.c               |    9 ++-
 scsi.c               |  118 +++++++++++++++++++++++++++++++--------------------
 scsi.h               |    2
 scsi_mid_low_api.txt |   24 ----------
 scsi_scan.c          |    9 ---
 sd.c                 |   10 +++-
 sg.c                 |   10 ++--
 sr.c                 |    7 ++-
 st.c                 |   11 +++-
 11 files changed, 106 insertions(+), 105 deletions(-)

===== drivers/scsi/hosts.c 1.23 vs edited =====

drivers/scsi/hosts.c

@@ -195,10 +195,6 @@ int scsi_remove_host(struct Scsi_Host *shost)
 			       sdev->attached);
 			return 1;
 		}
-
-		if (shost->hostt->slave_detach)
-			(*shost->hostt->slave_detach) (sdev);
-
 		devfs_unregister(sdev->de);
 		device_unregister(&sdev->sdev_driverfs_dev);
 	}
@@ -207,7 +203,6 @@ int scsi_remove_host(struct Scsi_Host *shost)
 
 	for (sdev = shost->host_queue; sdev;
 	     sdev = shost->host_queue) {
-		scsi_release_commandblocks(sdev);
 		blk_cleanup_queue(&sdev->request_queue);
 		/* Next free up the Scsi_Device structures for this host */
 		shost->host_queue = sdev->next;

drivers/scsi/hosts.h

@@ -93,12 +93,6 @@ typedef struct	SHT
      */
     int (* detect)(struct SHT *);
 
-    /*
-     * This function is only used by one driver and will be going away
-     * once it switches over to using the slave_detach() function instead.
-     */
-    int (*revoke)(Scsi_Device *);
-
     /* Used with loadable modules to unload the host structures.  Note:
      * there is a default action built into the modules code which may
      * be sufficient for most host adapters.  Thus you may not have to supply

drivers/scsi/osst.c

@@ -5421,10 +5421,12 @@ static int osst_attach(Scsi_Device * SDp)
 		return 1;
 
 	if (osst_nr_dev >= osst_dev_max) {
-		 SDp->attached--;
 		 put_disk(disk);
 		 return 1;
 	}
+
+	if (scsi_slave_attach(SDp))
+		return 1;
 	
 	/* find a free minor number */
 	for (i=0; os_scsi_tapes[i] && i<osst_dev_max; i++);
@@ -5433,9 +5435,10 @@ static int osst_attach(Scsi_Device * SDp)
 	/* allocate a OS_Scsi_Tape for this device */
 	tpnt = (OS_Scsi_Tape *)kmalloc(sizeof(OS_Scsi_Tape), GFP_ATOMIC);
 	if (tpnt == NULL) {
-		 SDp->attached--;
 		 printk(KERN_WARNING "osst :W: Can't allocate device descriptor.\n");
 		 put_disk(disk);
+
+		 scsi_slave_detach(SDp);
 		 return 1;
 	}
 	memset(tpnt, 0, sizeof(OS_Scsi_Tape));
@@ -5648,7 +5651,7 @@ static void osst_detach(Scsi_Device * SDp)
 		put_disk(tpnt->disk);
 		kfree(tpnt);
 		os_scsi_tapes[i] = NULL;
-		SDp->attached--;
+		scsi_slave_detach(SDp);
 		osst_nr_dev--;
 		osst_dev_noticed--;
 		return;

drivers/scsi/scsi.c

@@ -1664,10 +1664,6 @@ void scsi_adjust_queue_depth(Scsi_Device *SDpnt, int tagged, int tags)
 			break;
 	}
 	spin_unlock_irqrestore(&device_request_lock, flags);
-	if(SDpnt->current_queue_depth == 0)
-	{
-		scsi_build_commandblocks(SDpnt);
-	}
 }
 
 #ifdef CONFIG_PROC_FS
@@ -1932,16 +1928,7 @@ static int proc_scsi_gen_write(struct file * file, const char * buf,
 		scsi_detach_device(scd);
 
 		if (scd->attached == 0) {
-			/*
-			 * Nobody is using this device any more.
-			 * Free all of the command structures.
-			 */
-                        if (HBA_ptr->hostt->revoke)
-                                HBA_ptr->hostt->revoke(scd);
-			if (HBA_ptr->hostt->slave_detach)
-				(*HBA_ptr->hostt->slave_detach) (scd);
 			devfs_unregister (scd->de);
-			scsi_release_commandblocks(scd);
 
 			/* Now we can remove the device structure */
 			if (scd->next != NULL)
@@ -1976,7 +1963,7 @@ void scsi_detect_device(struct scsi_device *sdev)
 	down_read(&scsi_devicelist_mutex);
 	for (sdt = scsi_devicelist; sdt; sdt = sdt->next)
 		if (sdt->detect)
-			sdev->attached += (*sdt->detect)(sdev);
+			(*sdt->detect)(sdev);
 	up_read(&scsi_devicelist_mutex);
 }
 
@@ -1984,18 +1971,16 @@ int scsi_attach_device(struct scsi_device *sdev)
 {
 	struct Scsi_Device_Template *sdt;
 
-	scsi_build_commandblocks(sdev);
-	if (sdev->current_queue_depth == 0)
-		goto fail;
-
 	down_read(&scsi_devicelist_mutex);
 	for (sdt = scsi_devicelist; sdt; sdt = sdt->next)
 		if (sdt->attach)
+			/*
+			 * XXX check result when the upper level attach
+			 * return values are fixed, and on failure goto
+			 * fail.
+			 */
 			(*sdt->attach) (sdev);
 	up_read(&scsi_devicelist_mutex);
-
-	if (!sdev->attached)
-		scsi_release_commandblocks(sdev);
 	return 0;
 
 fail:
@@ -2016,6 +2001,66 @@ void scsi_detach_device(struct scsi_device *sdev)
 	up_read(&scsi_devicelist_mutex);
 }
 
+/*
+ * Function:	scsi_slave_attach()
+ *
+ * Purpose:	Called from the upper level driver attach to handle common
+ * 		attach code.
+ *
+ * Arguments:	sdev - scsi_device to attach
+ *
+ * Returns:	1 on error, 0 on succes
+ *
+ * Lock Status:	Protected via scsi_devicelist_mutex.
+ */
+int scsi_slave_attach(struct scsi_device *sdev)
+{
+	if (sdev->attached++ == 0) {
+		/*
+		 * No one was attached.
+		 */
+		if ((sdev->host->hostt->slave_attach != NULL) &&
+		    (sdev->host->hostt->slave_attach(sdev) != 0)) {
+			printk(KERN_INFO "scsi: failed low level driver"
+			       " attach, some SCSI device might not be"
+			       " configured\n");
+			return 1;
+		}
+		if ((sdev->new_queue_depth == 0) &&
+		    (sdev->host->cmd_per_lun != 0))
+			scsi_adjust_queue_depth(sdev, 0,
+						sdev->host->cmd_per_lun);
+		scsi_build_commandblocks(sdev);
+		if (sdev->current_queue_depth == 0) {
+			printk(KERN_ERR "scsi: Allocation failure during"
+			       " attach, some SCSI devices might not be"
+			       " configured\n");
+			if (sdev->host->hostt->slave_detach != NULL)
+				sdev->host->hostt->slave_detach(sdev);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+/*
+ * Function:	scsi_slave_detach()
+ *
+ * Purpose:	Called from the upper level driver attach to handle common
+ * 		detach code.
+ *
+ * Arguments:	sdev - struct scsi_device to detach
+ *
+ * Lock Status:	Protected via scsi_devicelist_mutex.
+ */
+void scsi_slave_detach(struct scsi_device *sdev)
+{
+	if (--sdev->attached == 0) {
+		if (sdev->host->hostt->slave_detach != NULL)
+			sdev->host->hostt->slave_detach(sdev);
+		scsi_release_commandblocks(sdev);
+	}
+}
 /*
  * This entry point should be called by a loadable module if it is trying
  * add a high level scsi driver to the system.
@@ -2053,7 +2098,7 @@ int scsi_register_device(struct Scsi_Device_Template *tpnt)
 		for (SDpnt = shpnt->host_queue; SDpnt;
 		     SDpnt = SDpnt->next) {
 			if (tpnt->detect)
-				SDpnt->attached += (*tpnt->detect) (SDpnt);
+				(*tpnt->detect) (SDpnt);
 		}
 	}
 
@@ -2064,22 +2109,14 @@ int scsi_register_device(struct Scsi_Device_Template *tpnt)
 	     shpnt = scsi_host_get_next(shpnt)) {
 		for (SDpnt = shpnt->host_queue; SDpnt;
 		     SDpnt = SDpnt->next) {
-			scsi_build_commandblocks(SDpnt);
-			if (SDpnt->current_queue_depth == 0) {
-				out_of_space = 1;
-				continue;
-			}
 			if (tpnt->attach)
+				/*
+				 * XXX check result when the upper level
+				 * attach return values are fixed, and
+				 * stop attaching on failure.
+				 */
 				(*tpnt->attach) (SDpnt);
 
-			/*
-			 * If this driver attached to the device, and don't have any
-			 * command blocks for this device, allocate some.
-			 */
-			if (SDpnt->attached)
-				SDpnt->online = TRUE;
-			else
-				scsi_release_commandblocks(SDpnt);
 		}
 	}
 
@@ -2119,17 +2156,6 @@ int scsi_unregister_device(struct Scsi_Device_Template *tpnt)
 		     SDpnt = SDpnt->next) {
 			if (tpnt->detach)
 				(*tpnt->detach) (SDpnt);
-			if (SDpnt->attached == 0) {
-				SDpnt->online = FALSE;
-
-				/*
-				 * Nobody is using this device any more.  Free all of the
-				 * command structures.
-				 */
-				if (shpnt->hostt->slave_detach)
-					(*shpnt->hostt->slave_detach) (SDpnt);
-				scsi_release_commandblocks(SDpnt);
-			}
 		}
 	}
 	/*

drivers/scsi/scsi.h

@@ -466,6 +466,8 @@ extern void scsi_bottom_half_handler(void);
 extern void scsi_release_commandblocks(Scsi_Device * SDpnt);
 extern void scsi_build_commandblocks(Scsi_Device * SDpnt);
 extern void scsi_adjust_queue_depth(Scsi_Device *, int, int);
+extern int scsi_slave_attach(struct scsi_device *sdev);
+extern void scsi_slave_detach(struct scsi_device *sdev);
 extern void scsi_done(Scsi_Cmnd * SCpnt);
 extern void scsi_finish_command(Scsi_Cmnd *);
 extern int scsi_retry_command(Scsi_Cmnd *);

drivers/scsi/scsi_mid_low_api.txt

@@ -352,34 +352,12 @@ int proc_info(char * buffer, char ** start, off_t offset,
  *      Locks: lock_kernel() active on entry and expected to be active
  *      on return.
  *
- *      Notes: Invoked from mid level's scsi_unregister_host(). When a
- *      host is being unregistered the mid level does not bother to
- *      call revoke() on the devices it controls.
+ *      Notes: Invoked from mid level's scsi_unregister_host().
  *      This function should call scsi_unregister(shp) [found in hosts.c]
  *      prior to returning.
  **/
     int release(struct Scsi_Host * shp);
 
-
-/**
- *      revoke - indicate disinterest in a scsi device
- *      @sdp: host template for this driver.
- *
- *      Return value ignored.
- *
- *      Required: no
- *
- *      Locks: none held
- *
- *      Notes: Called when "scsi remove-single-device <h> <b> <t> <l>"
- *      is written to /proc/scsi/scsi to indicate the device is no longer 
- *      required. It is called after the upper level drivers have detached 
- *      this device and before the device name  (e.g. /dev/sdc) is 
- *      unregistered and the resources associated with it are freed.
- **/
-    int revoke(Scsi_device * sdp);
-
-
 /**
  *      select_queue_depths - calculate allowable number of scsi commands
  *                            that can be queued on each device (disk)

drivers/scsi/scsi_scan.c

@@ -1480,15 +1480,6 @@ static int scsi_add_lun(Scsi_Device *sdevscan, Scsi_Device **sdevnew,
 
 	scsi_detect_device(sdev);
 
-	if (sdev->host->hostt->slave_attach != NULL) {
-		if (sdev->host->hostt->slave_attach(sdev) != 0) {
-			printk(KERN_INFO "%s: scsi_add_lun: failed low level driver attach, setting device offline", devname);
-			sdev->online = FALSE;
-		}
-	} else if(sdev->host->cmd_per_lun) {
-		scsi_adjust_queue_depth(sdev, 0, sdev->host->cmd_per_lun);
-	}
-
 	if (sdevnew != NULL)
 		*sdevnew = sdev;
 

drivers/scsi/sd.c

@@ -1212,9 +1212,12 @@ static int sd_attach(struct scsi_device * sdp)
 	SCSI_LOG_HLQUEUE(3, printk("sd_attach: scsi device: <%d,%d,%d,%d>\n", 
 			 sdp->host->host_no, sdp->channel, sdp->id, sdp->lun));
 
+	if (scsi_slave_attach(sdp))
+		goto out;
+
 	sdkp = kmalloc(sizeof(*sdkp), GFP_KERNEL);
 	if (!sdkp)
-		goto out;
+		goto out_detach;
 
 	gd = alloc_disk(16);
 	if (!gd)
@@ -1263,8 +1266,9 @@ static int sd_attach(struct scsi_device * sdp)
 
 out_free:
 	kfree(sdkp);
+out_detach:
+	scsi_slave_detach(sdp);
 out:
-	sdp->attached--;
 	return 1;
 }
 
@@ -1312,7 +1316,7 @@ static void sd_detach(struct scsi_device * sdp)
 
 	sd_devlist_remove(sdkp);
 	del_gendisk(sdkp->disk);
-	sdp->attached--;
+	scsi_slave_detach(sdp);
 	sd_nr_dev--;
 	put_disk(sdkp->disk);
 	kfree(sdkp);

drivers/scsi/sg.c

@@ -1396,6 +1396,8 @@ sg_attach(Scsi_Device * scsidp)
 
 	if (!disk)
 		return 1;
+	if (scsi_slave_attach(scsidp))
+		return 1;
 	write_lock_irqsave(&sg_dev_arr_lock, iflags);
 	if (sg_nr_dev >= sg_dev_max) {	/* try to resize */
 		Sg_device **tmp_da;
@@ -1405,10 +1407,10 @@ sg_attach(Scsi_Device * scsidp)
 		tmp_da = (Sg_device **)vmalloc(
 				tmp_dev_max * sizeof(Sg_device *));
 		if (NULL == tmp_da) {
-			scsidp->attached--;
 			printk(KERN_ERR
 			       "sg_attach: device array cannot be resized\n");
 			put_disk(disk);
+			scsi_slave_detach(scsidp);
 			return 1;
 		}
 		write_lock_irqsave(&sg_dev_arr_lock, iflags);
@@ -1425,7 +1427,6 @@ sg_attach(Scsi_Device * scsidp)
 		if (!sg_dev_arr[k])
 			break;
 	if (k > SG_MAX_DEVS_MASK) {
-		scsidp->attached--;
 		write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
 		printk(KERN_WARNING
 		       "Unable to attach sg device <%d, %d, %d, %d>"
@@ -1435,6 +1436,7 @@ sg_attach(Scsi_Device * scsidp)
 		if (NULL != sdp)
 			vfree((char *) sdp);
 		put_disk(disk);
+		scsi_slave_detach(scsidp);
 		return 1;
 	}
 	if (k < sg_dev_max) {
@@ -1448,10 +1450,10 @@ sg_attach(Scsi_Device * scsidp)
 	} else
 		sdp = NULL;
 	if (NULL == sdp) {
-		scsidp->attached--;
 		write_unlock_irqrestore(&sg_dev_arr_lock, iflags);
 		printk(KERN_ERR "sg_attach: Sg_device cannot be allocated\n");
 		put_disk(disk);
+		scsi_slave_detach(scsidp);
 		return 1;
 	}
 
@@ -1559,7 +1561,7 @@ sg_detach(Scsi_Device * scsidp)
 			SCSI_LOG_TIMEOUT(3, printk("sg_detach: dev=%d\n", k));
 			sg_dev_arr[k] = NULL;
 		}
-		scsidp->attached--;
+		scsi_slave_detach(scsidp);
 		sg_nr_dev--;
 		sg_dev_noticed--;	/* from <dan@lectra.fr> */
 		break;

drivers/scsi/sr.c

@@ -506,6 +506,9 @@ static int sr_attach(struct scsi_device *sdev)
 	if (sdev->type != TYPE_ROM && sdev->type != TYPE_WORM)
 		return 1;
 
+	if (scsi_slave_attach(sdev))
+		return 1;
+
 	cd = kmalloc(sizeof(*cd), GFP_KERNEL);
 	if (!cd)
 		goto fail;
@@ -574,7 +577,7 @@ static int sr_attach(struct scsi_device *sdev)
 fail_free:
 	kfree(cd);
 fail:
-	sdev->attached--;
+	scsi_slave_detach(sdev);
 	return 1;
 }
 
@@ -807,7 +810,7 @@ static void sr_detach(struct scsi_device * SDp)
 	put_disk(cd->disk);
 	unregister_cdrom(&cd->cdi);
 
-	SDp->attached--;
+	scsi_slave_detach(SDp);
 	sr_nr_dev--;
 
 	kfree(cd);

drivers/scsi/st.c

@@ -3667,6 +3667,9 @@ static int st_attach(Scsi_Device * SDp)
 		return 1;
 	}
 
+	if (scsi_slave_attach(SDp))
+		return 1;
+
 	i = SDp->host->sg_tablesize;
 	if (st_max_sg_segs < i)
 		i = st_max_sg_segs;
@@ -3692,11 +3695,11 @@ static int st_attach(Scsi_Device * SDp)
 		if (tmp_dev_max > ST_MAX_TAPES)
 			tmp_dev_max = ST_MAX_TAPES;
 		if (tmp_dev_max <= st_nr_dev) {
-			SDp->attached--;
 			write_unlock(&st_dev_arr_lock);
 			printk(KERN_ERR "st: Too many tape devices (max. %d).\n",
 			       ST_MAX_TAPES);
 			put_disk(disk);
+			scsi_slave_detach(SDp);
 			return 1;
 		}
 
@@ -3707,10 +3710,10 @@ static int st_attach(Scsi_Device * SDp)
 				kfree(tmp_da);
 			if (tmp_ba != NULL)
 				kfree(tmp_ba);
-			SDp->attached--;
 			write_unlock(&st_dev_arr_lock);
 			printk(KERN_ERR "st: Can't extend device array.\n");
 			put_disk(disk);
+			scsi_slave_detach(SDp);
 			return 1;
 		}
 
@@ -3733,10 +3736,10 @@ static int st_attach(Scsi_Device * SDp)
 
 	tpnt = kmalloc(sizeof(Scsi_Tape), GFP_ATOMIC);
 	if (tpnt == NULL) {
-		SDp->attached--;
 		write_unlock(&st_dev_arr_lock);
 		printk(KERN_ERR "st: Can't allocate device descriptor.\n");
 		put_disk(disk);
+		scsi_slave_detach(SDp);
 		return 1;
 	}
 	memset(tpnt, 0, sizeof(Scsi_Tape));
@@ -3906,7 +3909,7 @@ static void st_detach(Scsi_Device * SDp)
 				tpnt->de_n[mode] = NULL;
 			}
 			scsi_tapes[i] = 0;
-			SDp->attached--;
+			scsi_slave_detach(SDp);
 			st_nr_dev--;
 			write_unlock(&st_dev_arr_lock);