[PATCH] abstract out more scsi_device acess out of the low-level drivers

Two new helpers: scsi_device_get and scsi_device_put that get/release a
reference to the underlying HBA driver and increment/decrement
->access_count.  Cleanup ->attach/->detach routines in the upper layer
drivers a bit to consolidate the error pathes once we're cleaning them
up.

->attach and ->detach in upper layer drivers are mandatory now (not
having them would be rather pointless).

(note that the sd.c changes are not in this patch, it'll be part of
my next, bigger patch)

drivers/scsi/osst.c

@@ -4234,14 +4234,14 @@ static int os_scsi_tape_open(struct inode * inode, struct file * filp)
 #endif
 		return (-EBUSY);
 	}
-	if (!try_module_get(STp->device->host->hostt->module)) {
+
+	if (!scsi_device_get(STp->device)) {
 		write_unlock(&os_scsi_tapes_lock);
 #if DEBUG
-                printk(OSST_DEB_MSG "%s:D: Failed try_module_get.\n", name);
+                printk(OSST_DEB_MSG "%s:D: Failed scsi_device_get.\n", name);
 #endif
 		return (-ENXIO);
 	}
-	STp->device->access_count++;
 	filp->private_data = STp;
 	STp->in_use = 1;
 	write_unlock(&os_scsi_tapes_lock);
@@ -4589,9 +4589,7 @@ static int os_scsi_tape_open(struct inode * inode, struct file * filp)
 	normalize_buffer(STp->buffer);
 	STp->header_ok = 0;
 	STp->in_use = 0;
-	STp->device->access_count--;
-
-	module_put(STp->device->host->hostt->module);
+	scsi_device_put(STp->device);
 
 	return retval;
 }
@@ -4719,9 +4717,8 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)
 	write_lock(&os_scsi_tapes_lock);
 	STp->in_use = 0;
 	write_unlock(&os_scsi_tapes_lock);
-	STp->device->access_count--;
 
-	module_put(STp->device->host->hostt->module);
+	scsi_device_put(STp->device);
 
 	return result;
 }

drivers/scsi/scsi.c

@@ -1877,20 +1877,12 @@ int scsi_attach_device(struct scsi_device *sdev)
 	struct Scsi_Device_Template *sdt;
 
 	down_read(&scsi_devicelist_mutex);
-	list_for_each_entry(sdt, &scsi_devicelist, list)
-		if (sdt->attach) {
-			/*
-			 * XXX check result when the upper level attach
-			 * return values are fixed, and on failure goto
-			 * fail.
-			 */
-			if(try_module_get(sdt->module)) {
-				(*sdt->attach)(sdev);
-				module_put(sdt->module);
-			} else {
-				printk(KERN_WARNING "SCSI module %s not ready, skipping attach.\n", sdt->name);
-			}
-		}
+	list_for_each_entry(sdt, &scsi_devicelist, list) {
+		if (!try_module_get(sdt->module))
+			continue;
+		(*sdt->attach)(sdev);
+		module_put(sdt->module);
+	}
 	up_read(&scsi_devicelist_mutex);
 	return 0;
 }
@@ -1900,18 +1892,30 @@ void scsi_detach_device(struct scsi_device *sdev)
 	struct Scsi_Device_Template *sdt;
 
 	down_read(&scsi_devicelist_mutex);
-	list_for_each_entry(sdt, &scsi_devicelist, list)
-		if (sdt->detach) {
-			if(try_module_get(sdt->module)) {
-				(*sdt->detach)(sdev);
-				module_put(sdt->module);
-			} else {
-				printk(KERN_WARNING "SCSI module %s not ready, skipping detach.\n", sdt->name);
-			}
-		}
+	list_for_each_entry(sdt, &scsi_devicelist, list) {
+		if (!try_module_get(sdt->module))
+			continue;
+		(*sdt->detach)(sdev);
+		module_put(sdt->module);
+	}
 	up_read(&scsi_devicelist_mutex);
 }
 
