[v6,0/4] scsi: ufs & ums-* & esp_scsi: fix module reference counting

Commit Message

Akinobu Mita May 5, 2015, 1:39 p.m. UTC

2015-05-05 6:41 GMT+09:00 James Bottomley
<[e-mail]>:
> On Mon, 2015-05-04 at 16:09 -0400, Alan Stern wrote:
>> On Mon, 4 May 2015, James Bottomley wrote:
>>
>> > However, it does also strike me that these three drivers have problems
>> > because they're using the wrong initialisation pattern: the template is
>> > supposed to be in the bus connector for compound drivers not in the
>> > core.
>>
>> Why is it supposed to be done that way?  Isn't that less efficient?  It
>> means you have to have a separate copy of the template for each bus
>> connector driver, instead of letting them all share a common template
>> in the core.
>
> Well, no it doesn't.  The way 53c700 implements it is that there is a
> common template in the core.  The drivers just initialise their variant
> fields (for 53c700 it's name, proc_name and this_id) and the core fills
> in the rest.  Admittedly wd33c93 doesn't quite get this right, that's
> why I cited 53c700.

I have converted usb-storage and ums-alauda in the attached patch.
Each ums-* driver needs to expand module_usb_driver() macro as we need
to initialize their scsi host template in module_init().

Alan, how do you feel the change like this?

Patch

From 4c6aa0b56c153afe331a98969a857f16d99f96b6 Mon Sep 17 00:00:00 2001
From: Akinobu Mita <[e-mail]>
Date: Tue, 5 May 2015 22:30:04 +0900
Subject: [PATCH] usb: storage: fix module reference  for scsi host

---
 drivers/usb/storage/alauda.c   | 21 +++++++++++++++++++--
 drivers/usb/storage/scsiglue.c | 12 +++++++++++-
 drivers/usb/storage/scsiglue.h |  3 ++-
 drivers/usb/storage/usb.c      | 25 +++++++++++++++++++++----
 drivers/usb/storage/usb.h      |  3 ++-
 5 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 4b55ab6..0baa484 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -42,6 +42,7 @@ 
 #include "transport.h"
 #include "protocol.h"
 #include "debug.h"
+#include "scsiglue.h"
 
 MODULE_DESCRIPTION("Driver for Alauda-based card readers");
 MODULE_AUTHOR("Daniel Drake <[e-mail]>");
@@ -1232,6 +1233,8 @@  static int alauda_transport(struct scsi_cmnd *srb, struct us_data *us)
 	return USB_STOR_TRANSPORT_FAILED;
 }
 
