diff options
author | FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> | 2008-01-24 03:24:50 -0500 |
---|---|---|
committer | James Bottomley <James.Bottomley@HansenPartnership.com> | 2008-01-25 10:21:55 -0500 |
commit | da707c54c3424b4b50d4352c2103867284ba6724 (patch) | |
tree | 05503ae45fc0f0df3368c89d364a9373d1070df4 | |
parent | a3d2c2e8f5e01e185013d8f944c0a26fdc558ad8 (diff) |
[SCSI] ch: fix device minor number management bug
ch_probe uses the total number of ch devices as minor.
ch_probe:
ch->minor = ch_devcount;
...
ch_devcount++;
Then ch_remove decreases ch_devcount:
ch_remove:
ch_devcount--;
If you have two ch devices, sch0 and sch1, and remove sch0,
ch_devcount is 1. Then if you add another ch device, ch_probe tries to
create sch1. So you get a warning and fail to create sch1:
Jan 24 16:01:05 nice kernel: sysfs: duplicate filename 'sch1' can not be created
Jan 24 16:01:05 nice kernel: WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
Jan 24 16:01:05 nice kernel: Pid: 2571, comm: iscsid Not tainted 2.6.24-rc7-ga3d2c2e8-dirty #1
Jan 24 16:01:05 nice kernel:
Jan 24 16:01:05 nice kernel: Call Trace:
Jan 24 16:01:05 nice kernel: [<ffffffff802a22b8>] sysfs_add_one+0x54/0xbd
Jan 24 16:01:05 nice kernel: [<ffffffff802a283c>] create_dir+0x4f/0x87
Jan 24 16:01:05 nice kernel: [<ffffffff802a28a9>] sysfs_create_dir+0x35/0x4a
Jan 24 16:01:05 nice kernel: [<ffffffff803069a1>] kobject_get+0x12/0x17
Jan 24 16:01:05 nice kernel: [<ffffffff80306ece>] kobject_add+0xf3/0x1a6
Jan 24 16:01:05 nice kernel: [<ffffffff8034252b>] class_device_add+0xaa/0x39d
Jan 24 16:01:05 nice kernel: [<ffffffff803428fb>] class_device_create+0xcb/0xfa
Jan 24 16:01:05 nice kernel: [<ffffffff80229e09>] printk+0x4e/0x56
Jan 24 16:01:05 nice kernel: [<ffffffff802a2054>] sysfs_ilookup_test+0x0/0xf
Jan 24 16:01:05 nice kernel: [<ffffffff88022580>] :ch:ch_probe+0xbe/0x61a
(snip)
This patch converts ch to use a standard minor number management way,
idr like sg and bsg.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
-rw-r--r-- | drivers/scsi/ch.c | 71 |
1 files changed, 40 insertions, 31 deletions
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c index 765f2fc001aa..2b07014cbc83 100644 --- a/drivers/scsi/ch.c +++ b/drivers/scsi/ch.c | |||
@@ -21,6 +21,7 @@ | |||
21 | #include <linux/compat.h> | 21 | #include <linux/compat.h> |
22 | #include <linux/chio.h> /* here are all the ioctls */ | 22 | #include <linux/chio.h> /* here are all the ioctls */ |
23 | #include <linux/mutex.h> | 23 | #include <linux/mutex.h> |
24 | #include <linux/idr.h> | ||
24 | 25 | ||
25 | #include <scsi/scsi.h> | 26 | #include <scsi/scsi.h> |
26 | #include <scsi/scsi_cmnd.h> | 27 | #include <scsi/scsi_cmnd.h> |
@@ -33,6 +34,7 @@ | |||
33 | 34 | ||
34 | #define CH_DT_MAX 16 | 35 | #define CH_DT_MAX 16 |
35 | #define CH_TYPES 8 | 36 | #define CH_TYPES 8 |
37 | #define CH_MAX_DEVS 128 | ||
36 | 38 | ||
37 | MODULE_DESCRIPTION("device driver for scsi media changer devices"); | 39 | MODULE_DESCRIPTION("device driver for scsi media changer devices"); |
38 | MODULE_AUTHOR("Gerd Knorr <kraxel@bytesex.org>"); | 40 | MODULE_AUTHOR("Gerd Knorr <kraxel@bytesex.org>"); |
@@ -113,9 +115,8 @@ typedef struct { | |||
113 | struct mutex lock; | 115 | struct mutex lock; |
114 | } scsi_changer; | 116 | } scsi_changer; |
115 | 117 | ||
116 | static LIST_HEAD(ch_devlist); | 118 | static DEFINE_IDR(ch_index_idr); |
117 | static DEFINE_SPINLOCK(ch_devlist_lock); | 119 | static DEFINE_SPINLOCK(ch_index_lock); |
118 | static int ch_devcount; | ||
119 | 120 | ||
120 | static struct scsi_driver ch_template = | 121 | static struct scsi_driver ch_template = |
121 | { | 122 | { |
@@ -598,20 +599,17 @@ ch_release(struct inode *inode, struct file *file) | |||
598 | static int | 599 | static int |
599 | ch_open(struct inode *inode, struct file *file) | 600 | ch_open(struct inode *inode, struct file *file) |
600 | { | 601 | { |
601 | scsi_changer *tmp, *ch; | 602 | scsi_changer *ch; |
602 | int minor = iminor(inode); | 603 | int minor = iminor(inode); |
603 | 604 | ||
604 | spin_lock(&ch_devlist_lock); | 605 | spin_lock(&ch_index_lock); |
605 | ch = NULL; | 606 | ch = idr_find(&ch_index_idr, minor); |
606 | list_for_each_entry(tmp,&ch_devlist,list) { | 607 | |
607 | if (tmp->minor == minor) | ||
608 | ch = tmp; | ||
609 | } | ||
610 | if (NULL == ch || scsi_device_get(ch->device)) { | 608 | if (NULL == ch || scsi_device_get(ch->device)) { |
611 | spin_unlock(&ch_devlist_lock); | 609 | spin_unlock(&ch_index_lock); |
612 | return -ENXIO; | 610 | return -ENXIO; |
613 | } | 611 | } |
614 | spin_unlock(&ch_devlist_lock); | 612 | spin_unlock(&ch_index_lock); |
615 | 613 | ||
616 | file->private_data = ch; | 614 | file->private_data = ch; |
617 | return 0; | 615 | return 0; |
@@ -914,6 +912,7 @@ static int ch_probe(struct device *dev) | |||
914 | { | 912 | { |
915 | struct scsi_device *sd = to_scsi_device(dev); | 913 | struct scsi_device *sd = to_scsi_device(dev); |
916 | struct class_device *class_dev; | 914 | struct class_device *class_dev; |
915 | int minor, ret = -ENOMEM; | ||
917 | scsi_changer *ch; | 916 | scsi_changer *ch; |
918 | 917 | ||
919 | if (sd->type != TYPE_MEDIUM_CHANGER) | 918 | if (sd->type != TYPE_MEDIUM_CHANGER) |
@@ -923,7 +922,22 @@ static int ch_probe(struct device *dev) | |||
923 | if (NULL == ch) | 922 | if (NULL == ch) |
924 | return -ENOMEM; | 923 | return -ENOMEM; |
925 | 924 | ||
926 | ch->minor = ch_devcount; | 925 | if (!idr_pre_get(&ch_index_idr, GFP_KERNEL)) |
926 | goto free_ch; | ||
927 | |||
928 | spin_lock(&ch_index_lock); | ||
929 | ret = idr_get_new(&ch_index_idr, ch, &minor); | ||
930 | spin_unlock(&ch_index_lock); | ||
931 | |||
932 | if (ret) | ||
933 | goto free_ch; | ||
934 | |||
935 | if (minor > CH_MAX_DEVS) { | ||
936 | ret = -ENODEV; | ||
937 | goto remove_idr; | ||
938 | } | ||
939 | |||
940 | ch->minor = minor; | ||
927 | sprintf(ch->name,"ch%d",ch->minor); | 941 | sprintf(ch->name,"ch%d",ch->minor); |
928 | 942 | ||
929 | class_dev = class_device_create(ch_sysfs_class, NULL, | 943 | class_dev = class_device_create(ch_sysfs_class, NULL, |
@@ -932,8 +946,8 @@ static int ch_probe(struct device *dev) | |||
932 | if (IS_ERR(class_dev)) { | 946 | if (IS_ERR(class_dev)) { |
933 | printk(KERN_WARNING "ch%d: class_device_create failed\n", | 947 | printk(KERN_WARNING "ch%d: class_device_create failed\n", |
934 | ch->minor); | 948 | ch->minor); |
935 | kfree(ch); | 949 | ret = PTR_ERR(class_dev); |
936 | return PTR_ERR(class_dev); | 950 | goto remove_idr; |
937 | } | 951 | } |
938 | 952 | ||
939 | mutex_init(&ch->lock); | 953 | mutex_init(&ch->lock); |
@@ -942,35 +956,29 @@ static int ch_probe(struct device *dev) | |||
942 | if (init) | 956 | if (init) |
943 | ch_init_elem(ch); | 957 | ch_init_elem(ch); |
944 | 958 | ||
959 | dev_set_drvdata(dev, ch); | ||
945 | sdev_printk(KERN_INFO, sd, "Attached scsi changer %s\n", ch->name); | 960 | sdev_printk(KERN_INFO, sd, "Attached scsi changer %s\n", ch->name); |
946 | 961 | ||
947 | spin_lock(&ch_devlist_lock); | ||
948 | list_add_tail(&ch->list,&ch_devlist); | ||
949 | ch_devcount++; | ||
950 | spin_unlock(&ch_devlist_lock); | ||
951 | return 0; | 962 | return 0; |
963 | remove_idr: | ||
964 | idr_remove(&ch_index_idr, minor); | ||
965 | free_ch: | ||
966 | kfree(ch); | ||
967 | return ret; | ||
952 | } | 968 | } |
953 | 969 | ||
954 | static int ch_remove(struct device *dev) | 970 | static int ch_remove(struct device *dev) |
955 | { | 971 | { |
956 | struct scsi_device *sd = to_scsi_device(dev); | 972 | scsi_changer *ch = dev_get_drvdata(dev); |
957 | scsi_changer *tmp, *ch; | ||
958 | 973 | ||
959 | spin_lock(&ch_devlist_lock); | 974 | spin_lock(&ch_index_lock); |
960 | ch = NULL; | 975 | idr_remove(&ch_index_idr, ch->minor); |
961 | list_for_each_entry(tmp,&ch_devlist,list) { | 976 | spin_unlock(&ch_index_lock); |
962 | if (tmp->device == sd) | ||
963 | ch = tmp; | ||
964 | } | ||
965 | BUG_ON(NULL == ch); | ||
966 | list_del(&ch->list); | ||
967 | spin_unlock(&ch_devlist_lock); | ||
968 | 977 | ||
969 | class_device_destroy(ch_sysfs_class, | 978 | class_device_destroy(ch_sysfs_class, |
970 | MKDEV(SCSI_CHANGER_MAJOR,ch->minor)); | 979 | MKDEV(SCSI_CHANGER_MAJOR,ch->minor)); |
971 | kfree(ch->dt); | 980 | kfree(ch->dt); |
972 | kfree(ch); | 981 | kfree(ch); |
973 | ch_devcount--; | ||
974 | return 0; | 982 | return 0; |
975 | } | 983 | } |
976 | 984 | ||
@@ -1007,6 +1015,7 @@ static void __exit exit_ch_module(void) | |||
1007 | scsi_unregister_driver(&ch_template.gendrv); | 1015 | scsi_unregister_driver(&ch_template.gendrv); |
1008 | unregister_chrdev(SCSI_CHANGER_MAJOR, "ch"); | 1016 | unregister_chrdev(SCSI_CHANGER_MAJOR, "ch"); |
1009 | class_destroy(ch_sysfs_class); | 1017 | class_destroy(ch_sysfs_class); |
1018 | idr_destroy(&ch_index_idr); | ||
1010 | } | 1019 | } |
1011 | 1020 | ||
1012 | module_init(init_ch_module); | 1021 | module_init(init_ch_module); |