+int scsi_device_get(struct scsi_device *sdev)
+{
+	if (!try_module_get(sdev->host->hostt->module))
+		return -ENXIO;
+
+	sdev->access_count++;
+	return 0;
+}
+
+void scsi_device_put(struct scsi_device *sdev)
+{
+	sdev->access_count--;
+	module_put(sdev->host->hostt->module);
+}
+
 /*
  * Function:	scsi_slave_attach()
  *
@@ -1935,7 +1939,7 @@ int scsi_slave_attach(struct scsi_device *sdev)
 			printk(KERN_ERR "scsi: Allocation failure during"
 			       " attach, some SCSI devices might not be"
 			       " configured\n");
-			return 1;
+			return -ENOMEM;
 		}
 		if (sdev->host->hostt->slave_configure != NULL) {
 			if (sdev->host->hostt->slave_configure(sdev) != 0) {
@@ -1943,7 +1947,7 @@ int scsi_slave_attach(struct scsi_device *sdev)
 				       " attach, some SCSI device might not be"
 				       " configured\n");
 				scsi_release_commandblocks(sdev);
-				return 1;
+				return -ENOMEM;
 			}
 		} else if (sdev->host->cmd_per_lun != 0)
 			scsi_adjust_queue_depth(sdev, 0,

drivers/scsi/scsi.h

@@ -448,8 +448,10 @@ 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_track_queue_full(Scsi_Device *, int);
-extern int scsi_slave_attach(struct scsi_device *sdev);
-extern void scsi_slave_detach(struct scsi_device *sdev);
+extern int scsi_slave_attach(struct scsi_device *);
+extern void scsi_slave_detach(struct scsi_device *);
+extern int scsi_device_get(struct scsi_device *);
+extern void scsi_device_put(struct scsi_device *);
 extern void scsi_done(Scsi_Cmnd * SCpnt);
 extern void scsi_finish_command(Scsi_Cmnd *);
 extern int scsi_retry_command(Scsi_Cmnd *);

drivers/scsi/scsi_scan.c

@@ -1340,7 +1340,7 @@ static int scsi_add_lun(Scsi_Device *sdevscan, Scsi_Device **sdevnew,
 	return SCSI_SCAN_LUN_PRESENT;
 }
 
-static int scsi_remove_lun(struct scsi_device *sdev)
+static void scsi_remove_lun(struct scsi_device *sdev)
 {
 	devfs_unregister(sdev->de);
 	scsi_device_unregister(sdev);

drivers/scsi/scsi_syms.c

@@ -81,6 +81,8 @@ EXPORT_SYMBOL(scsi_register_blocked_host);
 EXPORT_SYMBOL(scsi_deregister_blocked_host);
 EXPORT_SYMBOL(scsi_slave_attach);
 EXPORT_SYMBOL(scsi_slave_detach);
+EXPORT_SYMBOL(scsi_device_get);
+EXPORT_SYMBOL(scsi_device_put);
 
 /*
  * This symbol is for the highlevel drivers (e.g. sg) only.

drivers/scsi/sg.c

@@ -261,9 +261,9 @@ sg_open(struct inode *inode, struct file *filp)
 
 	/* This driver's module count bumped by fops_get in <linux/fs.h> */
 	/* Prevent the device driver from vanishing while we sleep */
-	if (!try_module_get(sdp->device->host->hostt->module))
-		return -ENXIO;
-	sdp->device->access_count++;
+	retval = scsi_device_get(sdp->device);
+	if (retval)
+		return retval;
 
 	if (!((flags & O_NONBLOCK) ||
 	      scsi_block_when_processing_errors(sdp->device))) {
@@ -316,8 +316,7 @@ sg_open(struct inode *inode, struct file *filp)
 	return 0;
 
       error_out:
-	sdp->device->access_count--;
-	module_put(sdp->device->host->hostt->module);
+	scsi_device_put(sdp->device);
 	return retval;
 }
 
