Skip to contentSkip to conversation

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

PATCH Superseded Akinobu Mita 2 participants 5 messages Feb 28, 2015 1 message not in the archive

Subsystems (4)

v5 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v5 cover scsi: ufs & ums-* & esp_scsi: 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 ufs, unusual usb storage drivers, and sub drivers for esp_scsi. 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 actual LLDDs by adjusting module reference after scsi host allocation.

* v5:
- Discard v4 changes and restore to v3. Because v4 shows that moving owner module field from scsi_host_template to Scsi_Host requires a lot of changes and introduces complication to existing drivers which don't have the module reference mismatch issue.
- Rebased to the 4.0-rc1

* v4:
- Patch series is almost rewritten as module reference field in struct scsi_host_template has been unused anymore. So Acked-by: and
Reviewed-by: tags that have been received are deleted.

* v3:
- Add fix for ESP SCSI drivers

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

Akinobu Mita (4):
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
scsi: esp_scsi: adjust module reference for scsi host

 drivers/scsi/am53c974.c          |  3 +--
 drivers/scsi/esp_scsi.c          | 16 +++++++++++++---
 drivers/scsi/esp_scsi.h          | 11 +++++++----
 drivers/scsi/hosts.c             |  1 +
 drivers/scsi/jazz_esp.c          |  3 +--
 drivers/scsi/mac_esp.c           |  3 +--
 drivers/scsi/scsi.c              |  4 ++--
 drivers/scsi/sun3x_esp.c         |  3 +--
 drivers/scsi/sun_esp.c           |  3 +--
 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 +
 15 files changed, 41 insertions(+), 25 deletions(-)
Cc Vinayak Holikatti <[email protected]>
Cc Dolev Raviv <[email protected]>
Cc Sujit Reddy Thumma <[email protected]>
Cc Subhash Jadavani <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Matthew Dharm <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Alan Stern <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Hannes Reinecke <[email protected]>
Signature · 1 line
1.9.1

A message that is not in the archive (sent off-list or not yet received): 1425133017-7216-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#2 lore raw

PATCH v5 3/4 usb: storage: adjust module reference for scsi host

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() so that .module field in struct Scsi_Host can be adjusted.

Cc Matthew Dharm <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Alan Stern <stern-nwvwT67g6+6dFdvTe/[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>

---
No change from v3

 drivers/usb/storage/usb.c | 8 +++++---
 drivers/usb/storage/usb.h | 7 +++++--
 2 files changed, 10 insertions(+), 5 deletions(-)
drivers/usb/storage/usb.c +5 -3
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)
drivers/usb/storage/usb.h +5 -2
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);
 
Signature · 6 lines
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
#3 lore raw

PATCH v5 1/4 scsi: add ability to adjust module reference for scsi host

In reply to Akinobu Mita

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 ufs, unusual usb storage drivers, and sub drivers for esp_scsi. 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.

Cc Vinayak Holikatti <[email protected]>
Cc Dolev Raviv <[email protected]>
Cc Sujit Reddy Thumma <[email protected]>
Cc Subhash Jadavani <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Matthew Dharm <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc Alan Stern <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Hannes Reinecke <[email protected]>

---
Change from v3
- Rebased to the 4.0-rc1

 drivers/scsi/hosts.c     | 1 +
 drivers/scsi/scsi.c      | 4 ++--
 include/scsi/scsi_host.h | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
drivers/scsi/hosts.c +1 -0
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;
drivers/scsi/scsi.c +2 -2
index c9c3b57..0d89344 100644
@@ -981,7 +981,7 @@ int scsi_device_get(struct scsi_device *sdev)
 		return -ENXIO;
 	/* We can fail try_module_get if we're doing SCSI operations
 	 * from module exit (like cache flush) */
-	__module_get(sdev->host->hostt->module);
+	__module_get(sdev->host->module);
 
 	return 0;
 }
@@ -997,7 +997,7 @@ EXPORT_SYMBOL(scsi_device_get);
  */
 void scsi_device_put(struct scsi_device *sdev)
 {
-	module_put(sdev->host->hostt->module);
+	module_put(sdev->host->module);
 	put_device(&sdev->sdev_gendev);
 }
 EXPORT_SYMBOL(scsi_device_put);