+static struct scsi_host_template alauda_host_template;
+
 static int alauda_probe(struct usb_interface *intf,
 			 const struct usb_device_id *id)
 {
@@ -1239,7 +1242,8 @@  static int alauda_probe(struct usb_interface *intf,
 	int result;
 
 	result = usb_stor_probe1(&us, intf, id,
-			(id - alauda_usb_ids) + alauda_unusual_dev_list);
+			(id - alauda_usb_ids) + alauda_unusual_dev_list,
+			&alauda_host_template);
 	if (result)
 		return result;
 
@@ -1266,4 +1270,17 @@  static struct usb_driver alauda_driver = {
 	.no_dynamic_id = 1,
 };
 
-module_usb_driver(alauda_driver);
+static int __init alauda_driver_init(void)
+{
+	usb_stor_host_template_init(&alauda_host_template, "ums-alauda",
+				    THIS_MODULE);
+
+	return usb_register(&alauda_driver);
+}
+module_init(alauda_driver_init);
+
+static void __exit alauda_driver_exit(void)
+{
+	usb_deregister(&alauda_driver);
+}
+module_exit(alauda_driver_exit);
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 0e400f3..05b8fe2 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -540,7 +540,7 @@  static struct device_attribute *sysfs_device_attr_list[] = {
  * this defines our host template, with which we'll allocate hosts
  */
 
-struct scsi_host_template usb_stor_host_template = {
+static struct scsi_host_template usb_stor_host_template = {
 	/* basic userland interface stuff */
 	.name =				"usb-storage",
 	.proc_name =			"usb-storage",
@@ -592,6 +592,16 @@  struct scsi_host_template usb_stor_host_template = {
 	.module =			THIS_MODULE
 };
 
+void usb_stor_host_template_init(struct scsi_host_template *sht,
+				 const char *name, struct module *owner)
+{
+	memcpy(sht, &usb_stor_host_template, sizeof(*sht));
+	sht->name = name;
+	sht->proc_name = name;
+	sht->module = owner;
+}
+EXPORT_SYMBOL_GPL(usb_stor_host_template_init);
+
 /* To Report "Illegal Request: Invalid Field in CDB */
 unsigned char usb_stor_sense_invalidCDB[18] = {
 	[0]	= 0x70,			    /* current error */
diff --git a/drivers/usb/storage/scsiglue.h b/drivers/usb/storage/scsiglue.h
index ffa1cca..1909601 100644
--- a/drivers/usb/storage/scsiglue.h
+++ b/drivers/usb/storage/scsiglue.h
@@ -43,6 +43,7 @@  extern void usb_stor_report_device_reset(struct us_data *us);
 extern void usb_stor_report_bus_reset(struct us_data *us);
 
 extern unsigned char usb_stor_sense_invalidCDB[18];
-extern struct scsi_host_template usb_stor_host_template;
+extern void usb_stor_host_template_init(struct scsi_host_template *sht,
+					const char *name, struct module *owner);
 
 #endif
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 6c10c88..0f76899 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -924,7 +924,8 @@  static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf)
 int usb_stor_probe1(struct us_data **pus,
 		struct usb_interface *intf,
 		const struct usb_device_id *id,
-		struct us_unusual_dev *unusual_dev)
+		struct us_unusual_dev *unusual_dev,
+		struct scsi_host_template *sht)
 {
 	struct Scsi_Host *host;
 	struct us_data *us;
@@ -936,7 +937,7 @@  int usb_stor_probe1(struct us_data **pus,
 	 * Ask the SCSI layer to allocate a host structure, with extra
 	 * space at the end for our private us_data structure.
 	 */
-	host = scsi_host_alloc(&usb_stor_host_template, sizeof(*us));
+	host = scsi_host_alloc(sht, sizeof(*us));
 	if (!host) {
 		dev_warn(&intf->dev, "Unable to allocate the scsi host\n");
 		return -ENOMEM;
@@ -1073,6 +1074,8 @@  void usb_stor_disconnect(struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usb_stor_disconnect);
 
+static struct scsi_host_template usb_stor_host_template;
+
 /* The main probe routine for standard devices */
 static int storage_probe(struct usb_interface *intf,
 			 const struct usb_device_id *id)
@@ -1113,7 +1116,8 @@  static int storage_probe(struct usb_interface *intf,
 			id->idVendor, id->idProduct);
 	}
 
-	result = usb_stor_probe1(&us, intf, id, unusual_dev);
+	result = usb_stor_probe1(&us, intf, id, unusual_dev,
+				 &usb_stor_host_template);
 	if (result)
 		return result;
 
@@ -1137,4 +1141,17 @@  static struct usb_driver usb_storage_driver = {
 	.soft_unbind =	1,
 };
 
-module_usb_driver(usb_storage_driver);
+static int __init usb_storage_driver_init(void)
+{
+	usb_stor_host_template_init(&usb_stor_host_template, "usb-storage",
+				    THIS_MODULE);
+
+	return usb_register(&usb_storage_driver);
+}
+module_init(usb_storage_driver_init);
+
+static void __exit usb_storage_driver_exit(void)
+{
+	usb_deregister(&usb_storage_driver);
+}
+module_exit(usb_storage_driver_exit);
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 307e339..a00ec4f 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -197,7 +197,8 @@  extern int usb_stor_post_reset(struct usb_interface *iface);
 extern int usb_stor_probe1(struct us_data **pus,
 		struct usb_interface *intf,
 		const struct usb_device_id *id,
-		struct us_unusual_dev *unusual_dev);
+		struct us_unusual_dev *unusual_dev,
+		struct scsi_host_template *sht);
 extern int usb_stor_probe2(struct us_data *us);
 extern void usb_stor_disconnect(struct usb_interface *intf);
 
-- 
1.9.1