All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
@ 2015-01-18 15:05 Akinobu Mita
  2015-01-18 15:05 ` [PATCH v4 01/11] ata: prepare to move module reference from scsi_host_template to Scsi_Host Akinobu Mita
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:05 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Vinayak Holikatti, Dolev Raviv, Sujit Reddy Thumma,
	Subhash Jadavani, Matthew Dharm, Greg Kroah-Hartman, Alan Stern,
	David S. Miller, Hannes Reinecke, Tejun Heo, Hans de Goede,
	Mike Christie, Karen Xie, Robert Love, Christoph Hellwig,
	James E.J. Bottomley, open-iscsi, fcoe-devel, linux-ide,
	linux-usb, usb-storage

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 drivers which consist with the core driver and the actual
LLDDs and scsi_host_template is defined in the core driver.  In these
cases, the actual LLDDs can be unloaded even if the scsi_device is
being accessed.

This patch series fixes the module reference mismatch problem for
ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
by moving owner module reference field from struct scsi_host_template
to struct Scsi_Host and allowing the LLDDs to set their correct module
reference.

* 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 (11):
  ata: prepare to move module reference from scsi_host_template to
    Scsi_Host
  iscsi: prepare to move module reference from scsi_host_template to
    Scsi_Host
  cxgbi: prepare to move module reference from scsi_host_template to
    Scsi_Host
  libfc: prepare to move module reference from scsi_host_template to
    Scsi_Host
  53c700: prepare move module reference from scsi_host_template to
    Scsi_Host
  scsi: legacy: prepare to move module reference from scsi_host_template
    to Scsi_Host
  scsi: move module reference from scsi_host_template to Scsi_Host
  scsi: ufs: adjust module reference for scsi host
  usb: storage: adjust module reference for scsi host
  ata: ahci_platform: adjust module reference for scsi host
  ata: pata_of_platform: adjust module reference for scsi host

 drivers/ata/libahci_platform.c | 14 +++++-----
 drivers/ata/libata-core.c      | 22 ++++++++-------
 drivers/ata/libata-scsi.c      |  2 +-
 drivers/ata/libata-sff.c       | 61 +++++++++++++++++++++++-------------------
 drivers/ata/pata_platform.c    | 18 +++++++------
 drivers/scsi/53c700.c          |  9 ++++---
 drivers/scsi/53c700.h          |  7 +++--
 drivers/scsi/cxgbi/libcxgbi.c  |  8 +++---
 drivers/scsi/cxgbi/libcxgbi.h  |  6 +++--
 drivers/scsi/hosts.c           | 16 ++++++-----
 drivers/scsi/libfc/fc_lport.c  | 26 ++++++++++++++++++
 drivers/scsi/libfc/fc_npiv.c   |  2 +-
 drivers/scsi/libiscsi.c        | 12 +++++----
 drivers/scsi/scsi.c            |  4 +--
 drivers/scsi/ufs/ufshcd.c      | 13 ++++-----
 drivers/scsi/ufs/ufshcd.h      |  5 +++-
 drivers/usb/storage/scsiglue.c |  3 ---
 drivers/usb/storage/usb.c      |  9 ++++---
 drivers/usb/storage/usb.h      |  7 +++--
 include/linux/ahci_platform.h  |  9 ++++---
 include/linux/ata_platform.h   | 16 ++++++-----
 include/linux/libata.h         | 52 ++++++++++++++++++++++++-----------
 include/scsi/libfc.h           | 27 +++----------------
 include/scsi/libiscsi.h        |  9 ++++---
 include/scsi/scsi_host.h       | 13 +++++++--
 25 files changed, 224 insertions(+), 146 deletions(-)

Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: Dolev Raviv <draviv@codeaurora.org>
Cc: Sujit Reddy Thumma <sthumma@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Karen Xie <kxie@chelsio.com>
Cc: Robert Love <robert.w.love@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: open-iscsi@googlegroups.com
Cc: fcoe-devel@open-fcoe.org
Cc: linux-ide@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: usb-storage@lists.one-eyed-alien.net
Cc: linux-scsi@vger.kernel.org
-- 
1.9.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v4 01/11] ata: prepare to move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
@ 2015-01-18 15:05 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 02/11] iscsi: " Akinobu Mita
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:05 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Hans de Goede, Tejun Heo, Christoph Hellwig,
	James E.J. Bottomley, linux-ide

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this converts the following
functions into macros so that LLDDs can pass THIS_MODULE to
scsi_host_alloc() through them instead of scsi_host_template->module.

ata_host_alloc()
ata_host_alloc_pinfo()
ata_pci_sff_prepare_host()
ata_pci_sff_init_one()
ata_pci_bmdma_prepare_host()
ata_pci_bmdma_init_one()

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-ide@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/ata/libata-core.c | 22 ++++++++++-------
 drivers/ata/libata-sff.c  | 61 +++++++++++++++++++++++++----------------------
 include/linux/libata.h    | 52 ++++++++++++++++++++++++++++------------
 3 files changed, 83 insertions(+), 52 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5c84fb5..a3cc0e3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5700,9 +5700,10 @@ static void ata_host_release(struct device *gendev, void *res)
 }
 
 /**
- *	ata_host_alloc - allocate and init basic ATA host resources
+ *	__ata_host_alloc - allocate and init basic ATA host resources
  *	@dev: generic device this host is associated with
  *	@max_ports: maximum number of ATA ports associated with this host
+ *	@owner: module which will be the owner of the ATA host
  *
  *	Allocate and initialize basic ATA host resources.  LLD calls
  *	this function to allocate a host, initializes it fully and
@@ -5719,7 +5720,8 @@ static void ata_host_release(struct device *gendev, void *res)
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  */
-struct ata_host *ata_host_alloc(struct device *dev, int max_ports)
+struct ata_host *__ata_host_alloc(struct device *dev, int max_ports,
+				  struct module *owner)
 {
 	struct ata_host *host;
 	size_t sz;
@@ -5744,6 +5746,7 @@ struct ata_host *ata_host_alloc(struct device *dev, int max_ports)
 	mutex_init(&host->eh_mutex);
 	host->dev = dev;
 	host->n_ports = max_ports;
+	host->module = owner;
 
 	/* allocate ports bound to this host */
 	for (i = 0; i < max_ports; i++) {
@@ -5766,10 +5769,11 @@ struct ata_host *ata_host_alloc(struct device *dev, int max_ports)
 }
 
 /**
- *	ata_host_alloc_pinfo - alloc host and init with port_info array
+ *	__ata_host_alloc_pinfo - alloc host and init with port_info array
  *	@dev: generic device this host is associated with
  *	@ppi: array of ATA port_info to initialize host with
  *	@n_ports: number of ATA ports attached to this host
+ *	@owner: module which will be the owner of the ATA host
  *
  *	Allocate ATA host and initialize with info from @ppi.  If NULL
  *	terminated, @ppi may contain fewer entries than @n_ports.  The
@@ -5781,15 +5785,15 @@ struct ata_host *ata_host_alloc(struct device *dev, int max_ports)
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  */
-struct ata_host *ata_host_alloc_pinfo(struct device *dev,
-				      const struct ata_port_info * const * ppi,
-				      int n_ports)
+struct ata_host *__ata_host_alloc_pinfo(struct device *dev,
+					const struct ata_port_info * const *ppi,
+					int n_ports, struct module *owner)
 {
 	const struct ata_port_info *pi;
 	struct ata_host *host;
 	int i, j;
 
-	host = ata_host_alloc(dev, n_ports);
+	host = __ata_host_alloc(dev, n_ports, owner);
 	if (!host)
 		return NULL;
 
@@ -6862,8 +6866,8 @@ EXPORT_SYMBOL_GPL(ata_dev_next);
 EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_scsi_unlock_native_capacity);
 EXPORT_SYMBOL_GPL(ata_host_init);
-EXPORT_SYMBOL_GPL(ata_host_alloc);
-EXPORT_SYMBOL_GPL(ata_host_alloc_pinfo);
+EXPORT_SYMBOL_GPL(__ata_host_alloc);
+EXPORT_SYMBOL_GPL(__ata_host_alloc_pinfo);
 EXPORT_SYMBOL_GPL(ata_slave_link_init);
 EXPORT_SYMBOL_GPL(ata_host_start);
 EXPORT_SYMBOL_GPL(ata_host_register);
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index db90aa3..323c9d8 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2349,10 +2349,11 @@ int ata_pci_sff_init_host(struct ata_host *host)
 EXPORT_SYMBOL_GPL(ata_pci_sff_init_host);
 
 /**
- *	ata_pci_sff_prepare_host - helper to prepare PCI PIO-only SFF ATA host
+ *	__ata_pci_sff_prepare_host - helper to prepare PCI PIO-only SFF ATA host
  *	@pdev: target PCI device
  *	@ppi: array of port_info, must be enough for two ports
  *	@r_host: out argument for the initialized ATA host
+ *	@owner: module which will be the owner of the ATA host
  *
  *	Helper to allocate PIO-only SFF ATA host for @pdev, acquire
  *	all PCI resources and initialize it accordingly in one go.
@@ -2363,9 +2364,9 @@ EXPORT_SYMBOL_GPL(ata_pci_sff_init_host);
  *	RETURNS:
  *	0 on success, -errno otherwise.
  */
-int ata_pci_sff_prepare_host(struct pci_dev *pdev,
-			     const struct ata_port_info * const *ppi,
-			     struct ata_host **r_host)
+int __ata_pci_sff_prepare_host(struct pci_dev *pdev,
+			       const struct ata_port_info * const *ppi,
+			       struct ata_host **r_host, struct module *owner)
 {
 	struct ata_host *host;
 	int rc;
@@ -2373,7 +2374,7 @@ int ata_pci_sff_prepare_host(struct pci_dev *pdev,
 	if (!devres_open_group(&pdev->dev, NULL, GFP_KERNEL))
 		return -ENOMEM;
 
-	host = ata_host_alloc_pinfo(&pdev->dev, ppi, 2);
+	host = __ata_host_alloc_pinfo(&pdev->dev, ppi, 2, owner);
 	if (!host) {
 		dev_err(&pdev->dev, "failed to allocate ATA host\n");
 		rc = -ENOMEM;
@@ -2392,7 +2393,7 @@ err_out:
 	devres_release_group(&pdev->dev, NULL);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(ata_pci_sff_prepare_host);
+EXPORT_SYMBOL_GPL(__ata_pci_sff_prepare_host);
 
 /**
  *	ata_pci_sff_activate_host - start SFF host, request IRQ and register it
@@ -2500,7 +2501,7 @@ static const struct ata_port_info *ata_sff_find_valid_pi(
 static int ata_pci_init_one(struct pci_dev *pdev,
 		const struct ata_port_info * const *ppi,
 		struct scsi_host_template *sht, void *host_priv,
-		int hflags, bool bmdma)
+		int hflags, bool bmdma, struct module *owner)
 {
 	struct device *dev = &pdev->dev;
 	const struct ata_port_info *pi;
@@ -2525,11 +2526,11 @@ static int ata_pci_init_one(struct pci_dev *pdev,
 #ifdef CONFIG_ATA_BMDMA
 	if (bmdma)
 		/* prepare and activate BMDMA host */
-		rc = ata_pci_bmdma_prepare_host(pdev, ppi, &host);
+		rc = __ata_pci_bmdma_prepare_host(pdev, ppi, &host, owner);
 	else
 #endif
 		/* prepare and activate SFF host */
-		rc = ata_pci_sff_prepare_host(pdev, ppi, &host);
+		rc = __ata_pci_sff_prepare_host(pdev, ppi, &host, owner);
 	if (rc)
 		goto out;
 	host->private_data = host_priv;
@@ -2552,12 +2553,13 @@ out:
 }
 
 /**
- *	ata_pci_sff_init_one - Initialize/register PIO-only PCI IDE controller
+ *	__ata_pci_sff_init_one - Initialize/register PIO-only PCI IDE controller
  *	@pdev: Controller to be initialized
  *	@ppi: array of port_info, must be enough for two ports
  *	@sht: scsi_host_template to use when registering the host
  *	@host_priv: host private_data
  *	@hflag: host flags
+ *	@owner: module which will be the owner of the ATA host
  *
  *	This is a helper function which can be called from a driver's
  *	xxx_init_one() probe function if the hardware uses traditional
@@ -2573,13 +2575,14 @@ out:
  *	RETURNS:
  *	Zero on success, negative on errno-based value on error.
  */
-int ata_pci_sff_init_one(struct pci_dev *pdev,
-		 const struct ata_port_info * const *ppi,
-		 struct scsi_host_template *sht, void *host_priv, int hflag)
+int __ata_pci_sff_init_one(struct pci_dev *pdev,
+			   const struct ata_port_info * const *ppi,
+			   struct scsi_host_template *sht, void *host_priv,
+			   int hflag, struct module *owner)
 {
-	return ata_pci_init_one(pdev, ppi, sht, host_priv, hflag, 0);
+	return ata_pci_init_one(pdev, ppi, sht, host_priv, hflag, 0, owner);
 }
-EXPORT_SYMBOL_GPL(ata_pci_sff_init_one);
+EXPORT_SYMBOL_GPL(__ata_pci_sff_init_one);
 
 #endif /* CONFIG_PCI */
 
@@ -3245,10 +3248,11 @@ void ata_pci_bmdma_init(struct ata_host *host)
 EXPORT_SYMBOL_GPL(ata_pci_bmdma_init);
 
 /**
- *	ata_pci_bmdma_prepare_host - helper to prepare PCI BMDMA ATA host
+ *	__ata_pci_bmdma_prepare_host - helper to prepare PCI BMDMA ATA host
  *	@pdev: target PCI device
  *	@ppi: array of port_info, must be enough for two ports
  *	@r_host: out argument for the initialized ATA host
+ *	@owner: module which will be the owner of the ATA host
  *
  *	Helper to allocate BMDMA ATA host for @pdev, acquire all PCI
  *	resources and initialize it accordingly in one go.
@@ -3259,28 +3263,29 @@ EXPORT_SYMBOL_GPL(ata_pci_bmdma_init);
  *	RETURNS:
  *	0 on success, -errno otherwise.
  */
-int ata_pci_bmdma_prepare_host(struct pci_dev *pdev,
-			       const struct ata_port_info * const * ppi,
-			       struct ata_host **r_host)
+int __ata_pci_bmdma_prepare_host(struct pci_dev *pdev,
+				 const struct ata_port_info * const *ppi,
+				 struct ata_host **r_host, struct module *owner)
 {
 	int rc;
 
-	rc = ata_pci_sff_prepare_host(pdev, ppi, r_host);
+	rc = __ata_pci_sff_prepare_host(pdev, ppi, r_host, owner);
 	if (rc)
 		return rc;
 
 	ata_pci_bmdma_init(*r_host);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(ata_pci_bmdma_prepare_host);
+EXPORT_SYMBOL_GPL(__ata_pci_bmdma_prepare_host);
 
 /**
- *	ata_pci_bmdma_init_one - Initialize/register BMDMA PCI IDE controller
+ *	__ata_pci_bmdma_init_one - Initialize/register BMDMA PCI IDE controller
  *	@pdev: Controller to be initialized
  *	@ppi: array of port_info, must be enough for two ports
  *	@sht: scsi_host_template to use when registering the host
  *	@host_priv: host private_data
  *	@hflags: host flags
+ *	@owner: module which will be the owner of the ATA host
  *
  *	This function is similar to ata_pci_sff_init_one() but also
  *	takes care of BMDMA initialization.
@@ -3291,14 +3296,14 @@ EXPORT_SYMBOL_GPL(ata_pci_bmdma_prepare_host);
  *	RETURNS:
  *	Zero on success, negative on errno-based value on error.
  */
-int ata_pci_bmdma_init_one(struct pci_dev *pdev,
-			   const struct ata_port_info * const * ppi,
-			   struct scsi_host_template *sht, void *host_priv,
-			   int hflags)
+int __ata_pci_bmdma_init_one(struct pci_dev *pdev,
+			     const struct ata_port_info * const *ppi,
+			     struct scsi_host_template *sht, void *host_priv,
+			     int hflags, struct module *owner)
 {
-	return ata_pci_init_one(pdev, ppi, sht, host_priv, hflags, 1);
+	return ata_pci_init_one(pdev, ppi, sht, host_priv, hflags, 1, owner);
 }
-EXPORT_SYMBOL_GPL(ata_pci_bmdma_init_one);
+EXPORT_SYMBOL_GPL(__ata_pci_bmdma_init_one);
 
 #endif /* CONFIG_PCI */
 #endif /* CONFIG_ATA_BMDMA */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2d18241..541ceb1 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -598,6 +598,7 @@ struct ata_host {
 	void			*private_data;
 	struct ata_port_operations *ops;
 	unsigned long		flags;
+	struct module		*module;
 
 	struct mutex		eh_mutex;
 	struct task_struct	*eh_owner;
@@ -1106,9 +1107,15 @@ extern int sata_std_hardreset(struct ata_link *link, unsigned int *class,
 			      unsigned long deadline);
 extern void ata_std_postreset(struct ata_link *link, unsigned int *classes);
 
-extern struct ata_host *ata_host_alloc(struct device *dev, int max_ports);
-extern struct ata_host *ata_host_alloc_pinfo(struct device *dev,
-			const struct ata_port_info * const * ppi, int n_ports);
+extern struct ata_host *__ata_host_alloc(struct device *dev, int max_ports,
+					struct module *owner);
+#define ata_host_alloc(dev, max_ports) \
+	__ata_host_alloc(dev, max_ports, THIS_MODULE)
+extern struct ata_host *__ata_host_alloc_pinfo(struct device *dev,
+			const struct ata_port_info * const *ppi, int n_ports,
+			struct module *owner);
+#define ata_host_alloc_pinfo(dev, ppi, n_ports) \
+	__ata_host_alloc_pinfo(dev, ppi, n_ports, THIS_MODULE)
 extern int ata_slave_link_init(struct ata_port *ap);
 extern int ata_host_start(struct ata_host *host);
 extern int ata_host_register(struct ata_host *host,
@@ -1835,15 +1842,22 @@ extern void ata_sff_error_handler(struct ata_port *ap);
 extern void ata_sff_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI
 extern int ata_pci_sff_init_host(struct ata_host *host);
-extern int ata_pci_sff_prepare_host(struct pci_dev *pdev,
-				    const struct ata_port_info * const * ppi,
-				    struct ata_host **r_host);
+extern int __ata_pci_sff_prepare_host(struct pci_dev *pdev,
+				      const struct ata_port_info * const *ppi,
+				      struct ata_host **r_host,
+				      struct module *owner);
+#define ata_pci_sff_prepare_host(pdev, ppi, r_host) \
+	__ata_pci_sff_prepare_host(pdev, ppi, r_host, THIS_MODULE)
 extern int ata_pci_sff_activate_host(struct ata_host *host,
 				     irq_handler_t irq_handler,
 				     struct scsi_host_template *sht);
-extern int ata_pci_sff_init_one(struct pci_dev *pdev,
+extern int __ata_pci_sff_init_one(struct pci_dev *pdev,
 		const struct ata_port_info * const * ppi,
-		struct scsi_host_template *sht, void *host_priv, int hflags);
+		struct scsi_host_template *sht, void *host_priv, int hflags,
+		struct module *owner);
+#define ata_pci_sff_init_one(pdev, ppi, sht, host_priv, hflag)	\
+	__ata_pci_sff_init_one(pdev, ppi, sht, host_priv, hflag, THIS_MODULE)
+
 #endif /* CONFIG_PCI */
 
 #ifdef CONFIG_ATA_BMDMA
@@ -1874,13 +1888,21 @@ extern int ata_bmdma_port_start32(struct ata_port *ap);
 #ifdef CONFIG_PCI
 extern int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev);
 extern void ata_pci_bmdma_init(struct ata_host *host);
-extern int ata_pci_bmdma_prepare_host(struct pci_dev *pdev,
-				      const struct ata_port_info * const * ppi,
-				      struct ata_host **r_host);
-extern int ata_pci_bmdma_init_one(struct pci_dev *pdev,
-				  const struct ata_port_info * const * ppi,
-				  struct scsi_host_template *sht,
-				  void *host_priv, int hflags);
+extern int __ata_pci_bmdma_prepare_host(struct pci_dev *pdev,
+					const struct ata_port_info * const *ppi,
+					struct ata_host **r_host,
+					struct module *owner);
+#define ata_pci_bmdma_prepare_host(pdev, ppi, r_host) \
+	__ata_pci_bmdma_prepare_host(pdev, ppi, r_host, THIS_MODULE)
+
+extern int __ata_pci_bmdma_init_one(struct pci_dev *pdev,
+				    const struct ata_port_info * const *ppi,
+				    struct scsi_host_template *sht,
+				    void *host_priv, int hflags,
+				    struct module *owner);
+#define ata_pci_bmdma_init_one(pdev, ppi, sht, host_priv, hflags) \
+	__ata_pci_bmdma_init_one(pdev, ppi, sht, host_priv, hflags, THIS_MODULE)
+
 #endif /* CONFIG_PCI */
 #endif /* CONFIG_ATA_BMDMA */
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 02/11] iscsi: prepare to move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
  2015-01-18 15:05 ` [PATCH v4 01/11] ata: prepare to move module reference from scsi_host_template to Scsi_Host Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 03/11] cxgbi: " Akinobu Mita
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Mike Christie, Karen Xie, Christoph Hellwig,
	James E.J. Bottomley, open-iscsi

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this converts iscsi_host_alloc()
into macro so that LLDDs can pass THIS_MODULE to scsi_host_alloc() through
it instead of scsi_host_template->module.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Karen Xie <kxie@chelsio.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: open-iscsi@googlegroups.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/libiscsi.c | 10 ++++++----
 include/scsi/libiscsi.h |  9 ++++++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 8053f24..4c5be6b 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2593,16 +2593,18 @@ int iscsi_host_add(struct Scsi_Host *shost, struct device *pdev)
 EXPORT_SYMBOL_GPL(iscsi_host_add);
 
 /**
- * iscsi_host_alloc - allocate a host and driver data
+ * __iscsi_host_alloc - allocate a host and driver data
  * @sht: scsi host template
  * @dd_data_size: driver host data size
  * @xmit_can_sleep: bool indicating if LLD will queue IO from a work queue
+ * @owner: module which will be the owner of the scsi host
  *
  * This should be called by partial offload and software iscsi drivers.
  * To access the driver specific memory use the iscsi_host_priv() macro.
  */
-struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht,
-				   int dd_data_size, bool xmit_can_sleep)
+struct Scsi_Host *__iscsi_host_alloc(struct scsi_host_template *sht,
+				     int dd_data_size, bool xmit_can_sleep,
+				     struct module *owner)
 {
 	struct Scsi_Host *shost;
 	struct iscsi_host *ihost;
@@ -2630,7 +2632,7 @@ free_host:
 	scsi_host_put(shost);
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(iscsi_host_alloc);
+EXPORT_SYMBOL_GPL(__iscsi_host_alloc);
 
 static void iscsi_notify_host_removed(struct iscsi_cls_session *cls_session)
 {
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 4d1c46a..c95d957 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -396,9 +396,12 @@ extern int iscsi_host_set_param(struct Scsi_Host *shost,
 extern int iscsi_host_get_param(struct Scsi_Host *shost,
 				enum iscsi_host_param param, char *buf);
 extern int iscsi_host_add(struct Scsi_Host *shost, struct device *pdev);
-extern struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht,
-					  int dd_data_size,
-					  bool xmit_can_sleep);
+extern struct Scsi_Host *__iscsi_host_alloc(struct scsi_host_template *sht,
+					    int dd_data_size,
+					    bool xmit_can_sleep,
+					    struct module *owner);
+#define iscsi_host_alloc(sht, dd_data_size, xmit_can_sleep) \
+	__iscsi_host_alloc(sht, dd_data_size, xmit_can_sleep, THIS_MODULE)
 extern void iscsi_host_remove(struct Scsi_Host *shost);
 extern void iscsi_host_free(struct Scsi_Host *shost);
 extern int iscsi_target_alloc(struct scsi_target *starget);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 03/11] cxgbi: prepare to move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
  2015-01-18 15:05 ` [PATCH v4 01/11] ata: prepare to move module reference from scsi_host_template to Scsi_Host Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 02/11] iscsi: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 04/11] libfc: " Akinobu Mita
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Mike Christie, Karen Xie, Christoph Hellwig,
	James E.J. Bottomley, open-iscsi

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this converts cxgbi_hbas_add() into
macro so that LLDDs can pass THIS_MODULE to __iscsi_host_alloc() through
it instead of scsi_host_template->module.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Karen Xie <kxie@chelsio.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: open-iscsi@googlegroups.com
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/cxgbi/libcxgbi.c | 8 ++++----
 drivers/scsi/cxgbi/libcxgbi.h | 6 ++++--
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index eb58afc..f090d63 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -330,9 +330,9 @@ void cxgbi_hbas_remove(struct cxgbi_device *cdev)
 }
 EXPORT_SYMBOL_GPL(cxgbi_hbas_remove);
 
-int cxgbi_hbas_add(struct cxgbi_device *cdev, u64 max_lun,
+int __cxgbi_hbas_add(struct cxgbi_device *cdev, u64 max_lun,
 		unsigned int max_id, struct scsi_host_template *sht,
-		struct scsi_transport_template *stt)
+		struct scsi_transport_template *stt, struct module *owner)
 {
 	struct cxgbi_hba *chba;
 	struct Scsi_Host *shost;
@@ -341,7 +341,7 @@ int cxgbi_hbas_add(struct cxgbi_device *cdev, u64 max_lun,
 	log_debug(1 << CXGBI_DBG_DEV, "cdev 0x%p, p#%u.\n", cdev, cdev->nports);
 
 	for (i = 0; i < cdev->nports; i++) {
-		shost = iscsi_host_alloc(sht, sizeof(*chba), 1);
+		shost = __iscsi_host_alloc(sht, sizeof(*chba), 1, owner);
 		if (!shost) {
 			pr_info("0x%p, p%d, %s, host alloc failed.\n",
 				cdev, i, cdev->ports[i]->name);
@@ -383,7 +383,7 @@ err_out:
 	cxgbi_hbas_remove(cdev);
 	return err;
 }
-EXPORT_SYMBOL_GPL(cxgbi_hbas_add);
+EXPORT_SYMBOL_GPL(__cxgbi_hbas_add);
 
 /*
  * iSCSI offload
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index aba1af7..26c8bf7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -707,9 +707,11 @@ struct cxgbi_device *cxgbi_device_find_by_lldev(void *);
 struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *, int *);
 struct cxgbi_device *cxgbi_device_find_by_netdev_rcu(struct net_device *,
 						     int *);
-int cxgbi_hbas_add(struct cxgbi_device *, u64, unsigned int,
+int __cxgbi_hbas_add(struct cxgbi_device *, u64, unsigned int,
 			struct scsi_host_template *,
-			struct scsi_transport_template *);
+			struct scsi_transport_template *, struct module *);
+#define cxgbi_hbas_add(cdev, max_lun, max_id, sht, stt) \
+	__cxgbi_hbas_add(cdev, max_lun, max_id, sht, stt, THIS_MODULE)
 void cxgbi_hbas_remove(struct cxgbi_device *);
 
 int cxgbi_device_portmap_create(struct cxgbi_device *cdev, unsigned int base,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 04/11] libfc: prepare to move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (2 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 03/11] cxgbi: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 05/11] 53c700: prepare " Akinobu Mita
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Robert Love, Christoph Hellwig,
	James E.J. Bottomley, fcoe-devel

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this enables libfc_vport_create()
to allocate Scsi_Host with correct module reference through
__libfc_host_alloc() which is introduced while changing libfc_host_alloc()
from inline function to macro and it takes module reference explicitly.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Robert Love <robert.w.love@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: fcoe-devel@open-fcoe.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/libfc/fc_lport.c | 26 ++++++++++++++++++++++++++
 drivers/scsi/libfc/fc_npiv.c  |  3 ++-
 include/scsi/libfc.h          | 27 ++++-----------------------
 3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index e01a298..d931e69 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -2142,3 +2142,29 @@ int fc_lport_bsg_request(struct fc_bsg_job *job)
 	return rc;
 }
 EXPORT_SYMBOL(fc_lport_bsg_request);
+
+/**
+ * __libfc_host_alloc() - Allocate a Scsi_Host with room for a local port and
+ *                      LLD private data
+ * @sht:       The SCSI host template
+ * @priv_size: Size of private data
+ * @owner: module which will be the owner of the scsi host
+ *
+ * Returns: libfc lport
+ */
+struct fc_lport *__libfc_host_alloc(struct scsi_host_template *sht,
+				    int priv_size, struct module *owner)
+{
+	struct fc_lport *lport;
+	struct Scsi_Host *shost;
+
+	shost = scsi_host_alloc(sht, sizeof(*lport) + priv_size);
+	if (!shost)
+		return NULL;
+	lport = shost_priv(shost);
+	lport->host = shost;
+	INIT_LIST_HEAD(&lport->ema_list);
+	INIT_LIST_HEAD(&lport->vports);
+	return lport;
+}
+EXPORT_SYMBOL(__libfc_host_alloc);
diff --git a/drivers/scsi/libfc/fc_npiv.c b/drivers/scsi/libfc/fc_npiv.c
index 9fbf78e..86133f9 100644
--- a/drivers/scsi/libfc/fc_npiv.c
+++ b/drivers/scsi/libfc/fc_npiv.c
@@ -36,7 +36,8 @@ struct fc_lport *libfc_vport_create(struct fc_vport *vport, int privsize)
 	struct fc_lport *n_port = shost_priv(shost);
 	struct fc_lport *vn_port;
 
-	vn_port = libfc_host_alloc(shost->hostt, privsize);
+	vn_port = __libfc_host_alloc(shost->hostt, privsize,
+				     shost->hostt->module);
 	if (!vn_port)
 		return vn_port;
 
diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
index 93d14da..b4fbfc2 100644
--- a/include/scsi/libfc.h
+++ b/include/scsi/libfc.h
@@ -1018,29 +1018,10 @@ static inline void *lport_priv(const struct fc_lport *lport)
 	return (void *)(lport + 1);
 }
 
-/**
- * libfc_host_alloc() - Allocate a Scsi_Host with room for a local port and
- *                      LLD private data
- * @sht:       The SCSI host template
- * @priv_size: Size of private data
- *
- * Returns: libfc lport
- */
-static inline struct fc_lport *
-libfc_host_alloc(struct scsi_host_template *sht, int priv_size)
-{
-	struct fc_lport *lport;
-	struct Scsi_Host *shost;
-
-	shost = scsi_host_alloc(sht, sizeof(*lport) + priv_size);
-	if (!shost)
-		return NULL;
-	lport = shost_priv(shost);
-	lport->host = shost;
-	INIT_LIST_HEAD(&lport->ema_list);
-	INIT_LIST_HEAD(&lport->vports);
-	return lport;
-}
+struct fc_lport *__libfc_host_alloc(struct scsi_host_template *sht,
+				    int priv_size, struct module *owner);
+#define libfc_host_alloc(sht, priv_size) \
+	__libfc_host_alloc(sht, priv_size, THIS_MODULE)
 
 /*
  * FC_FCP HELPER FUNCTIONS
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 05/11] 53c700: prepare move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (3 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 04/11] libfc: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 06/11] scsi: legacy: prepare to " Akinobu Mita
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi; +Cc: Akinobu Mita, Christoph Hellwig, James E.J. Bottomley

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this converts NCR_700_detect() into
macro so that LLDDs can pass THIS_MODULE to scsi_host_alloc() through it
instead of scsi_host_template->module.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/53c700.c | 7 ++++---
 drivers/scsi/53c700.h | 7 +++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 82abfce..e70fd05 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -283,8 +283,9 @@ NCR_700_get_SXFER(struct scsi_device *SDp)
 }
 
 struct Scsi_Host *
-NCR_700_detect(struct scsi_host_template *tpnt,
-	       struct NCR_700_Host_Parameters *hostdata, struct device *dev)
+__NCR_700_detect(struct scsi_host_template *tpnt,
+		 struct NCR_700_Host_Parameters *hostdata, struct device *dev,
+		 struct module *owner)
 {
 	dma_addr_t pScript, pSlots;
 	__u8 *memory;
@@ -2101,7 +2102,7 @@ STATIC struct device_attribute *NCR_700_dev_attrs[] = {
 	NULL,
 };
 
-EXPORT_SYMBOL(NCR_700_detect);
+EXPORT_SYMBOL(__NCR_700_detect);
 EXPORT_SYMBOL(NCR_700_release);
 EXPORT_SYMBOL(NCR_700_intr);
 
diff --git a/drivers/scsi/53c700.h b/drivers/scsi/53c700.h
index e06bdfe..4555958 100644
--- a/drivers/scsi/53c700.h
+++ b/drivers/scsi/53c700.h
@@ -54,8 +54,11 @@
 struct NCR_700_Host_Parameters;
 
 /* These are the externally used routines */
-struct Scsi_Host *NCR_700_detect(struct scsi_host_template *,
-		struct NCR_700_Host_Parameters *, struct device *);
+struct Scsi_Host *__NCR_700_detect(struct scsi_host_template *,
+		struct NCR_700_Host_Parameters *, struct device *,
+		struct module *);
+#define NCR_700_detect(tpnt, hostdata, dev) \
+	__NCR_700_detect(tpnt, hostdata, dev, THIS_MODULE)
 int NCR_700_release(struct Scsi_Host *host);
 irqreturn_t NCR_700_intr(int, void *);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 06/11] scsi: legacy: prepare to move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (4 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 05/11] 53c700: prepare " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 07/11] scsi: " Akinobu Mita
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi; +Cc: Akinobu Mita, Christoph Hellwig, James E.J. Bottomley

In preparation for moving owner module reference field from struct
scsi_host_template to struct Scsi_Host, this converts scsi_register() into
macro so that legacy LLDDs can pass THIS_MODULE to scsi_host_alloc()
through it instead of scsi_host_template->module.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/hosts.c     | 5 +++--
 include/scsi/scsi_host.h | 5 ++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 8bb173e..eb324f1 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -499,7 +499,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 }
 EXPORT_SYMBOL(scsi_host_alloc);
 
-struct Scsi_Host *scsi_register(struct scsi_host_template *sht, int privsize)
+struct Scsi_Host *__scsi_register(struct scsi_host_template *sht, int privsize,
+				  struct module *owner)
 {
 	struct Scsi_Host *shost = scsi_host_alloc(sht, privsize);
 
@@ -513,7 +514,7 @@ struct Scsi_Host *scsi_register(struct scsi_host_template *sht, int privsize)
 		list_add_tail(&shost->sht_legacy_list, &sht->legacy_hosts);
 	return shost;
 }
-EXPORT_SYMBOL(scsi_register);
+EXPORT_SYMBOL(__scsi_register);
 
 void scsi_unregister(struct Scsi_Host *shost)
 {
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 019e668..0369b4e 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -926,7 +926,10 @@ static inline unsigned char scsi_host_get_guard(struct Scsi_Host *shost)
 }
 
 /* legacy interfaces */
-extern struct Scsi_Host *scsi_register(struct scsi_host_template *, int);
+extern struct Scsi_Host *__scsi_register(struct scsi_host_template *, int,
+					 struct module *);
+#define scsi_register(sht, privsize) \
+	__scsi_register(sht, privsize, THIS_MODULE)
 extern void scsi_unregister(struct Scsi_Host *);
 extern int scsi_host_set_state(struct Scsi_Host *, enum scsi_host_state);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 07/11] scsi: move module reference from scsi_host_template to Scsi_Host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (5 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 06/11] scsi: legacy: prepare to " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 08/11] scsi: ufs: adjust module reference for scsi host Akinobu Mita
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Vinayak Holikatti, Dolev Raviv, Sujit Reddy Thumma,
	Subhash Jadavani, Matthew Dharm, Greg Kroah-Hartman, Alan Stern,
	David S. Miller, Hannes Reinecke, Tejun Heo, Hans de Goede,
	Mike Christie, Karen Xie, Robert Love, Christoph Hellwig,
	James E.J. Bottomley, open-iscsi, fcoe-devel, linux-ide,
	linux-usb, usb-storage

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 drivers which consist with the core driver and the actual
LLDDs and scsi_host_template is defined in the core driver.  In these
cases, the actual LLDDs can be unloaded even if the scsi_device is
being accessed.

This moves owner module reference from struct scsi_host_template to
struct Scsi_Host, and it is passed through scsi_host_alloc().  Some
drivers which have the module reference mismatch problem described above
can pass their correct module reference by using __scsi_host_alloc()
which takes module reference argument.

Sub drivers of esp_scsi (mac_esp, am53c974, sun_esp, jazz_esp, sun3x_esp)
use a single scsi_host_template defined in esp_scsi module, so these
had module reference mismatch problem, but this change implicitly fixes
it.  Because sub drivers directly call scsi_host_alloc() and THIS_MODULE
is used for the module reference instead of scsi_host_template->module.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: Dolev Raviv <draviv@codeaurora.org>
Cc: Sujit Reddy Thumma <sthumma@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Mike Christie <michaelc@cs.wisc.edu>
Cc: Karen Xie <kxie@chelsio.com>
Cc: Robert Love <robert.w.love@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: open-iscsi@googlegroups.com
Cc: fcoe-devel@open-fcoe.org
Cc: linux-ide@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: usb-storage@lists.one-eyed-alien.net
Cc: linux-scsi@vger.kernel.org
---
 drivers/ata/libata-scsi.c     |  2 +-
 drivers/scsi/53c700.c         |  2 +-
 drivers/scsi/hosts.c          | 11 +++++++----
 drivers/scsi/libfc/fc_lport.c |  2 +-
 drivers/scsi/libfc/fc_npiv.c  |  3 +--
 drivers/scsi/libiscsi.c       |  2 +-
 drivers/scsi/scsi.c           |  4 ++--
 include/scsi/scsi_host.h      |  8 +++++++-
 8 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e364e86..20a963c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3643,7 +3643,7 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		struct Scsi_Host *shost;
 
 		rc = -ENOMEM;
-		shost = scsi_host_alloc(sht, sizeof(struct ata_port *));
+		shost = __scsi_host_alloc(sht, sizeof(ap), host->module);
 		if (!shost)
 			goto err_alloc;
 
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index e70fd05..a5777f8 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -333,7 +333,7 @@ __NCR_700_detect(struct scsi_host_template *tpnt,
 	if(tpnt->proc_name == NULL)
 		tpnt->proc_name = "53c700";
 
-	host = scsi_host_alloc(tpnt, 4);
+	host = __scsi_host_alloc(tpnt, sizeof(hostdata), owner);
 	if (!host)
 		return NULL;
 	memset(hostdata->slots, 0, sizeof(struct NCR_700_command_slot)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index eb324f1..90c109e 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -354,9 +354,10 @@ static struct device_type scsi_host_type = {
 };
 
 /**
- * scsi_host_alloc - register a scsi host adapter instance.
+ * __scsi_host_alloc - register a scsi host adapter instance.
  * @sht:	pointer to scsi host template
  * @privsize:	extra bytes to allocate for driver
+ * @owner:	module which will be the owner of the scsi host
  *
  * Note:
  * 	Allocate a new Scsi_Host and perform basic initialization.
@@ -366,7 +367,8 @@ static struct device_type scsi_host_type = {
  * Return value:
  * 	Pointer to a new Scsi_Host
  **/
-struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
+struct Scsi_Host *__scsi_host_alloc(struct scsi_host_template *sht,
+				    int privsize, struct module *owner)
 {
 	struct Scsi_Host *shost;
 	gfp_t gfp_mask = GFP_KERNEL;
@@ -411,6 +413,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	 */
 	shost->max_cmd_len = 12;
 	shost->hostt = sht;
+	shost->module = owner;
 	shost->this_id = sht->this_id;
 	shost->can_queue = sht->can_queue;
 	shost->sg_tablesize = sht->sg_tablesize;
@@ -497,12 +500,12 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	kfree(shost);
 	return NULL;
 }
-EXPORT_SYMBOL(scsi_host_alloc);
+EXPORT_SYMBOL(__scsi_host_alloc);
 
 struct Scsi_Host *__scsi_register(struct scsi_host_template *sht, int privsize,
 				  struct module *owner)
 {
-	struct Scsi_Host *shost = scsi_host_alloc(sht, privsize);
+	struct Scsi_Host *shost = __scsi_host_alloc(sht, privsize, owner);
 
 	if (!sht->detect) {
 		printk(KERN_WARNING "scsi_register() called on new-style "
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index d931e69..54ef1c4 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -2158,7 +2158,7 @@ struct fc_lport *__libfc_host_alloc(struct scsi_host_template *sht,
 	struct fc_lport *lport;
 	struct Scsi_Host *shost;
 
-	shost = scsi_host_alloc(sht, sizeof(*lport) + priv_size);
+	shost = __scsi_host_alloc(sht, sizeof(*lport) + priv_size, owner);
 	if (!shost)
 		return NULL;
 	lport = shost_priv(shost);
diff --git a/drivers/scsi/libfc/fc_npiv.c b/drivers/scsi/libfc/fc_npiv.c
index 86133f9..1cf2abb 100644
--- a/drivers/scsi/libfc/fc_npiv.c
+++ b/drivers/scsi/libfc/fc_npiv.c
@@ -36,8 +36,7 @@ struct fc_lport *libfc_vport_create(struct fc_vport *vport, int privsize)
 	struct fc_lport *n_port = shost_priv(shost);
 	struct fc_lport *vn_port;
 
-	vn_port = __libfc_host_alloc(shost->hostt, privsize,
-				     shost->hostt->module);
+	vn_port = __libfc_host_alloc(shost->hostt, privsize, shost->module);
 	if (!vn_port)
 		return vn_port;
 
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 4c5be6b..6c2ebf3 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2609,7 +2609,7 @@ struct Scsi_Host *__iscsi_host_alloc(struct scsi_host_template *sht,
 	struct Scsi_Host *shost;
 	struct iscsi_host *ihost;
 
-	shost = scsi_host_alloc(sht, sizeof(struct iscsi_host) + dd_data_size);
+	shost = __scsi_host_alloc(sht, sizeof(*ihost) + dd_data_size, owner);
 	if (!shost)
 		return NULL;
 	ihost = shost_priv(shost);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e028854..5905b83 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -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 */
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 0369b4e..acdf828 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -47,6 +47,7 @@ struct blk_queue_tags;
 #define ENABLE_CLUSTERING 1
 
 struct scsi_host_template {
+	/* This is unused and will be removed after mass conversion */
 	struct module *module;
 	const char *name;
 
@@ -617,6 +618,7 @@ struct Scsi_Host {
 	 */
 	unsigned short max_cmd_len;
 
+	struct module *module;
 	int this_id;
 	int can_queue;
 	short cmd_per_lun;
@@ -783,7 +785,11 @@ static inline bool shost_use_blk_mq(struct Scsi_Host *shost)
 extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
 extern void scsi_flush_work(struct Scsi_Host *);
 
-extern struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *, int);
+extern struct Scsi_Host *__scsi_host_alloc(struct scsi_host_template *, int,
+					struct module *);
+#define scsi_host_alloc(sht, privsize) \
+	__scsi_host_alloc(sht, privsize, THIS_MODULE)
+
 extern int __must_check scsi_add_host_with_dma(struct Scsi_Host *,
 					       struct device *,
 					       struct device *);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 08/11] scsi: ufs: adjust module reference for scsi host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (6 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 07/11] scsi: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 09/11] usb: storage: " Akinobu Mita
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Vinayak Holikatti, Dolev Raviv, Sujit Reddy Thumma,
	Subhash Jadavani, Christoph Hellwig, James E.J. Bottomley

The ufs support consists with the core driver (ufshcd) and the actual
glue driver (ufshcd-pci or ufshcd-pltfrm).  The module reference of
this scsi host is initialized to the core driver's one.  Because the
glue drivers use ufshcd_alloc_host() which is defined in ufshcd module
and calls scsi_host_alloc().  So the glue drivers can be unloaded even
if the scsi device is being accessed.

This fixes it by converting ufshcd_alloc_host() into macro so that the
glue drivers can pass their correct module reference through
__scsi_host_alloc().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Vinayak Holikatti <vinholikatti@gmail.com>
Cc: Dolev Raviv <draviv@codeaurora.org>
Cc: Sujit Reddy Thumma <sthumma@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/ufs/ufshcd.c | 13 +++++++------
 drivers/scsi/ufs/ufshcd.h |  5 ++++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2e26025..59a4855 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -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,
@@ -5256,12 +5255,14 @@ static int ufshcd_set_dma_mask(struct ufs_hba *hba)
 }
 
 /**
- * ufshcd_alloc_host - allocate Host Bus Adapter (HBA)
+ * __ufshcd_alloc_host - allocate Host Bus Adapter (HBA)
  * @dev: pointer to device handle
  * @hba_handle: driver private handle
+ * @owner: module which will be the owner of the scsi host
  * Returns 0 on success, non-zero value on failure
  */
-int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
+int __ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle,
+			struct module *owner)
 {
 	struct Scsi_Host *host;
 	struct ufs_hba *hba;
@@ -5274,8 +5275,8 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
 		goto out_error;
 	}
 
-	host = scsi_host_alloc(&ufshcd_driver_template,
-				sizeof(struct ufs_hba));
+	host = __scsi_host_alloc(&ufshcd_driver_template,
+				sizeof(struct ufs_hba), owner);
 	if (!host) {
 		dev_err(dev, "scsi_host_alloc failed\n");
 		err = -ENOMEM;
@@ -5289,7 +5290,7 @@ int ufshcd_alloc_host(struct device *dev, struct ufs_hba **hba_handle)
 out_error:
 	return err;
 }
-EXPORT_SYMBOL(ufshcd_alloc_host);
+EXPORT_SYMBOL(__ufshcd_alloc_host);
 
 static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
 {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 4a574aa..ac7d72f 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -515,7 +515,10 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 	ufshcd_writel(hba, tmp, reg);
 }
 
-int ufshcd_alloc_host(struct device *, struct ufs_hba **);
+int __ufshcd_alloc_host(struct device *, struct ufs_hba **,
+			struct module *owner);
+#define ufshcd_alloc_host(dev, hba_handle) \
+	__ufshcd_alloc_host(dev, hba_handle, THIS_MODULE)
 int ufshcd_init(struct ufs_hba * , void __iomem * , unsigned int);
 void ufshcd_remove(struct ufs_hba *);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 09/11] usb: storage: adjust module reference for scsi host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (7 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 08/11] scsi: ufs: adjust module reference for scsi host Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:06 ` [PATCH v4 10/11] ata: ahci_platform: " Akinobu Mita
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Matthew Dharm, Greg Kroah-Hartman, Alan Stern,
	Christoph Hellwig, James E.J. Bottomley, linux-usb, usb-storage

The unusual usb storage drivers (ums-alauda, ums-cypress, ...) depend
on usb-storage module.  The module reference of these scsi host is
initialized to usb-storage's one.  Because these drivers use
usb_stor_probe1() which is defined in usb-storage module and calls
scsi_host_alloc().  So these drivers can be unloaded even if the scsi
device is being accessed.

This fixes it by converting usb_stor_probe1() into macro so that these
drivers can pass their correct module reference through
__scsi_host_alloc().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-usb@vger.kernel.org
Cc: usb-storage@lists.one-eyed-alien.net
Cc: linux-scsi@vger.kernel.org
---
 drivers/usb/storage/scsiglue.c | 3 ---
 drivers/usb/storage/usb.c      | 9 +++++----
 drivers/usb/storage/usb.h      | 7 +++++--
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 0e400f3..04fab36 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -587,9 +587,6 @@ struct scsi_host_template usb_stor_host_template = {
 
 	/* sysfs device attributes */
 	.sdev_attrs =			sysfs_device_attr_list,
-
-	/* module management */
-	.module =			THIS_MODULE
 };
 
 /* To Report "Illegal Request: Invalid Field in CDB */
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index d468d02..768413f0 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -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;
@@ -926,7 +927,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(&usb_stor_host_template, sizeof(*us), owner);
 	if (!host) {
 		dev_warn(&intf->dev, "Unable to allocate the scsi host\n");
 		return -ENOMEM;
@@ -969,7 +970,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)
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 307e339..0cb74ba 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 10/11] ata: ahci_platform: adjust module reference for scsi host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (8 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 09/11] usb: storage: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-18 15:30   ` Hans de Goede
  2015-01-18 15:06 ` [PATCH v4 11/11] ata: pata_of_platform: " Akinobu Mita
  2015-01-19 14:22 ` [PATCH v4 00/11] scsi: fix module reference mismatch " Tejun Heo
  11 siblings, 1 reply; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Hans de Goede, Tejun Heo, Christoph Hellwig,
	James E.J. Bottomley, linux-ide

The ahci platform drivers depend on libahci_platform module.  The
module reference of these scsi host is initialized to libahci_platform's
one.  Because these drivers use ahci_platform_init_host() which is
defined in libahci_platform module and calls scsi_host_alloc()
internally.  So these drivers can be unloaded even if the scsi device
is being accessed.

This fixes it by converting into ahci_platform_init_host() macro so that
these drivers can pass their correct module reference through
__ata_host_alloc_pinfo().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-ide@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/ata/libahci_platform.c | 14 ++++++++------
 include/linux/ahci_platform.h  |  9 ++++++---
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 0b03f90..ea75416 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -395,10 +395,11 @@ err_out:
 EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
 
 /**
- * ahci_platform_init_host - Bring up an ahci-platform host
+ * __ahci_platform_init_host - Bring up an ahci-platform host
  * @pdev: platform device pointer for the host
  * @hpriv: ahci-host private data for the host
  * @pi_template: template for the ata_port_info to use
+ * @owner: module which will be the owner of the ahci-platform host
  *
  * This function does all the usual steps needed to bring up an
  * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
@@ -407,9 +408,10 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
  * RETURNS:
  * 0 on success otherwise a negative error code
  */
-int ahci_platform_init_host(struct platform_device *pdev,
-			    struct ahci_host_priv *hpriv,
-			    const struct ata_port_info *pi_template)
+int __ahci_platform_init_host(struct platform_device *pdev,
+			      struct ahci_host_priv *hpriv,
+			      const struct ata_port_info *pi_template,
+			      struct module *owner)
 {
 	struct device *dev = &pdev->dev;
 	struct ata_port_info pi = *pi_template;
@@ -443,7 +445,7 @@ int ahci_platform_init_host(struct platform_device *pdev,
 	 */
 	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
 
-	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
+	host = __ata_host_alloc_pinfo(dev, ppi, n_ports, owner);
 	if (!host)
 		return -ENOMEM;
 
@@ -495,7 +497,7 @@ int ahci_platform_init_host(struct platform_device *pdev,
 
 	return ahci_host_activate(host, irq, &ahci_platform_sht);
 }
-EXPORT_SYMBOL_GPL(ahci_platform_init_host);
+EXPORT_SYMBOL_GPL(__ahci_platform_init_host);
 
 static void ahci_host_stop(struct ata_host *host)
 {
diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
index 642d6ae..0dcac84 100644
--- a/include/linux/ahci_platform.h
+++ b/include/linux/ahci_platform.h
@@ -28,9 +28,12 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
 void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
 struct ahci_host_priv *ahci_platform_get_resources(
 	struct platform_device *pdev);
-int ahci_platform_init_host(struct platform_device *pdev,
-			    struct ahci_host_priv *hpriv,
-			    const struct ata_port_info *pi_template);
+int __ahci_platform_init_host(struct platform_device *pdev,
+			      struct ahci_host_priv *hpriv,
+			      const struct ata_port_info *pi_template,
+			      struct module *owner);
+#define ahci_platform_init_host(pdev, hpriv, pi_template) \
+	__ahci_platform_init_host(pdev, hpriv, pi_template, THIS_MODULE)
 
 int ahci_platform_suspend_host(struct device *dev);
 int ahci_platform_resume_host(struct device *dev);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v4 11/11] ata: pata_of_platform: adjust module reference for scsi host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (9 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 10/11] ata: ahci_platform: " Akinobu Mita
@ 2015-01-18 15:06 ` Akinobu Mita
  2015-01-19 14:22 ` [PATCH v4 00/11] scsi: fix module reference mismatch " Tejun Heo
  11 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-18 15:06 UTC (permalink / raw)
  To: linux-scsi
  Cc: Akinobu Mita, Tejun Heo, Christoph Hellwig, James E.J. Bottomley,
	linux-ide

The pata_of_platform driver depends on pata_platform module.  The
module reference of this scsi host is initialized to pata_platform's
one.  Because this drivers use __pata_platform_probe() which is
defined in pata_platform module and calls scsi_host_alloc() internally.
So these drivers can be unloaded even if the scsi device is being
accessed.

This fixes it by converting into __pata_platform_probe() macro so that
these drivers can pass their correct module reference through
__ata_host_alloc().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-ide@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/ata/pata_platform.c  | 18 ++++++++++--------
 include/linux/ata_platform.h | 16 ++++++++++------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index 1eedfe4..c84e834 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -71,13 +71,14 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
 }
 
 /**
- *	__pata_platform_probe		-	attach a platform interface
+ *	___pata_platform_probe		-	attach a platform interface
  *	@dev: device
  *	@io_res: Resource representing I/O base
  *	@ctl_res: Resource representing CTL base
  *	@irq_res: Resource representing IRQ and its flags
  *	@ioport_shift: I/O port shift
  *	@__pio_mask: PIO mask
+ *	@owner: module which will be the owner of the ATA host
  *
  *	Register a platform bus IDE interface. Such interfaces are PIO and we
  *	assume do not support IRQ sharing.
@@ -97,9 +98,10 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
  *
  *	If no IRQ resource is present, PIO polling mode is used instead.
  */
-int __pata_platform_probe(struct device *dev, struct resource *io_res,
-			  struct resource *ctl_res, struct resource *irq_res,
-			  unsigned int ioport_shift, int __pio_mask)
+int ___pata_platform_probe(struct device *dev, struct resource *io_res,
+			   struct resource *ctl_res, struct resource *irq_res,
+			   unsigned int ioport_shift, int __pio_mask,
+			   struct module *owner)
 {
 	struct ata_host *host;
 	struct ata_port *ap;
@@ -124,7 +126,7 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
 	/*
 	 * Now that that's out of the way, wire up the port..
 	 */
-	host = ata_host_alloc(dev, 1);
+	host = __ata_host_alloc(dev, 1, owner);
 	if (!host)
 		return -ENOMEM;
 	ap = host->ports[0];
@@ -172,7 +174,7 @@ int __pata_platform_probe(struct device *dev, struct resource *io_res,
 	return ata_host_activate(host, irq, irq ? ata_sff_interrupt : NULL,
 				 irq_flags, &pata_platform_sht);
 }
-EXPORT_SYMBOL_GPL(__pata_platform_probe);
+EXPORT_SYMBOL_GPL(___pata_platform_probe);
 
 static int pata_platform_probe(struct platform_device *pdev)
 {
@@ -215,8 +217,8 @@ static int pata_platform_probe(struct platform_device *pdev)
 	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 
 	return __pata_platform_probe(&pdev->dev, io_res, ctl_res, irq_res,
-				     pp_info ? pp_info->ioport_shift : 0,
-				     pio_mask);
+				   pp_info ? pp_info->ioport_shift : 0,
+				   pio_mask);
 }
 
 static struct platform_driver pata_platform_driver = {
diff --git a/include/linux/ata_platform.h b/include/linux/ata_platform.h
index 5c618a0..20bca38 100644
--- a/include/linux/ata_platform.h
+++ b/include/linux/ata_platform.h
@@ -10,12 +10,16 @@ struct pata_platform_info {
 	unsigned int ioport_shift;
 };
 
-extern int __pata_platform_probe(struct device *dev,
-				 struct resource *io_res,
-				 struct resource *ctl_res,
-				 struct resource *irq_res,
-				 unsigned int ioport_shift,
-				 int __pio_mask);
+extern int ___pata_platform_probe(struct device *dev,
+				  struct resource *io_res,
+				  struct resource *ctl_res,
+				  struct resource *irq_res,
+				  unsigned int ioport_shift,
+				  int __pio_mask, struct module *owner);
+#define __pata_platform_probe(dev, io_res, ctl_res, irq_res,		\
+			      ioport_shift, __pio_mask)			\
+	___pata_platform_probe(dev, io_res, ctl_res, irq_res,		\
+			       ioport_shift, __pio_mask, THIS_MODULE)
 
 /*
  * Marvell SATA private data
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 10/11] ata: ahci_platform: adjust module reference for scsi host
  2015-01-18 15:06 ` [PATCH v4 10/11] ata: ahci_platform: " Akinobu Mita
