[patch 0/5] st: Clean up and raise max device limit (v4)

[patch 0/5] st: Clean up and raise max device limit (v4)

[Jeff Mahoney]

This patchset cleans up the SCSI tape device handling code and leverages it
to lift the limitation of the number of tape drives from the previous
arbitrary limit of 128 to the maximum supported by a device node that
creates 8 character devices per physical device. Since minors are 20 bits,
that means 2^17 tape drives can be supported.

Changed in this version: A previous revision introduced a regression where
tape drives would not be shown as tape devices in lsscsi. This was due to
the missing "tape" symlink in sysfs. That issue has been addressed and
lsscsi works properly again. Also, it passes checkpatch with no errors.

Please apply.

[patch 1/5] [PATCH 1/5] st: Use static class attributes

[Jeff Mahoney]

[-- Attachment #1: st-use-static-class-attributes --]
[-- Type: text/plain, Size: 6370 bytes --]

st currently sets up and tears down class attributes manually for
every tape drive in the system. This patch uses a statically defined
class with class attributes to let the device core do it for us.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 drivers/scsi/st.c |   74 ++++++++++++++++++++++++------------------------------
 1 file changed, 33 insertions(+), 41 deletions(-)

--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -84,7 +84,8 @@ static int try_wdio = 1;
 static int st_dev_max;
 static int st_nr_dev;
 
-static struct class *st_sysfs_class;
+static struct class st_sysfs_class;
+static struct device_attribute st_dev_attrs[];
 
 MODULE_AUTHOR("Kai Makisara");
 MODULE_DESCRIPTION("SCSI tape (st) driver");
@@ -4195,7 +4196,7 @@ out_free_tape:
 			if (STm->cdevs[j]) {
 				if (cdev == STm->cdevs[j])
 					cdev = NULL;
-					device_destroy(st_sysfs_class,
+					device_destroy(&st_sysfs_class,
 						       MKDEV(SCSI_TAPE_MAJOR,
 							     TAPE_MINOR(i, mode, j)));
 				cdev_del(STm->cdevs[j]);
@@ -4236,7 +4237,7 @@ static int st_remove(struct device *dev)
 					  "tape");
 			for (mode = 0; mode < ST_NBR_MODES; ++mode) {
 				for (j=0; j < 2; j++) {
-					device_destroy(st_sysfs_class,
+					device_destroy(&st_sysfs_class,
 						       MKDEV(SCSI_TAPE_MAJOR,
 							     TAPE_MINOR(i, mode, j)));
 					cdev_del(tpnt->modes[mode].cdevs[j]);
@@ -4283,6 +4284,11 @@ static void scsi_tape_release(struct kre
 	return;
 }
 
+static struct class st_sysfs_class = {
+	.name = "scsi_tape",
+	.dev_attrs = st_dev_attrs,
+};
+
 static int __init init_st(void)
 {
 	int err;
@@ -4292,10 +4298,10 @@ static int __init init_st(void)
 	printk(KERN_INFO "st: Version %s, fixed bufsize %d, s/g segs %d\n",
 		verstr, st_fixed_buffer_size, st_max_sg_segs);
 
-	st_sysfs_class = class_create(THIS_MODULE, "scsi_tape");
-	if (IS_ERR(st_sysfs_class)) {
-		printk(KERN_ERR "Unable create sysfs class for SCSI tapes\n");
-		return PTR_ERR(st_sysfs_class);
+	err = class_register(&st_sysfs_class);
+	if (err) {
+		pr_err("Unable register sysfs class for SCSI tapes\n");
+		return err;
 	}
 
 	err = register_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
@@ -4322,7 +4328,7 @@ err_chrdev:
 	unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
 				 ST_MAX_TAPE_ENTRIES);
 err_class:
-	class_destroy(st_sysfs_class);
+	class_unregister(&st_sysfs_class);
 	return err;
 }
 
@@ -4332,7 +4338,7 @@ static void __exit exit_st(void)
 	scsi_unregister_driver(&st_template.gendrv);
 	unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
 				 ST_MAX_TAPE_ENTRIES);
-	class_destroy(st_sysfs_class);
+	class_unregister(&st_sysfs_class);
 	kfree(scsi_tapes);
 	printk(KERN_INFO "st: Unloaded.\n");
 }
@@ -4405,10 +4411,9 @@ static void do_remove_sysfs_files(void)
 	driver_remove_file(sysfs, &driver_attr_try_direct_io);
 }
 
-
 /* The sysfs simple class interface */
 static ssize_t
-st_defined_show(struct device *dev, struct device_attribute *attr, char *buf)
+defined_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
 	ssize_t l = 0;
@@ -4417,10 +4422,9 @@ st_defined_show(struct device *dev, stru
 	return l;
 }
 
-DEVICE_ATTR(defined, S_IRUGO, st_defined_show, NULL);
-
 static ssize_t
-st_defblk_show(struct device *dev, struct device_attribute *attr, char *buf)
+default_blksize_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
 	ssize_t l = 0;
@@ -4429,10 +4433,10 @@ st_defblk_show(struct device *dev, struc
 	return l;
 }
 
-DEVICE_ATTR(default_blksize, S_IRUGO, st_defblk_show, NULL);
 
 static ssize_t
-st_defdensity_show(struct device *dev, struct device_attribute *attr, char *buf)
+default_density_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
 	ssize_t l = 0;
@@ -4443,11 +4447,9 @@ st_defdensity_show(struct device *dev, s
 	return l;
 }
 
-DEVICE_ATTR(default_density, S_IRUGO, st_defdensity_show, NULL);
-
 static ssize_t
-st_defcompression_show(struct device *dev, struct device_attribute *attr,
-		       char *buf)
+default_compression_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
 	ssize_t l = 0;
@@ -4456,10 +4458,8 @@ st_defcompression_show(struct device *de
 	return l;
 }
 
-DEVICE_ATTR(default_compression, S_IRUGO, st_defcompression_show, NULL);
-
 static ssize_t
-st_options_show(struct device *dev, struct device_attribute *attr, char *buf)
+options_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
 	struct scsi_tape *STp;
@@ -4498,7 +4498,14 @@ st_options_show(struct device *dev, stru
 	return l;
 }
 
-DEVICE_ATTR(options, S_IRUGO, st_options_show, NULL);
+static struct device_attribute st_dev_attrs[] = {
+	__ATTR_RO(defined),
+	__ATTR_RO(default_blksize),
+	__ATTR_RO(default_density),
+	__ATTR_RO(default_compression),
+	__ATTR_RO(options),
+	__ATTR_NULL,
+};
 
 static int do_create_class_files(struct scsi_tape *STp, int dev_num, int mode)
 {
@@ -4513,7 +4520,8 @@ static int do_create_class_files(struct
 		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
 			 STp->disk->disk_name, st_formats[i]);
 		st_class_member =
-			device_create(st_sysfs_class, &STp->device->sdev_gendev,
+			device_create(&st_sysfs_class,
+				      &STp->device->sdev_gendev,
 				      MKDEV(SCSI_TAPE_MAJOR,
 					    TAPE_MINOR(dev_num, mode, rew)),
 				      &STp->modes[mode], "%s", name);
@@ -4524,22 +4532,6 @@ static int do_create_class_files(struct
 			goto out;
 		}
 
-		error = device_create_file(st_class_member,
-					   &dev_attr_defined);
-		if (error) goto out;
-		error = device_create_file(st_class_member,
-					   &dev_attr_default_blksize);
-		if (error) goto out;
-		error = device_create_file(st_class_member,
-					   &dev_attr_default_density);
-		if (error) goto out;
-		error = device_create_file(st_class_member,
-					   &dev_attr_default_compression);
-		if (error) goto out;
-		error = device_create_file(st_class_member,
-					   &dev_attr_options);
-		if (error) goto out;
-
 		if (mode == 0 && rew == 0) {
 			error = sysfs_create_link(&STp->device->sdev_gendev.kobj,
 						  &st_class_member->kobj,

[patch 2/5] [PATCH 2/5] st: clean up dev cleanup in st_probe

[Jeff Mahoney]

[-- Attachment #1: st-clean-up-dev-cleanup-in-st_probe --]
[-- Type: text/plain, Size: 1230 bytes --]

st_probe leaves a cdev pointer hanging around that is compared
during the error path and freed later. There's no need for the pointer
to hang around at all. So we free it immediately and simplify the error
handling.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 drivers/scsi/st.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4156,6 +4156,7 @@ static int st_probe(struct device *dev)
 				printk(KERN_ERR
 				       "st%d: out of memory. Device not attached.\n",
 				       dev_num);
+				cdev_del(cdev);
 				goto out_free_tape;
 			}
 			cdev->owner = THIS_MODULE;
@@ -4194,17 +4195,13 @@ out_free_tape:
 				  "tape");
 		for (j=0; j < 2; j++) {
 			if (STm->cdevs[j]) {
-				if (cdev == STm->cdevs[j])
-					cdev = NULL;
-					device_destroy(&st_sysfs_class,
-						       MKDEV(SCSI_TAPE_MAJOR,
-							     TAPE_MINOR(i, mode, j)));
+				device_destroy(&st_sysfs_class,
+					       MKDEV(SCSI_TAPE_MAJOR,
+						     TAPE_MINOR(i, mode, j)));
 				cdev_del(STm->cdevs[j]);
 			}
 		}
 	}
-	if (cdev)
-		cdev_del(cdev);
 	write_lock(&st_dev_arr_lock);
 	scsi_tapes[dev_num] = NULL;
 	st_nr_dev--;

[patch 3/5] [PATCH 3/5] st: get rid of scsi_tapes array

[Jeff Mahoney]

[-- Attachment #1: st-get-rid-of-scsi_tapes-array --]
[-- Type: text/plain, Size: 10066 bytes --]

st currently allocates an array to store pointers to all of the
scsi_tape objects. It's used to discover available indexes to use as the
base for the minor number selection and later to look up scsi_tape
devices for character devices.

We switch to using an IDR for minor selection and a pointer from
st_modedef back to scsi_tape for the lookups.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 drivers/scsi/st.c |  176 ++++++++++++++++++++----------------------------------
 drivers/scsi/st.h |    2 
 2 files changed, 69 insertions(+), 109 deletions(-)

--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -37,6 +37,7 @@ static const char *verstr = "20101219";
 #include <linux/blkdev.h>
 #include <linux/moduleparam.h>
 #include <linux/cdev.h>
+#include <linux/idr.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>
 
@@ -81,9 +82,6 @@ static int try_direct_io = TRY_DIRECT_IO
 static int try_rdio = 1;
 static int try_wdio = 1;
 
-static int st_dev_max;
-static int st_nr_dev;
-
 static struct class st_sysfs_class;
 static struct device_attribute st_dev_attrs[];
 
@@ -174,13 +172,9 @@ static int debugging = DEBUG;
    24 bits) */
 #define SET_DENS_AND_BLK 0x10001
 
-static DEFINE_RWLOCK(st_dev_arr_lock);
-
 static int st_fixed_buffer_size = ST_FIXED_BUFFER_SIZE;
 static int st_max_sg_segs = ST_MAX_SG;
 
-static struct scsi_tape **scsi_tapes = NULL;
-
 static int modes_defined;
 
 static int enlarge_buffer(struct st_buffer *, int, int);
@@ -222,6 +216,10 @@ static void scsi_tape_release(struct kre
 #define to_scsi_tape(obj) container_of(obj, struct scsi_tape, kref)
 
 static DEFINE_MUTEX(st_ref_mutex);
+static DEFINE_SPINLOCK(st_index_lock);
+static DEFINE_SPINLOCK(st_use_lock);
+static DEFINE_IDR(st_index_idr);
+
 
 \f

 #include "osst_detect.h"
@@ -239,10 +237,9 @@ static struct scsi_tape *scsi_tape_get(i
 	struct scsi_tape *STp = NULL;
 
 	mutex_lock(&st_ref_mutex);
-	write_lock(&st_dev_arr_lock);
+	spin_lock(&st_index_lock);
 
-	if (dev < st_dev_max && scsi_tapes != NULL)
-		STp = scsi_tapes[dev];
+	STp = idr_find(&st_index_idr, dev);
 	if (!STp) goto out;
 
 	kref_get(&STp->kref);
@@ -259,7 +256,7 @@ out_put:
 	kref_put(&STp->kref, scsi_tape_release);
 	STp = NULL;
 out:
-	write_unlock(&st_dev_arr_lock);
+	spin_unlock(&st_index_lock);
 	mutex_unlock(&st_ref_mutex);
 	return STp;
 }
@@ -1202,12 +1199,12 @@ static int st_open(struct inode *inode,
 		return -ENXIO;
 	}
 
-	write_lock(&st_dev_arr_lock);
 	filp->private_data = STp;
 	name = tape_name(STp);
 
+	spin_lock(&st_use_lock);
 	if (STp->in_use) {
-		write_unlock(&st_dev_arr_lock);
+		spin_unlock(&st_use_lock);
 		scsi_tape_put(STp);
 		mutex_unlock(&st_mutex);
 		DEB( printk(ST_DEB_MSG "%s: Device already in use.\n", name); )
@@ -1215,7 +1212,7 @@ static int st_open(struct inode *inode,
 	}
 
 	STp->in_use = 1;
-	write_unlock(&st_dev_arr_lock);
+	spin_unlock(&st_use_lock);
 	STp->rew_at_close = STp->autorew_dev = (iminor(inode) & 0x80) == 0;
 
 	if (scsi_autopm_get_device(STp->device) < 0) {
@@ -1404,9 +1401,9 @@ static int st_release(struct inode *inod
 		do_door_lock(STp, 0);
 
 	normalize_buffer(STp->buffer);
-	write_lock(&st_dev_arr_lock);
+	spin_lock(&st_use_lock);
 	STp->in_use = 0;
-	write_unlock(&st_dev_arr_lock);
+	spin_unlock(&st_use_lock);
 	scsi_autopm_put_device(STp->device);
 	scsi_tape_put(STp);
 
@@ -4029,58 +4026,16 @@ static int st_probe(struct device *dev)
 		goto out_buffer_free;
 	}
 
-	write_lock(&st_dev_arr_lock);
-	if (st_nr_dev >= st_dev_max) {
-		struct scsi_tape **tmp_da;
-		int tmp_dev_max;
-
-		tmp_dev_max = max(st_nr_dev * 2, 8);
-		if (tmp_dev_max > ST_MAX_TAPES)
-			tmp_dev_max = ST_MAX_TAPES;
-		if (tmp_dev_max <= st_nr_dev) {
-			write_unlock(&st_dev_arr_lock);
-			printk(KERN_ERR "st: Too many tape devices (max. %d).\n",
-			       ST_MAX_TAPES);
-			goto out_put_disk;
-		}
-
-		tmp_da = kzalloc(tmp_dev_max * sizeof(struct scsi_tape *), GFP_ATOMIC);
-		if (tmp_da == NULL) {
-			write_unlock(&st_dev_arr_lock);
-			printk(KERN_ERR "st: Can't extend device array.\n");
-			goto out_put_disk;
-		}
-
-		if (scsi_tapes != NULL) {
-			memcpy(tmp_da, scsi_tapes,
-			       st_dev_max * sizeof(struct scsi_tape *));
-			kfree(scsi_tapes);
-		}
-		scsi_tapes = tmp_da;
-
-		st_dev_max = tmp_dev_max;
-	}
-
-	for (i = 0; i < st_dev_max; i++)
-		if (scsi_tapes[i] == NULL)
-			break;
-	if (i >= st_dev_max)
-		panic("scsi_devices corrupt (st)");
-
 	tpnt = kzalloc(sizeof(struct scsi_tape), GFP_ATOMIC);
 	if (tpnt == NULL) {
-		write_unlock(&st_dev_arr_lock);
 		printk(KERN_ERR "st: Can't allocate device descriptor.\n");
 		goto out_put_disk;
 	}
 	kref_init(&tpnt->kref);
 	tpnt->disk = disk;
-	sprintf(disk->disk_name, "st%d", i);
 	disk->private_data = &tpnt->driver;
 	disk->queue = SDp->request_queue;
 	tpnt->driver = &st_template;
-	scsi_tapes[i] = tpnt;
-	dev_num = i;
 
 	tpnt->device = SDp;
 	if (SDp->scsi_level <= 2)
@@ -4126,6 +4081,7 @@ static int st_probe(struct device *dev)
 		STm->default_compression = ST_DONT_TOUCH;
 		STm->default_blksize = (-1);	/* No forced size */
 		STm->default_density = (-1);	/* No forced density */
+		STm->tape = tpnt;
 	}
 
 	for (i = 0; i < ST_NBR_PARTITIONS; i++) {
@@ -4145,8 +4101,29 @@ static int st_probe(struct device *dev)
 	    tpnt->blksize_changed = 0;
 	mutex_init(&tpnt->lock);
 
-	st_nr_dev++;
-	write_unlock(&st_dev_arr_lock);
+	if (!idr_pre_get(&st_index_idr, GFP_KERNEL)) {
+		pr_warn("st: idr expansion failed\n");
+		error = -ENOMEM;
+		goto out_put_disk;
+	}
+
+	spin_lock(&st_index_lock);
+	error = idr_get_new(&st_index_idr, tpnt, &dev_num);
+	spin_unlock(&st_index_lock);
+	if (error) {
+		pr_warn("st: idr allocation failed: %d\n", error);
+		goto out_put_disk;
+	}
+
+	if (dev_num > ST_MAX_TAPES) {
+		pr_err("st: Too many tape devices (max. %d).\n", ST_MAX_TAPES);
+		goto out_put_index;
+	}
+
+	tpnt->index = dev_num;
+	sprintf(disk->disk_name, "st%d", dev_num);
+
+	dev_set_drvdata(dev, tpnt);
 
 	for (mode = 0; mode < ST_NBR_MODES; ++mode) {
 		STm = &(tpnt->modes[mode]);
@@ -4189,10 +4166,10 @@ static int st_probe(struct device *dev)
 	return 0;
 
 out_free_tape:
+	sysfs_remove_link(&tpnt->device->sdev_gendev.kobj,
+			  "tape");
 	for (mode=0; mode < ST_NBR_MODES; mode++) {
 		STm = &(tpnt->modes[mode]);
-		sysfs_remove_link(&tpnt->device->sdev_gendev.kobj,
-				  "tape");
 		for (j=0; j < 2; j++) {
 			if (STm->cdevs[j]) {
 				device_destroy(&st_sysfs_class,
@@ -4202,10 +4179,10 @@ out_free_tape:
 			}
 		}
 	}
-	write_lock(&st_dev_arr_lock);
-	scsi_tapes[dev_num] = NULL;
-	st_nr_dev--;
-	write_unlock(&st_dev_arr_lock);
+out_put_index:
+	spin_lock(&st_index_lock);
+	idr_remove(&st_index_idr, dev_num);
+	spin_unlock(&st_index_lock);
 out_put_disk:
 	put_disk(disk);
 	kfree(tpnt);
@@ -4218,38 +4195,32 @@ out:
 
 static int st_remove(struct device *dev)
 {
-	struct scsi_device *SDp = to_scsi_device(dev);
-	struct scsi_tape *tpnt;
-	int i, j, mode;
+	struct scsi_tape *tpnt = dev_get_drvdata(dev);
+	int rew, mode;
+	dev_t cdev_devno;
+	struct cdev *cdev;
+	int index = tpnt->index;
 
-	scsi_autopm_get_device(SDp);
-	write_lock(&st_dev_arr_lock);
-	for (i = 0; i < st_dev_max; i++) {
-		tpnt = scsi_tapes[i];
-		if (tpnt != NULL && tpnt->device == SDp) {
-			scsi_tapes[i] = NULL;
-			st_nr_dev--;
-			write_unlock(&st_dev_arr_lock);
-			sysfs_remove_link(&tpnt->device->sdev_gendev.kobj,
-					  "tape");
-			for (mode = 0; mode < ST_NBR_MODES; ++mode) {
-				for (j=0; j < 2; j++) {
-					device_destroy(&st_sysfs_class,
-						       MKDEV(SCSI_TAPE_MAJOR,
-							     TAPE_MINOR(i, mode, j)));
-					cdev_del(tpnt->modes[mode].cdevs[j]);
-					tpnt->modes[mode].cdevs[j] = NULL;
-				}
-			}
-
-			mutex_lock(&st_ref_mutex);
-			kref_put(&tpnt->kref, scsi_tape_release);
-			mutex_unlock(&st_ref_mutex);
-			return 0;
+	scsi_autopm_get_device(to_scsi_device(dev));
+	sysfs_remove_link(&tpnt->device->sdev_gendev.kobj, "tape");
+	for (mode = 0; mode < ST_NBR_MODES; ++mode) {
+		for (rew = 0; rew < 2; rew++) {
+			cdev = tpnt->modes[mode].cdevs[rew];
+			if (!cdev)
+				continue;
+			cdev_devno = cdev->dev;
+			device_destroy(&st_sysfs_class, cdev_devno);
+			cdev_del(tpnt->modes[mode].cdevs[rew]);
+			tpnt->modes[mode].cdevs[rew] = NULL;
 		}
 	}
 
-	write_unlock(&st_dev_arr_lock);
+	mutex_lock(&st_ref_mutex);
+	kref_put(&tpnt->kref, scsi_tape_release);
+	mutex_unlock(&st_ref_mutex);
+	spin_lock(&st_index_lock);
+	idr_remove(&st_index_idr, index);
+	spin_unlock(&st_index_lock);
 	return 0;
 }
 
@@ -4336,7 +4307,6 @@ static void __exit exit_st(void)
 	unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
 				 ST_MAX_TAPE_ENTRIES);
 	class_unregister(&st_sysfs_class);
-	kfree(scsi_tapes);
 	printk(KERN_INFO "st: Unloaded.\n");
 }
 
@@ -4459,22 +4429,10 @@ static ssize_t
 options_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct st_modedef *STm = dev_get_drvdata(dev);
-	struct scsi_tape *STp;
-	int i, j, options;
+	struct scsi_tape *STp = STm->tape;
+	int options;
 	ssize_t l = 0;
 
-	for (i=0; i < st_dev_max; i++) {
-		for (j=0; j < ST_NBR_MODES; j++)
-			if (&scsi_tapes[i]->modes[j] == STm)
-				break;
-		if (j < ST_NBR_MODES)
-			break;
-	}
-	if (i == st_dev_max)
-		return 0;  /* should never happen */
-
-	STp = scsi_tapes[i];
-
 	options = STm->do_buffer_writes ? MT_ST_BUFFER_WRITES : 0;
 	options |= STm->do_async_writes ? MT_ST_ASYNC_WRITES : 0;
 	options |= STm->do_read_ahead ? MT_ST_READ_AHEAD : 0;
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -66,6 +66,7 @@ struct st_modedef {
 	unsigned char default_compression;	/* 0 = don't touch, etc */
 	short default_density;	/* Forced density, -1 = no value */
 	int default_blksize;	/* Forced blocksize, -1 = no value */
+	struct scsi_tape *tape;
 	struct cdev *cdevs[2];  /* Auto-rewind and non-rewind devices */
 };
 
@@ -99,6 +100,7 @@ struct scsi_tape {
 	struct mutex lock;	/* For serialization */
 	struct completion wait;	/* For SCSI commands */
 	struct st_buffer *buffer;
+	int index;
 
 	/* Drive characteristics */
 	unsigned char omit_blklims;

[patch 4/5] [PATCH 4/5] st: clean up device file creation and removal

[Jeff Mahoney]

[-- Attachment #1: st-clean-up-device-file-creation-and-removal --]
[-- Type: text/plain, Size: 7332 bytes --]

This patch cleans up the st device file creation and removal.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 drivers/scsi/st.c |  195 ++++++++++++++++++++++++------------------------------
 drivers/scsi/st.h |    1 
 2 files changed, 91 insertions(+), 105 deletions(-)

--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -193,7 +193,6 @@ static int st_remove(struct device *);
 
 static int do_create_sysfs_files(void);
 static void do_remove_sysfs_files(void);
-static int do_create_class_files(struct scsi_tape *, int, int);
 
 static struct scsi_driver st_template = {
 	.owner			= THIS_MODULE,
@@ -3990,16 +3989,98 @@ static const struct file_operations st_f
 	.llseek =	noop_llseek,
 };
 
+static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
+{
+	int i, error;
+	dev_t cdev_devno;
+	struct cdev *cdev;
+	struct device *dev;
+	struct st_modedef *STm = &(tape->modes[mode]);
+	char name[10];
+	int dev_num = tape->index;
+
+	cdev_devno = MKDEV(SCSI_TAPE_MAJOR, TAPE_MINOR(dev_num, mode, rew));
+
+	cdev = cdev_alloc();
+	if (!cdev) {
+		pr_err("st%d: out of memory. Device not attached.\n", dev_num);
+		error = -ENOMEM;
+		goto out;
+	}
+	cdev->owner = THIS_MODULE;
+	cdev->ops = &st_fops;
+
+	error = cdev_add(cdev, cdev_devno, 1);
+	if (error) {
+		pr_err("st%d: Can't add %s-rewind mode %d\n", dev_num,
+		       rew ? "non" : "auto", mode);
+		pr_err("st%d: Device not attached.\n", dev_num);
+		goto out_free;
+	}
+	STm->cdevs[rew] = cdev;
+
+	i = mode << (4 - ST_NBR_MODE_BITS);
+	snprintf(name, 10, "%s%s%s", rew ? "n" : "",
+		 tape->disk->disk_name, st_formats[i]);
+
+	dev = device_create(&st_sysfs_class, &tape->device->sdev_gendev,
+			    cdev_devno, &tape->modes[mode], "%s", name);
+	if (IS_ERR(dev)) {
+		pr_err("st%d: device_create failed\n", dev_num);
+		error = PTR_ERR(dev);
+		goto out_free;
+	}
+
+	STm->devs[rew] = dev;
+
+	return 0;
+out_free:
+	cdev_del(STm->cdevs[rew]);
+	STm->cdevs[rew] = NULL;
+out:
+	return error;
+}
+
+static int create_cdevs(struct scsi_tape *tape)
+{
+	int mode, error;
+	for (mode = 0; mode < ST_NBR_MODES; ++mode) {
+		error = create_one_cdev(tape, mode, 0);
+		if (error)
+			return error;
+		error = create_one_cdev(tape, mode, 1);
+		if (error)
+			return error;
+	}
+
+	return sysfs_create_link(&tape->device->sdev_gendev.kobj,
+				 &tape->modes[0].devs[0]->kobj, "tape");
+}
+
+static void remove_cdevs(struct scsi_tape *tape)
+{
+	int mode, rew;
+	sysfs_remove_link(&tape->device->sdev_gendev.kobj, "tape");
+	for (mode = 0; mode < ST_NBR_MODES; mode++) {
+		struct st_modedef *STm = &(tape->modes[mode]);
+		for (rew = 0; rew < 2; rew++) {
+			if (STm->cdevs[rew])
+				cdev_del(STm->cdevs[rew]);
+			if (STm->devs[rew])
+				device_unregister(STm->devs[rew]);
+		}
+	}
+}
+
 static int st_probe(struct device *dev)
 {
 	struct scsi_device *SDp = to_scsi_device(dev);
 	struct gendisk *disk = NULL;
-	struct cdev *cdev = NULL;
 	struct scsi_tape *tpnt = NULL;
 	struct st_modedef *STm;
 	struct st_partstat *STps;
 	struct st_buffer *buffer;
-	int i, j, mode, dev_num, error;
+	int i, dev_num, error;
 	char *stp;
 
 	if (SDp->type != TYPE_TAPE)
@@ -4125,36 +4206,10 @@ static int st_probe(struct device *dev)
 
 	dev_set_drvdata(dev, tpnt);
 
-	for (mode = 0; mode < ST_NBR_MODES; ++mode) {
-		STm = &(tpnt->modes[mode]);
-		for (j=0; j < 2; j++) {
-			cdev = cdev_alloc();
-			if (!cdev) {
-				printk(KERN_ERR
-				       "st%d: out of memory. Device not attached.\n",
-				       dev_num);
-				cdev_del(cdev);
-				goto out_free_tape;
-			}
-			cdev->owner = THIS_MODULE;
-			cdev->ops = &st_fops;
 
-			error = cdev_add(cdev,
-					 MKDEV(SCSI_TAPE_MAJOR, TAPE_MINOR(dev_num, mode, j)),
-					 1);
-			if (error) {
-				printk(KERN_ERR "st%d: Can't add %s-rewind mode %d\n",
-				       dev_num, j ? "non" : "auto", mode);
-				printk(KERN_ERR "st%d: Device not attached.\n", dev_num);
-				goto out_free_tape;
-			}
-			STm->cdevs[j] = cdev;
-
-		}
-		error = do_create_class_files(tpnt, dev_num, mode);
-		if (error)
-			goto out_free_tape;
-	}
+	error = create_cdevs(tpnt);
+	if (error)
+		goto out_remove_devs;
 	scsi_autopm_put_device(SDp);
 
 	sdev_printk(KERN_NOTICE, SDp,
@@ -4165,20 +4220,8 @@ static int st_probe(struct device *dev)
 
 	return 0;
 
-out_free_tape:
-	sysfs_remove_link(&tpnt->device->sdev_gendev.kobj,
-			  "tape");
-	for (mode=0; mode < ST_NBR_MODES; mode++) {
-		STm = &(tpnt->modes[mode]);
-		for (j=0; j < 2; j++) {
-			if (STm->cdevs[j]) {
-				device_destroy(&st_sysfs_class,
-					       MKDEV(SCSI_TAPE_MAJOR,
-						     TAPE_MINOR(i, mode, j)));
-				cdev_del(STm->cdevs[j]);
-			}
-		}
-	}
+out_remove_devs:
+	remove_cdevs(tpnt);
 out_put_index:
 	spin_lock(&st_index_lock);
 	idr_remove(&st_index_idr, dev_num);
@@ -4196,24 +4239,10 @@ out:
 static int st_remove(struct device *dev)
 {
 	struct scsi_tape *tpnt = dev_get_drvdata(dev);
-	int rew, mode;
-	dev_t cdev_devno;
-	struct cdev *cdev;
 	int index = tpnt->index;
 
 	scsi_autopm_get_device(to_scsi_device(dev));
-	sysfs_remove_link(&tpnt->device->sdev_gendev.kobj, "tape");
-	for (mode = 0; mode < ST_NBR_MODES; ++mode) {
-		for (rew = 0; rew < 2; rew++) {
-			cdev = tpnt->modes[mode].cdevs[rew];
-			if (!cdev)
-				continue;
-			cdev_devno = cdev->dev;
-			device_destroy(&st_sysfs_class, cdev_devno);
-			cdev_del(tpnt->modes[mode].cdevs[rew]);
-			tpnt->modes[mode].cdevs[rew] = NULL;
-		}
-	}
+	remove_cdevs(tpnt);
 
 	mutex_lock(&st_ref_mutex);
 	kref_put(&tpnt->kref, scsi_tape_release);
@@ -4462,50 +4491,6 @@ static struct device_attribute st_dev_at
 	__ATTR_NULL,
 };
 
-static int do_create_class_files(struct scsi_tape *STp, int dev_num, int mode)
-{
-	int i, rew, error;
-	char name[10];
-	struct device *st_class_member;
-
-	for (rew=0; rew < 2; rew++) {
-		/* Make sure that the minor numbers corresponding to the four
-		   first modes always get the same names */
-		i = mode << (4 - ST_NBR_MODE_BITS);
-		snprintf(name, 10, "%s%s%s", rew ? "n" : "",
-			 STp->disk->disk_name, st_formats[i]);
-		st_class_member =
-			device_create(&st_sysfs_class,
-				      &STp->device->sdev_gendev,
-				      MKDEV(SCSI_TAPE_MAJOR,
-					    TAPE_MINOR(dev_num, mode, rew)),
-				      &STp->modes[mode], "%s", name);
-		if (IS_ERR(st_class_member)) {
-			printk(KERN_WARNING "st%d: device_create failed\n",
-			       dev_num);
-			error = PTR_ERR(st_class_member);
-			goto out;
-		}
-
-		if (mode == 0 && rew == 0) {
-			error = sysfs_create_link(&STp->device->sdev_gendev.kobj,
-						  &st_class_member->kobj,
-						  "tape");
-			if (error) {
-				printk(KERN_ERR
-				       "st%d: Can't create sysfs link from SCSI device.\n",
-				       dev_num);
-				goto out;
-			}
-		}
-	}
-
-	return 0;
-
-out:
-	return error;
-}
-
 /* The following functions may be useful for a larger audience. */
 static int sgl_map_user_pages(struct st_buffer *STbp,
 			      const unsigned int max_pages, unsigned long uaddr,
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -67,6 +67,7 @@ struct st_modedef {
 	short default_density;	/* Forced density, -1 = no value */
 	int default_blksize;	/* Forced blocksize, -1 = no value */
 	struct scsi_tape *tape;
+	struct device *devs[2];  /* Auto-rewind and non-rewind devices */
 	struct cdev *cdevs[2];  /* Auto-rewind and non-rewind devices */
 };
 

[patch 5/5] [PATCH 5/5] st: raise device limit

[Jeff Mahoney]

[-- Attachment #1: st-raise-device-limit --]
[-- Type: text/plain, Size: 1631 bytes --]

The device limit of 128 tape drives was established in 2003 as a
significant increase from the 8 tape drives allowed previously.

We're seeing customer sites that between a large number of drives
and multipath are discovering more than 128 devices and running
into problems.

Now that we're not stuck having to store a pointer in array
and aren't limited by kmalloc failing on higher order allocs we can
lift the limit to fill the entire minor range based on the number
of modes.

Based on the current code, that's 2^17 devices.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
 Documentation/scsi/st.txt |    6 ++----
 drivers/scsi/st.h         |    2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

--- a/Documentation/scsi/st.txt
+++ b/Documentation/scsi/st.txt
@@ -112,10 +112,8 @@ attempted).
 
 MINOR NUMBERS
 
-The tape driver currently supports 128 drives by default. This number
-can be increased by editing st.h and recompiling the driver if
-necessary. The upper limit is 2^17 drives if 4 modes for each drive
-are used.
+The tape driver currently supports up to 2^17 drives if 4 modes for
+each drive are used.
 
 The minor numbers consist of the following bit fields:
 
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -78,7 +78,7 @@ struct st_modedef {
 #define ST_MODE_SHIFT (7 - ST_NBR_MODE_BITS)
 #define ST_MODE_MASK ((ST_NBR_MODES - 1) << ST_MODE_SHIFT)
 
-#define ST_MAX_TAPES 128
+#define ST_MAX_TAPES (1 << (20 - (ST_NBR_MODE_BITS + 1)))
 #define ST_MAX_TAPE_ENTRIES  (ST_MAX_TAPES << (ST_NBR_MODE_BITS + 1))
 
 /* The status related to each partition */

Re: [patch 0/5] st: Clean up and raise max device limit (v4)

[Kai Makisara]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 822 bytes --]

On Sat, 18 Aug 2012, Jeff Mahoney wrote:

> This patchset cleans up the SCSI tape device handling code and leverages it
> to lift the limitation of the number of tape drives from the previous
> arbitrary limit of 128 to the maximum supported by a device node that
> creates 8 character devices per physical device. Since minors are 20 bits,
> that means 2^17 tape drives can be supported.
> 
> Changed in this version: A previous revision introduced a regression where
> tape drives would not be shown as tape devices in lsscsi. This was due to
> the missing "tape" symlink in sysfs. That issue has been addressed and
> lsscsi works properly again. Also, it passes checkpatch with no errors.
> 
> Please apply.
> 
The whole series (1-5):

Acked-by: Kai Mäkisara <kai.makisara@kolumbus.fi>

Thanks,
Kai