scsi: ufs & ums-*: fix module reference counting

While accessing a scsi_device, the use count of the underlying LLDD module is incremented. The module reference is retrieved through .module field of struct scsi_host_template.

This mapping between scsi_device and underlying LLDD module works well except some scsi drivers (ufs and unusual usb storage drivers). These drivers consist with core driver and actual LLDDs, and scsi_host_template is defined in the core driver. So the actual LLDDs can be unloaded even if the scsi_device is being accessed.

This patch series first adds ability to adjust module reference for scsi host by LLDDs and then fixes ufs and unusual usb storage drivers by adjusting module reference after scsi host allocation.

* v2:
- Pass correct module reference to usb_stor_probe1() instead of touching all ums-* drivers, suggested by Alan Stern

Akinobu Mita (3):
scsi: add ability to adjust module reference for scsi host
scsi: ufs: adjust module reference for scsi host
usb: storage: adjust module reference for scsi host

 drivers/scsi/hosts.c             | 1 +
 drivers/scsi/scsi.c              | 4 ++--
 drivers/scsi/ufs/ufshcd-pci.c    | 1 +
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 +
 drivers/scsi/ufs/ufshcd.c        | 1 -
 drivers/usb/storage/usb.c        | 8 +++++---
 drivers/usb/storage/usb.h        | 7 +++++--
 include/scsi/scsi_host.h         | 1 +
 8 files changed, 16 insertions(+), 8 deletions(-)

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

While accessing a scsi_device, the use count of the underlying LLDD module is incremented. The module reference is retrieved through .module field of struct scsi_host_template.

This mapping between scsi_device and underlying LLDD module works well except some scsi drivers (ufs and unusual usb storage drivers). These drivers consist with core driver and actual LLDDs, and scsi_host_template is defined in the core driver. So the actual LLDDs can be unloaded even if the scsi_device is being accessed.

This adds .module field in struct Scsi_Host and let the module reference be retrieved though it instead of struct scsi_host_template. This allows the actual LLDDs adjust module reference.

---
No change from v1

 drivers/scsi/hosts.c     | 1 +
 drivers/scsi/scsi.c      | 4 ++--
 include/scsi/scsi_host.h | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

index 8bb173e..21f1442 100644
@@ -411,6 +411,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	 */
 	shost->max_cmd_len = 12;
 	shost->hostt = sht;
+	shost->module = sht->module;
 	shost->this_id = sht->this_id;
 	shost->can_queue = sht->can_queue;
 	shost->sg_tablesize = sht->sg_tablesize;

index e028854..5905b83 100644
@@ -988,7 +988,7 @@ int scsi_device_get(struct scsi_device *sdev)
 		return -ENXIO;
 	/* We can fail this if we're doing SCSI operations
 	 * from module exit (like cache flush) */
-	try_module_get(sdev->host->hostt->module);
+	try_module_get(sdev->host->module);
 
 	return 0;
 }

@@ -1005,7 +1005,7 @@ EXPORT_SYMBOL(scsi_device_get);
 void scsi_device_put(struct scsi_device *sdev)
 {
 #ifdef CONFIG_MODULE_UNLOAD
-	struct module *module = sdev->host->hostt->module;
+	struct module *module = sdev->host->module;
 
 	/* The module refcount will be zero if scsi_device_get()
 	 * was called from a module removal routine */

index 019e668..5133f2f 100644
@@ -617,6 +617,7 @@ struct Scsi_Host {
 	 */
 	unsigned short max_cmd_len;
 
+	struct module *module;
 	int this_id;
 	int can_queue;
 	short cmd_per_lun;

1.9.1

While accessing a UFS device, the module reference count for core driver (ufshcd) is incremented but not incremented for the actual glue driver (ufshcd-pci or ufshcd-pltfrm). Because these drivers allocate scsi hosts with scsi_host_template defined in ufshcd module. So these drivers always can be unloaded.

This fixes it by adjusting module reference after scsi host allocation.

---
No change from v1

 drivers/scsi/ufs/ufshcd-pci.c    | 1 +
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 +
 drivers/scsi/ufs/ufshcd.c        | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

index d15eaa4..9fe21b4 100644
@@ -142,6 +142,7 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return err;
 	}
 
+	hba->host->module = THIS_MODULE;
 	INIT_LIST_HEAD(&hba->clk_list_head);
 
 	err = ufshcd_init(hba, mmio_base, pdev->irq);

index 95b64e0..ea3ca99 100644
@@ -322,6 +322,7 @@ static int ufshcd_pltfrm_probe(struct platform_device *pdev)
 		goto out;
 	}
 