@@ -334,8 +333,7 @@ sg_release(struct inode *inode, struct file *filp)
 	sg_fasync(-1, filp, 0);	/* remove filp from async notification list */
 	if (0 == sg_remove_sfp(sdp, sfp)) {	/* Returns 1 when sdp gone */
 		if (!sdp->detached) {
-			sdp->device->access_count--;
-			module_put(sdp->device->host->hostt->module);
+			scsi_device_put(sdp->device);
 		}
 		sdp->exclude = 0;
 		wake_up_interruptible(&sdp->o_excl_wait);
@@ -1298,8 +1296,7 @@ sg_cmd_done(Scsi_Cmnd * SCpnt)
 		if (NULL == sfp->headrp) {
 			SCSI_LOG_TIMEOUT(1, printk("sg...bh: already closed, final cleanup\n"));
 			if (0 == sg_remove_sfp(sdp, sfp)) {	/* device still present */
-				sdp->device->access_count--;
-				module_put(sdp->device->host->hostt->module);
+				scsi_device_put(sdp->device);
 			}
 			sfp = NULL;
 		}
@@ -1374,15 +1371,19 @@ static DEVICE_ATTR(type,S_IRUGO,sg_device_type_read,NULL);
 static int
 sg_attach(Scsi_Device * scsidp)
 {
-	struct gendisk *disk = alloc_disk(1);
+	struct gendisk *disk;
 	Sg_device *sdp = NULL;
 	unsigned long iflags;
-	int k;
+	int k, error;
 
+	disk = alloc_disk(1);
 	if (!disk)
-		return 1;
-	if (scsi_slave_attach(scsidp))
-		return 1;
+		return -ENOMEM;
+
+	error = scsi_slave_attach(scsidp);
+	if (error)
+		goto out_put;
+		
 	write_lock_irqsave(&sg_dev_arr_lock, iflags);
 	if (sg_nr_dev >= sg_dev_max) {	/* try to resize */
 		Sg_device **tmp_da;
@@ -1394,9 +1395,8 @@ sg_attach(Scsi_Device * scsidp)
 		if (NULL == tmp_da) {
 			printk(KERN_ERR
 			       "sg_attach: device array cannot be resized\n");
-			put_disk(disk);
-			scsi_slave_detach(scsidp);
-			return 1;
+			error = -ENOMEM;
+			goto out_detach;
 		}
 		write_lock_irqsave(&sg_dev_arr_lock, iflags);
 		memset(tmp_da, 0, tmp_dev_max * sizeof (Sg_device *));
@@ -1420,9 +1420,8 @@ sg_attach(Scsi_Device * scsidp)
 		       scsidp->lun, scsidp->type, SG_MAX_DEVS_MASK);
 		if (NULL != sdp)
 			vfree((char *) sdp);
-		put_disk(disk);
-		scsi_slave_detach(scsidp);
-		return 1;
+		error = -ENODEV;
+		goto out_detach;
 	}
 	if (k < sg_dev_max) {
 		if (NULL == sdp) {
@@ -1437,9 +1436,8 @@ sg_attach(Scsi_Device * scsidp)
 	if (NULL == sdp) {
 		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;
+		error = -ENOMEM;
+		goto out_detach;
 	}
 
 	SCSI_LOG_TIMEOUT(3, printk("sg_attach: dev=%d \n", k));
@@ -1492,6 +1490,12 @@ sg_attach(Scsi_Device * scsidp)
 		       scsidp->lun, scsidp->type);
 	}
 	return 0;
+
+out_detach:
+	scsi_slave_detach(scsidp);
+out_put:
+	put_disk(disk);
+	return error;
 }
 
 static void
@@ -1523,8 +1527,7 @@ sg_detach(Scsi_Device * scsidp)
 						sg_finish_rem_req(srp);
 				}
 				if (sfp->closed) {
-					sdp->device->access_count--;
-					module_put(sdp->device->host->hostt->module);
+					scsi_device_put(sdp->device);
 					__sg_remove_sfp(sdp, sfp);
 				} else {
 					delay = 1;
@@ -2512,8 +2515,7 @@ sg_remove_sfp(Sg_device * sdp, Sg_fd * sfp)
 		/* MOD_INC's to inhibit unloading sg and associated adapter driver */
 		/* only bump the access_count if we actually succeeded in
 		 * throwing another counter on the host module */
-		if(try_module_get(sdp->device->host->hostt->module))
-			sdp->device->access_count++;
+		scsi_device_get(sdp->device);	/* XXX: retval ignored? */	
 		sfp->closed = 1;	/* flag dirty state on this fd */
 		SCSI_LOG_TIMEOUT(1, printk("sg_remove_sfp: worrisome, %d writes pending\n",
 				  dirty));

drivers/scsi/sr.c

@@ -87,6 +87,8 @@ static LIST_HEAD(sr_devlist);
 static spinlock_t sr_devlist_lock = SPIN_LOCK_UNLOCKED;
 
 static int sr_open(struct cdrom_device_info *, int);
+static void sr_release(struct cdrom_device_info *);
+
 static void get_sectorsize(struct scsi_cd *);
 static void get_capabilities(struct scsi_cd *);
 
@@ -124,16 +126,6 @@ static inline void sr_devlist_remove(Scsi_CD *cd)
 	spin_unlock(&sr_devlist_lock);
 }
 