@ 2015-01-18 15:30   ` Hans de Goede
  0 siblings, 0 replies; 18+ messages in thread
From: Hans de Goede @ 2015-01-18 15:30 UTC (permalink / raw)
  To: Akinobu Mita, linux-scsi
  Cc: Tejun Heo, Christoph Hellwig, James E.J. Bottomley, linux-ide

Hi,

On 18-01-15 16:06, Akinobu Mita wrote:
> The ahci platform drivers depend on libahci_platform module.  The
> module reference of these scsi host is initialized to libahci_platform's
> one.  Because these drivers use ahci_platform_init_host() which is
> defined in libahci_platform module and calls scsi_host_alloc()
> internally.  So these drivers can be unloaded even if the scsi device
> is being accessed.
>
> This fixes it by converting into ahci_platform_init_host() macro so that
> these drivers can pass their correct module reference through
> __ata_host_alloc_pinfo().
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: linux-ide@vger.kernel.org
> Cc: linux-scsi@vger.kernel.org

Looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
>   drivers/ata/libahci_platform.c | 14 ++++++++------
>   include/linux/ahci_platform.h  |  9 ++++++---
>   2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 0b03f90..ea75416 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -395,10 +395,11 @@ err_out:
>   EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
>
>   /**
> - * ahci_platform_init_host - Bring up an ahci-platform host
> + * __ahci_platform_init_host - Bring up an ahci-platform host
>    * @pdev: platform device pointer for the host
>    * @hpriv: ahci-host private data for the host
>    * @pi_template: template for the ata_port_info to use
> + * @owner: module which will be the owner of the ahci-platform host
>    *
>    * This function does all the usual steps needed to bring up an
>    * ahci-platform host, note any necessary resources (ie clks, phys, etc.)
> @@ -407,9 +408,10 @@ EXPORT_SYMBOL_GPL(ahci_platform_get_resources);
>    * RETURNS:
>    * 0 on success otherwise a negative error code
>    */
> -int ahci_platform_init_host(struct platform_device *pdev,
> -			    struct ahci_host_priv *hpriv,
> -			    const struct ata_port_info *pi_template)
> +int __ahci_platform_init_host(struct platform_device *pdev,
> +			      struct ahci_host_priv *hpriv,
> +			      const struct ata_port_info *pi_template,
> +			      struct module *owner)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct ata_port_info pi = *pi_template;
> @@ -443,7 +445,7 @@ int ahci_platform_init_host(struct platform_device *pdev,
>   	 */
>   	n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map));
>
> -	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
> +	host = __ata_host_alloc_pinfo(dev, ppi, n_ports, owner);
>   	if (!host)
>   		return -ENOMEM;
>
> @@ -495,7 +497,7 @@ int ahci_platform_init_host(struct platform_device *pdev,
>
>   	return ahci_host_activate(host, irq, &ahci_platform_sht);
>   }
> -EXPORT_SYMBOL_GPL(ahci_platform_init_host);
> +EXPORT_SYMBOL_GPL(__ahci_platform_init_host);
>
>   static void ahci_host_stop(struct ata_host *host)
>   {
> diff --git a/include/linux/ahci_platform.h b/include/linux/ahci_platform.h
> index 642d6ae..0dcac84 100644
> --- a/include/linux/ahci_platform.h
> +++ b/include/linux/ahci_platform.h
> @@ -28,9 +28,12 @@ int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
>   void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
>   struct ahci_host_priv *ahci_platform_get_resources(
>   	struct platform_device *pdev);
> -int ahci_platform_init_host(struct platform_device *pdev,
> -			    struct ahci_host_priv *hpriv,
> -			    const struct ata_port_info *pi_template);
> +int __ahci_platform_init_host(struct platform_device *pdev,
> +			      struct ahci_host_priv *hpriv,
> +			      const struct ata_port_info *pi_template,
> +			      struct module *owner);
> +#define ahci_platform_init_host(pdev, hpriv, pi_template) \
> +	__ahci_platform_init_host(pdev, hpriv, pi_template, THIS_MODULE)
>
>   int ahci_platform_suspend_host(struct device *dev);
>   int ahci_platform_resume_host(struct device *dev);
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
  2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
                   ` (10 preceding siblings ...)
  2015-01-18 15:06 ` [PATCH v4 11/11] ata: pata_of_platform: " Akinobu Mita
@ 2015-01-19 14:22 ` Tejun Heo
       [not found]   ` <20150119142206.GE8140-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
  11 siblings, 1 reply; 18+ messages in thread
From: Tejun Heo @ 2015-01-19 14:22 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-scsi, Vinayak Holikatti, Dolev Raviv, Sujit Reddy Thumma,
	Subhash Jadavani, Matthew Dharm, Greg Kroah-Hartman, Alan Stern,
	David S. Miller, Hannes Reinecke, Hans de Goede, Mike Christie,
	Karen Xie, Robert Love, Christoph Hellwig, James E.J. Bottomley,
	open-iscsi, fcoe-devel, linux-ide, linux-usb, usb-storage

On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote:
> 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 drivers which consist with the core driver and the actual
> LLDDs and scsi_host_template is defined in the core driver.  In these
> cases, the actual LLDDs can be unloaded even if the scsi_device is
> being accessed.
> 
> This patch series fixes the module reference mismatch problem for
> ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
> by moving owner module reference field from struct scsi_host_template
> to struct Scsi_Host and allowing the LLDDs to set their correct module
> reference.

Hmmm, can't we just move sht definitions to actual LLDDs?  libata can
do that easily.  sht, as its name implies, is the template for
creating the scsi_hosts of a given type.  We're now just moving module
ownership from sht definition site to whatever callsite the actual
instance is being created which can also be wrapped in a separate
layer requiring explicit propagation.  Why not just propagate sht's
directly?  What's the difference?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
       [not found]   ` <20150119142206.GE8140-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
