scsi: ufs & ums-* & esp_scsi: fix module reference counting
Subsystems (4)
v3 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.
* 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(-)
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
A message that is not in the archive (sent off-list or not yet received): 1420984206-22341-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
PATCH v3 1/4 scsi: add ability to adjust module reference for scsi host
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.
---
No change from v2
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
@@ -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
@@ -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 */
include/scsi/scsi_host.h +1 -0
@@ -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;
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
A message that is not in the archive (sent off-list or not yet received): 1420984206-22341-2-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Hannes Reinecke
Reviewed-byRe: [PATCH v3 1/4] scsi: add ability to adjust module reference for scsi host
Quoted 14 lines · 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 structscsi_host_template.This mapping between
scsi_deviceand underlying LLDD module works well except ufs, unusual usb storage drivers, and sub drivers foresp_scsi. These drivers consist with core driver and actual LLDDs, andscsi_host_templateis defined in the core driver. So the actual LLDDs can be unloaded even if thescsi_deviceis being accessed.This adds .module field in struct
Scsi_Hostand let the module reference be retrieved though it instead of structscsi_host_template. This allows the actual LLDDs adjust module reference.
Quoted 13 lines
Signed-off-by Akinobu Mita <[email protected]>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 <stern-nwvwT67g6+6dFdvTe/[email protected]>
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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
PATCH v3 2/4 scsi: ufs: adjust module reference for scsi host
In reply to Akinobu MitaWhile 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 v2
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
@@ -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
@@ -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
@@ -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,
Signature · 1 line
1.9.1
PATCH v3 3/4 usb: storage: adjust module reference for scsi host
In reply to Akinobu MitaWhile 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.
---
No change from v2
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
@@ -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
@@ -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 · 1 line
1.9.1
PATCH v3 4/4 scsi: esp_scsi: adjust module reference for scsi host
In reply to Akinobu MitaWhile 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.
---
New patch 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
@@ -406,7 +406,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;
@@ -423,7 +422,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
@@ -2677,8 +2677,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,
@@ -2698,7 +2697,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
Re: [PATCH v3 4/4] scsi: esp_scsi: adjust module reference for scsi host
From: Akinobu Mita <[email protected]>
Date: Sun, 11 Jan 2015 22:50:06 +0900
Quoted 11 lines
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 withscsi_esp_templatedefined 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 structScsi_Hostcan be adjusted.Signed-off-by Akinobu Mita <[email protected]>
Looks fine to me, thanks for fixing this:
Re: [PATCH v3 4/4] scsi: esp_scsi: adjust module reference for scsi host
In reply to Akinobu MitaQuoted 10 lines · 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 withscsi_esp_templatedefined 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 structScsi_Hostcan be adjusted.
Quoted 9 lines
Signed-off-by Akinobu Mita <[email protected]>Cc "David S. Miller" <[email protected]>Cc Hannes Reinecke <[email protected]>Cc Christoph Hellwig <[email protected]>Cc "James E.J. Bottomley" <[email protected]>---
New patch from v3
Cheers,
Hannes
Signature · 9 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 10 lines · 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 structscsi_host_template.This mapping between
scsi_deviceand underlying LLDD module works well except ufs, unusual usb storage drivers, and sub drivers foresp_scsi. These drivers consist with core driver and actual LLDDs, andscsi_host_templateis defined in the core driver. So the actual LLDDs can be unloaded even if thescsi_deviceis being accessed.
Quoted 3 lines
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.
Why don't we move the module into the Scsi_Host only, and use the same macro that passes THIS_MODULE trick you are using in the sub drivers? That seems to be a fairly common scheme in other subsystems as well.
A message that is not in the archive (sent off-list or not yet received): [email protected]
2015-01-12 18:36 GMT+09:00 Christoph Hellwig <[email protected]>:
Quoted 11 lines · Akinobu Mita
Quoted 10 lines
While accessing a
scsi_device, the use count of the underlying LLDD module is incremented. The module reference is retrieved through .module field of structscsi_host_template.This mapping between
scsi_deviceand underlying LLDD module works well except ufs, unusual usb storage drivers, and sub drivers foresp_scsi. These drivers consist with core driver and actual LLDDs, andscsi_host_templateis defined in the core driver. So the actual LLDDs can be unloaded even if thescsi_deviceis being accessed.
Quoted 8 lines
Quoted 3 lines
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.
Why don't we move the module into the
Scsi_Hostonly, and use the same macro that passesTHIS_MODULEtrick you are using in the sub drivers? That seems to be a fairly common scheme in other subsystems as well.
Sure, we can take this approach. But ata drivers require more changes for it because ata drivers don't call scsi_host_alloc() directly so we need to pass THIS_MODULE to a variety of init functions() provided by libata.
While looking through ata drivers, I found that libahci_platform sub-drivers and pata_of_platform driver also have module reference mismatch problem. So ata drivers need to be touched anyway. I'll update this patch series with the new approach.
--
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