-static void sr_release(struct cdrom_device_info *cdi)
-{
-	struct scsi_cd *cd = cdi->handle;
-
-	if (cd->device->sector_size > 2048)
-		sr_set_blocklength(cd, 2048);
-	cd->device->access_count--;
-	module_put(cd->device->host->hostt->module);
-}
-
 static struct cdrom_device_ops sr_dops = {
 	.open			= sr_open,
 	.release	 	= sr_release,
@@ -460,43 +452,59 @@ struct block_device_operations sr_bdops =
 static int sr_open(struct cdrom_device_info *cdi, int purpose)
 {
 	struct scsi_cd *cd = cdi->handle;
+	struct scsi_device *sdev = cd->device;
+	int retval;
 
-	if (!cd->device)
-		return -ENXIO;	/* No such device */
+	retval = scsi_device_get(sdev);
+	if (retval)
+		return retval;
+	
 	/*
 	 * If the device is in error recovery, wait until it is done.
 	 * If the device is offline, then disallow any access to it.
 	 */
-	if (!scsi_block_when_processing_errors(cd->device)) {
-		return -ENXIO;
-	}
-	if(!try_module_get(cd->device->host->hostt->module))
-		return -ENXIO;
-	cd->device->access_count++;
+	retval = -ENXIO;
+	if (!scsi_block_when_processing_errors(sdev))
+		goto error_out;
 
-	/* If this device did not have media in the drive at boot time, then
+	/*
+	 * If this device did not have media in the drive at boot time, then
 	 * we would have been unable to get the sector size.  Check to see if
 	 * this is the case, and try again.
 	 */
-
 	if (cd->needs_sector_size)
 		get_sectorsize(cd);
-
 	return 0;
+
+error_out:
+	scsi_device_put(sdev);
+	return retval;	
+}
+
+static void sr_release(struct cdrom_device_info *cdi)
+{
+	struct scsi_cd *cd = cdi->handle;
+
+	if (cd->device->sector_size > 2048)
+		sr_set_blocklength(cd, 2048);
+
+	scsi_device_put(cd->device);
 }
 
 static int sr_attach(struct scsi_device *sdev)
 {
 	struct gendisk *disk;
 	struct scsi_cd *cd;
-	int minor;
+	int minor, error;
 
 	if (sdev->type != TYPE_ROM && sdev->type != TYPE_WORM)
 		return 1;
 
-	if (scsi_slave_attach(sdev))
-		return 1;
+	error = scsi_slave_attach(sdev);
+	if (error)
+		return error;
 
+	error = -ENOMEM;
 	cd = kmalloc(sizeof(*cd), GFP_KERNEL);
 	if (!cd)
 		goto fail;
@@ -566,7 +574,7 @@ static int sr_attach(struct scsi_device *sdev)
 	kfree(cd);
 fail:
 	scsi_slave_detach(sdev);
-	return 1;
+	return error;
 }
 
 

drivers/scsi/st.c

@@ -993,9 +993,9 @@ static int st_open(struct inode *inode, struct file *filp)
 		DEB( printk(ST_DEB_MSG "%s: Device already in use.\n", name); )
 		return (-EBUSY);
 	}
-	if(!try_module_get(STp->device->host->hostt->module))
+
+	if(!scsi_device_get(STp->device))
 		return (-ENXIO);
-	STp->device->access_count++;
 	STp->in_use = 1;
 	write_unlock(&st_dev_arr_lock);
 	STp->rew_at_close = STp->autorew_dev = (minor(inode->i_rdev) & 0x80) == 0;
@@ -1040,8 +1040,7 @@ static int st_open(struct inode *inode, struct file *filp)
  err_out:
 	normalize_buffer(STp->buffer);
 	STp->in_use = 0;
-	STp->device->access_count--;
-	module_put(STp->device->host->hostt->module);
+	scsi_device_put(STp->device);
 	return retval;
 
 }
@@ -1174,8 +1173,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);
-	STp->device->access_count--;
-	module_put(STp->device->host->hostt->module);
+	scsi_device_put(STp->device);
 
 	return result;
 }