@ 2015-01-20 14:57     ` Akinobu Mita
  2015-01-20 15:18       ` Tejun Heo
  2015-01-20 15:20       ` Alan Stern
  0 siblings, 2 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-20 14:57 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, Vinayak Holikatti,
	Dolev Raviv, Sujit Reddy Thumma, Subhash Jadavani, Matthew Dharm,
	Greg Kroah-Hartman, Alan Stern, David S. Miller, Hannes Reinecke,
	Hans de Goede, Mike Christie, Karen Xie, Robert Love,
	Christoph Hellwig, James E.J. Bottomley,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	fcoe-devel-s9riP+hp16TNLxjTenLetw,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	usb-storage-ijkIwGHArpdIPJnuZ7Njw4oP9KaGy4wf

2015-01-19 23:22 GMT+09:00 Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote:
>> 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 drivers which consist with the core driver and the actual
>> LLDDs and scsi_host_template is defined in the core driver.  In these
>> cases, the actual LLDDs can be unloaded even if the scsi_device is
>> being accessed.
>>
>> This patch series fixes the module reference mismatch problem for
>> ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
>> by moving owner module reference field from struct scsi_host_template
>> to struct Scsi_Host and allowing the LLDDs to set their correct module
>> reference.
>
> Hmmm, can't we just move sht definitions to actual LLDDs?  libata can
> do that easily.  sht, as its name implies, is the template for
> creating the scsi_hosts of a given type.  We're now just moving module
> ownership from sht definition site to whatever callsite the actual
> instance is being created which can also be wrapped in a separate
> layer requiring explicit propagation.  Why not just propagate sht's
> directly?  What's the difference?

The reason I didn't move sht from the core driver to the LLDDs for
fixing ufs and ums-* in the first place is to avoid exporting many
symbols for callbacks in sht.  But I realized that we can do it
without that many exported symbols by creating a single function that
returns a kmemdup()ed sht with a few change including ->module.

So there are three options we can take for fixing this problem.
I would like to know the opinions which one should be taken.

(1) Add owner module field to Scsi_Host for allowing LLDDs to adjust
it after scsi host allocation.  This approach is used by v1,2,3 of
this patch series and the scsi midlayer change is minimum.

(2) Move owner module field from scsi_host_template to Scsi_Host.
The owner module reference is retrieved from the callsite of
scsi_host_alloc() by passing THIS_MODULE to the extra argument.
The scsi midlayer change is small, but we need the same macro trick
for each scsi_host_alloc() wrapper and these changes are relatively
large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy).
This approach is used by v4 of this patch series.

(3) Allocate scsi host template for each module.  No scsi midlayer
change is required.  Instead of sharing a single scsi host template
defined in the core, create a single function that returns a
kmemdup()ed sht with a few change including ->module so that the sub
modules can use it as their sht.

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
  2015-01-20 14:57     ` Akinobu Mita