include/scsi/scsi_host.h +1 -0
index e113c75..8742bfd 100644
@@ -620,6 +620,7 @@ struct Scsi_Host {
 	 */
 	unsigned short max_cmd_len;
 
+	struct module *module;
 	int this_id;
 	int can_queue;
 	short cmd_per_lun;
Signature · 1 line
1.9.1
#4 lore raw

PATCH v5 2/4 scsi: ufs: adjust module reference for scsi host

In reply to Akinobu Mita

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.

Cc Vinayak Holikatti <[email protected]>
Cc Dolev Raviv <[email protected]>
Cc Sujit Reddy Thumma <[email protected]>
Cc Subhash Jadavani <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>

---
No change from v3

 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(-)
drivers/scsi/ufs/ufshcd-pci.c +1 -0
index 7062cc5..d98f072 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);
drivers/scsi/ufs/ufshcd-pltfrm.c +1 -0
index f9d0644..bd76a0c 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);
drivers/scsi/ufs/ufshcd.c +0 -1
index 8830aeb..87c4847 100644
@@ -4200,7 +4200,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,
Signature · 1 line
1.9.1
#5 lore raw

PATCH v5 4/4 scsi: esp_scsi: adjust module reference for scsi host

In reply to Akinobu Mita

While accessing a scsi device on host adapter supported by sub driver for the ESP chip (mac_esp, am53c974, sun_esp, jazz_esp, sun3x_esp), the module reference count is not incremented. Because these drivers allocate scsi hosts with scsi_esp_template defined in ESP SCSI driver core module. So these drivers always can be unloaded.

This fixes it by passing correct module reference to newly introduced scsi_esp_host_alloc() so that .module field in struct Scsi_Host can be adjusted.

Cc "David S. Miller" <[email protected]>
Cc Hannes Reinecke <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>

---
No change from v3

 drivers/scsi/am53c974.c  |  3 +--
 drivers/scsi/esp_scsi.c  | 16 +++++++++++++---
 drivers/scsi/esp_scsi.h  | 11 +++++++----
 drivers/scsi/jazz_esp.c  |  3 +--
 drivers/scsi/mac_esp.c   |  3 +--
 drivers/scsi/sun3x_esp.c |  3 +--
 drivers/scsi/sun_esp.c   |  3 +--
 7 files changed, 25 insertions(+), 17 deletions(-)