+	hba->host->module = THIS_MODULE;
 	hba->vops = get_variant_ops(&pdev->dev);
 
 	err = ufshcd_parse_clock_info(hba);

index 2e26025..f0aff90 100644
@@ -4202,7 +4202,6 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 }
 
 static struct scsi_host_template ufshcd_driver_template = {
-	.module			= THIS_MODULE,
 	.name			= UFSHCD,
 	.proc_name		= UFSHCD,
 	.queuecommand		= ufshcd_queuecommand,

1.9.1

While accessing a unusual usb storage (ums-alauda, ums-cypress, ...), the module reference count is not incremented. Because these drivers allocate scsi hosts with usb_stor_host_template defined in usb-storage module. So these drivers always can be unloaded.

This fixes it by passing correct module reference to usb_stor_probe1() to adjust it after scsi host allocation.

---
* v2:
- Pass correct module reference to usb_stor_probe1() instead of touching all ums-* drivers, suggested by Alan Stern

 drivers/usb/storage/usb.c | 8 +++++---
 drivers/usb/storage/usb.h | 7 +++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

index d468d02..3bb2558 100644
@@ -911,10 +911,11 @@ static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf)
 }
 
 /* First part of general USB mass-storage probing */
-int usb_stor_probe1(struct us_data **pus,
+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 module *owner)
 {
 	struct Scsi_Host *host;
 	struct us_data *us;

@@ -937,6 +938,7 @@ int usb_stor_probe1(struct us_data **pus,
 	 */
 	host->max_cmd_len = 16;
 	host->sg_tablesize = usb_stor_sg_tablesize(intf);
+	host->module = owner;
 	*pus = us = host_to_us(host);
 	mutex_init(&(us->dev_mutex));
 	us_set_lock_class(&us->dev_mutex, intf);

@@ -969,7 +971,7 @@ BadDevice:
 	release_everything(us);
 	return result;
 }
-EXPORT_SYMBOL_GPL(usb_stor_probe1);
+EXPORT_SYMBOL_GPL(__usb_stor_probe1);
 
 /* Second part of general USB mass-storage probing */
 int usb_stor_probe2(struct us_data *us)

index 307e339..0cb74ba 100644
@@ -194,10 +194,13 @@ extern int usb_stor_reset_resume(struct usb_interface *iface);
 extern int usb_stor_pre_reset(struct usb_interface *iface);
 extern int usb_stor_post_reset(struct usb_interface *iface);
 
-extern int usb_stor_probe1(struct us_data **pus,
+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 module *owner);
+#define usb_stor_probe1(pus, intf, id, unusual_dev) \
+	__usb_stor_probe1(pus, intf, id, unusual_dev, THIS_MODULE)
 extern int usb_stor_probe2(struct us_data *us);
 extern void usb_stor_disconnect(struct usb_interface *intf);
 

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Why don't ufs and usb-storage define the host templates in the sub drivers? That's what libata or the mpt fusion driver do.

Originally the subdrivers were all part of usb-storage. When they were split out into separate modules, there didn't seem to be any need to copy the host template into each one. (Especially since some of the method pointers in the host template refer to routines in usb-storage itself, not in the subdrivers.) The fact that the .module member was no longer adequate escaped our notice at the time.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-01-08 0:09 GMT+09:00 Alan Stern <[email protected]>:

For ufs, the reason why a host template is shared by sub drivers is similar to usb-storage. Originally there was only a single pci driver for ufs. When platform driver for ufs was introduced, it was split into core driver (ufshcd) and sub drivers (ufshcd-pci and ufshcd-pltfrm). They require exactly the same host template.

Is there any other preferable way to fix this problem? The first time I found this in ufs driver, I was trying to fix it by merging core and sub drivers into a single compound module. But I realized that usm-* drivers have the same problem, then I decided to propose what this patch series do instead.

I can't think of any other ways to fix it. The only options seem to be to duplicate the host template in each subdriver or to add the .module field to the host structure. I prefer the second option because it doesn't involve so much code duplication.

Alan Stern