@ 2015-01-20 15:18       ` Tejun Heo
  2015-01-20 15:20       ` Alan Stern
  1 sibling, 0 replies; 18+ messages in thread
From: Tejun Heo @ 2015-01-20 15:18 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-scsi, Vinayak Holikatti, Dolev Raviv, Sujit Reddy Thumma,
	Subhash Jadavani, Matthew Dharm, Greg Kroah-Hartman, Alan Stern,
	David S. Miller, Hannes Reinecke, Hans de Goede, Mike Christie,
	Karen Xie, Robert Love, Christoph Hellwig, James E.J. Bottomley,
	open-iscsi, fcoe-devel, linux-ide, linux-usb, usb-storage

Hello, Akinobu.

On Tue, Jan 20, 2015 at 11:57:37PM +0900, Akinobu Mita wrote:
> The reason I didn't move sht from the core driver to the LLDDs for
> fixing ufs and ums-* in the first place is to avoid exporting many
> symbols for callbacks in sht.  But I realized that we can do it
> without that many exported symbols by creating a single function that
> returns a kmemdup()ed sht with a few change including ->module.

Hmmm, libata already exports most of the necessary symbols.  libahci
or platform drivers might have to export more but that shouldn't be
much.  For libata, pushing sht's to the leaf drivers would make far
more sense as sht's already get inherited and modified along the
hierarchy.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
  2015-01-20 14:57     ` Akinobu Mita
  2015-01-20 15:18       ` Tejun Heo