drivers/scsi/am53c974.c +1 -2
index a6f5ee8..3b53f01 100644
@@ -400,7 +400,6 @@ static void dc390_check_eeprom(struct esp *esp)
 static int pci_esp_probe_one(struct pci_dev *pdev,
 			      const struct pci_device_id *id)
 {
-	struct scsi_host_template *hostt = &scsi_esp_template;
 	int err = -ENODEV;
 	struct Scsi_Host *shost;
 	struct esp *esp;
@@ -417,7 +416,7 @@ static int pci_esp_probe_one(struct pci_dev *pdev,
 		goto fail_disable_device;
 	}
 
-	shost = scsi_host_alloc(hostt, sizeof(struct esp));
+	shost = scsi_esp_host_alloc(sizeof(struct esp));
 	if (!shost) {
 		dev_printk(KERN_INFO, &pdev->dev,
 			   "failed to allocate scsi host\n");
drivers/scsi/esp_scsi.c +13 -3
index 065b25d..4484c90 100644
@@ -2675,8 +2675,7 @@ static const char *esp_info(struct Scsi_Host *host)
 	return "esp";
 }
 
-struct scsi_host_template scsi_esp_template = {
-	.module			= THIS_MODULE,
+static struct scsi_host_template scsi_esp_template = {
 	.name			= "esp",
 	.info			= esp_info,
 	.queuecommand		= esp_queuecommand,
@@ -2696,7 +2695,18 @@ struct scsi_host_template scsi_esp_template = {
 	.skip_settle_delay	= 1,
 	.use_blk_tags		= 1,
 };
-EXPORT_SYMBOL(scsi_esp_template);
+
+struct Scsi_Host *__scsi_esp_host_alloc(int privsize, struct module *owner)
+{
+	struct Scsi_Host *shost;
+
+	shost = scsi_host_alloc(&scsi_esp_template, privsize);
+	if (shost)
+		shost->module = owner;
+
+	return shost;
+}
+EXPORT_SYMBOL(__scsi_esp_host_alloc);
 
 static void esp_get_signalling(struct Scsi_Host *host)
 {
drivers/scsi/esp_scsi.h +7 -4
index 84dcbe4..9309e08 100644
@@ -544,9 +544,8 @@ struct esp {
 
 /* A front-end driver for the ESP chip should do the following in
  * it's device probe routine:
- * 1) Allocate the host and private area using scsi_host_alloc()
- *    with size 'sizeof(struct esp)'.  The first argument to
- *    scsi_host_alloc() should be &scsi_esp_template.
+ * 1) Allocate the host and private area using scsi_esp_host_alloc()
+ *    with size 'sizeof(struct esp)'.
  * 2) Set host->max_id as appropriate.
  * 3) Set esp->host to the scsi_host itself, and esp->dev
  *    to the device object pointer.
@@ -573,7 +572,11 @@ struct esp {
  * 13) Check scsi_esp_register() return value, release all resources
  *     if an error was returned.
  */
-extern struct scsi_host_template scsi_esp_template;
+extern struct Scsi_Host *__scsi_esp_host_alloc(int privsize,
+						struct module *owner);
+#define scsi_esp_host_alloc(privsize)	\
+	__scsi_esp_host_alloc(privsize, THIS_MODULE)
+
 extern int scsi_esp_register(struct esp *, struct device *);
 
 extern void scsi_esp_unregister(struct esp *);
drivers/scsi/jazz_esp.c +1 -2
index 9aaa74e..9c268f7 100644
@@ -131,13 +131,12 @@ static const struct esp_driver_ops jazz_esp_ops = {
 
 static int esp_jazz_probe(struct platform_device *dev)
 {
-	struct scsi_host_template *tpnt = &scsi_esp_template;
 	struct Scsi_Host *host;
 	struct esp *esp;
 	struct resource *res;
 	int err;
 
-	host = scsi_host_alloc(tpnt, sizeof(struct esp));
+	host = scsi_esp_host_alloc(sizeof(struct esp));
 
 	err = -ENOMEM;
 	if (!host)
drivers/scsi/mac_esp.c +1 -2
index 14c0334..ad4ad71 100644
@@ -483,7 +483,6 @@ static struct esp_driver_ops mac_esp_ops = {
 
 static int esp_mac_probe(struct platform_device *dev)
 {
-	struct scsi_host_template *tpnt = &scsi_esp_template;
 	struct Scsi_Host *host;
 	struct esp *esp;
 	int err;
@@ -495,7 +494,7 @@ static int esp_mac_probe(struct platform_device *dev)
 	if (dev->id > 1)
 		return -ENODEV;
 
-	host = scsi_host_alloc(tpnt, sizeof(struct esp));
+	host = scsi_esp_host_alloc(sizeof(struct esp));
 
 	err = -ENOMEM;
 	if (!host)
drivers/scsi/sun3x_esp.c +1 -2
index e26e81d..adb4368 100644
@@ -196,13 +196,12 @@ static const struct esp_driver_ops sun3x_esp_ops = {
 
 static int esp_sun3x_probe(struct platform_device *dev)
 {
-	struct scsi_host_template *tpnt = &scsi_esp_template;
 	struct Scsi_Host *host;
 	struct esp *esp;
 	struct resource *res;
 	int err = -ENOMEM;
 
-	host = scsi_host_alloc(tpnt, sizeof(struct esp));
+	host = scsi_esp_host_alloc(sizeof(struct esp));
 	if (!host)
 		goto fail;
 
drivers/scsi/sun_esp.c +1 -2
index 7b6d4c2..1449bea 100644
@@ -489,12 +489,11 @@ static const struct esp_driver_ops sbus_esp_ops = {
 static int esp_sbus_probe_one(struct platform_device *op,
 			      struct platform_device *espdma, int hme)
 {
-	struct scsi_host_template *tpnt = &scsi_esp_template;
 	struct Scsi_Host *host;
 	struct esp *esp;
 	int err;
 
-	host = scsi_host_alloc(tpnt, sizeof(struct esp));
+	host = scsi_esp_host_alloc(sizeof(struct esp));
 
 	err = -ENOMEM;
 	if (!host)
Signature · 1 line
1.9.1