@ 2015-01-20 15:20       ` Alan Stern
       [not found]         ` <Pine.LNX.4.44L0.1501201013440.1150-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
  1 sibling, 1 reply; 18+ messages in thread
From: Alan Stern @ 2015-01-20 15:20 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: Tejun Heo, linux-scsi, Vinayak Holikatti, Dolev Raviv,
	Sujit Reddy Thumma, Subhash Jadavani, Matthew Dharm,
	Greg Kroah-Hartman, David S. Miller, Hannes Reinecke,
	Hans de Goede, Mike Christie, Karen Xie, Robert Love,
	Christoph Hellwig, James E.J. Bottomley, open-iscsi, fcoe-devel,
	linux-ide, linux-usb, usb-storage

On Tue, 20 Jan 2015, Akinobu Mita wrote:

> 2015-01-19 23:22 GMT+09:00 Tejun Heo <tj@kernel.org>:
> > On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote:
> >> 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 drivers which consist with the core driver and the actual
> >> LLDDs and scsi_host_template is defined in the core driver.  In these
> >> cases, the actual LLDDs can be unloaded even if the scsi_device is
> >> being accessed.
> >>
> >> This patch series fixes the module reference mismatch problem for
> >> ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
> >> by moving owner module reference field from struct scsi_host_template
> >> to struct Scsi_Host and allowing the LLDDs to set their correct module
> >> reference.
> >
> > Hmmm, can't we just move sht definitions to actual LLDDs?  libata can
> > do that easily.  sht, as its name implies, is the template for
> > creating the scsi_hosts of a given type.  We're now just moving module
> > ownership from sht definition site to whatever callsite the actual
> > instance is being created which can also be wrapped in a separate
> > layer requiring explicit propagation.  Why not just propagate sht's
> > directly?  What's the difference?
> 
> The reason I didn't move sht from the core driver to the LLDDs for
> fixing ufs and ums-* in the first place is to avoid exporting many
> symbols for callbacks in sht.  But I realized that we can do it
> without that many exported symbols by creating a single function that
> returns a kmemdup()ed sht with a few change including ->module.
> 
> So there are three options we can take for fixing this problem.
> I would like to know the opinions which one should be taken.
> 
> (1) Add owner module field to Scsi_Host for allowing LLDDs to adjust
> it after scsi host allocation.  This approach is used by v1,2,3 of
> this patch series and the scsi midlayer change is minimum.
> 
> (2) Move owner module field from scsi_host_template to Scsi_Host.
> The owner module reference is retrieved from the callsite of
> scsi_host_alloc() by passing THIS_MODULE to the extra argument.
> The scsi midlayer change is small, but we need the same macro trick
> for each scsi_host_alloc() wrapper and these changes are relatively
> large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy).
> This approach is used by v4 of this patch series.
> 
> (3) Allocate scsi host template for each module.  No scsi midlayer
> change is required.  Instead of sharing a single scsi host template
> defined in the core, create a single function that returns a
> kmemdup()ed sht with a few change including ->module so that the sub
> modules can use it as their sht.

(3) means duplicating a reasonably large data structure in order to
alter just one field.  It also means changing all the subdrivers to
make them call the new function.

(1) is the simplest.  Since the use of subdrivers in general tends to 
be a special case (most SCSI drivers don't do it), I prefer to keep the 
code optimized for it.  In other words, I prefer option (1).  If people 
think (2) is better, it can always be layered on top of (1).

Alan Stern


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host
       [not found]         ` <Pine.LNX.4.44L0.1501201013440.1150-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
@ 2015-01-22 13:17           ` Akinobu Mita
  0 siblings, 0 replies; 18+ messages in thread
From: Akinobu Mita @ 2015-01-22 13:17 UTC (permalink / raw)
  To: Alan Stern
  Cc: Tejun Heo, linux-scsi-u79uwXL29TY76Z2rM5mHXA, Vinayak Holikatti,
	Dolev Raviv, Sujit Reddy Thumma, Subhash Jadavani, Matthew Dharm,
	Greg Kroah-Hartman, David S. Miller, Hannes Reinecke,
	Hans de Goede, Mike Christie, Karen Xie, Robert Love,
	Christoph Hellwig, James E.J. Bottomley,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	fcoe-devel-s9riP+hp16TNLxjTenLetw,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	usb-storage-ijkIwGHArpdIPJnuZ7Njw4oP9KaGy4wf

2015-01-21 0:20 GMT+09:00 Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>:
> On Tue, 20 Jan 2015, Akinobu Mita wrote:
>
>> 2015-01-19 23:22 GMT+09:00 Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>> > On Mon, Jan 19, 2015 at 12:05:58AM +0900, Akinobu Mita wrote:
>> >> 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 drivers which consist with the core driver and the actual
>> >> LLDDs and scsi_host_template is defined in the core driver.  In these
>> >> cases, the actual LLDDs can be unloaded even if the scsi_device is
>> >> being accessed.
>> >>
>> >> This patch series fixes the module reference mismatch problem for
>> >> ufs, usb-storage, esp_scsi, ahci_platform, and pata_platform drivers
>> >> by moving owner module reference field from struct scsi_host_template
>> >> to struct Scsi_Host and allowing the LLDDs to set their correct module
>> >> reference.
>> >
>> > Hmmm, can't we just move sht definitions to actual LLDDs?  libata can
>> > do that easily.  sht, as its name implies, is the template for
>> > creating the scsi_hosts of a given type.  We're now just moving module
>> > ownership from sht definition site to whatever callsite the actual
>> > instance is being created which can also be wrapped in a separate
>> > layer requiring explicit propagation.  Why not just propagate sht's
>> > directly?  What's the difference?
>>
>> The reason I didn't move sht from the core driver to the LLDDs for
>> fixing ufs and ums-* in the first place is to avoid exporting many
>> symbols for callbacks in sht.  But I realized that we can do it
>> without that many exported symbols by creating a single function that
>> returns a kmemdup()ed sht with a few change including ->module.
>>
>> So there are three options we can take for fixing this problem.
>> I would like to know the opinions which one should be taken.
>>
>> (1) Add owner module field to Scsi_Host for allowing LLDDs to adjust
>> it after scsi host allocation.  This approach is used by v1,2,3 of
>> this patch series and the scsi midlayer change is minimum.
>>
>> (2) Move owner module field from scsi_host_template to Scsi_Host.
>> The owner module reference is retrieved from the callsite of
>> scsi_host_alloc() by passing THIS_MODULE to the extra argument.
>> The scsi midlayer change is small, but we need the same macro trick
>> for each scsi_host_alloc() wrapper and these changes are relatively
>> large (required by libata, libiscsi, libfc, cxgbi, 53c700, legacy).
>> This approach is used by v4 of this patch series.
>>
>> (3) Allocate scsi host template for each module.  No scsi midlayer
>> change is required.  Instead of sharing a single scsi host template
>> defined in the core, create a single function that returns a
>> kmemdup()ed sht with a few change including ->module so that the sub
>> modules can use it as their sht.
>
> (3) means duplicating a reasonably large data structure in order to
> alter just one field.  It also means changing all the subdrivers to
> make them call the new function.
>
> (1) is the simplest.  Since the use of subdrivers in general tends to
> be a special case (most SCSI drivers don't do it), I prefer to keep the
> code optimized for it.  In other words, I prefer option (1).  If people
> think (2) is better, it can always be layered on top of (1).

OK, I agree that (1) is the simplest.

Christoph,

Option (2) required a lot of changes as this v4 shows that
scsi_host_alloc() is used by several libraries on scsi mid level in
various forms.  It is a bit different from pci_register_driver() or
similar registration interfaces.  Although they also use the same
macro trick to get a module reference, but they are called directly
or through thin and straightforward wrapper interfaces by lower
level drivers.

So, option (1) seems to be a better choice than (2), (3) for fixing
ufs, ums-*, and esp_scsi drivers.  For ahci_platform and pata_platform,
they can be fixed easily by moving sht definision to LLDDs as Tejun
suggested.  Do you think it is ok to prepare v5 of series in this way?

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-01-22 13:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-18 15:05 [PATCH v4 00/11] scsi: fix module reference mismatch for scsi host Akinobu Mita
2015-01-18 15:05 ` [PATCH v4 01/11] ata: prepare to move module reference from scsi_host_template to Scsi_Host Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 02/11] iscsi: " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 03/11] cxgbi: " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 04/11] libfc: " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 05/11] 53c700: prepare " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 06/11] scsi: legacy: prepare to " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 07/11] scsi: " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 08/11] scsi: ufs: adjust module reference for scsi host Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 09/11] usb: storage: " Akinobu Mita
2015-01-18 15:06 ` [PATCH v4 10/11] ata: ahci_platform: " Akinobu Mita
2015-01-18 15:30   ` Hans de Goede
2015-01-18 15:06 ` [PATCH v4 11/11] ata: pata_of_platform: " Akinobu Mita
2015-01-19 14:22 ` [PATCH v4 00/11] scsi: fix module reference mismatch " Tejun Heo
     [not found]   ` <20150119142206.GE8140-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2015-01-20 14:57     ` Akinobu Mita
2015-01-20 15:18       ` Tejun Heo
2015-01-20 15:20       ` Alan Stern
     [not found]         ` <Pine.LNX.4.44L0.1501201013440.1150-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2015-01-22 13:17           ` Akinobu Mita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.