From: Dan Williams <dan.j.williams@intel.com>
To: linux-scsi@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Subject: [PATCH 06/24] libsas: fix domain_device leak
Date: Fri, 16 Dec 2011 18:33:39 -0800 [thread overview]
Message-ID: <20111217023339.15036.16390.stgit@localhost6.localdomain6> (raw)
In-Reply-To: <20111217022912.15036.85808.stgit@localhost6.localdomain6>
Arrange for the deallocation of a struct domain_device object when it no
longer has:
1/ any children
2/ references by any scsi_targets
3/ references by a lldd
The comment about domain_device lifetime in
Documentation/scsi/libsas.txt is stale as it appears mainline never had
a version of a struct domain_device that was registered as a kobject.
We now manage domain_device reference counts on behalf of external
agents.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/scsi/libsas.txt | 15 ---------------
drivers/scsi/libsas/sas_discover.c | 36 +++++++++++++++++++++++------------
drivers/scsi/libsas/sas_expander.c | 10 ++++++----
drivers/scsi/libsas/sas_internal.h | 19 ++++++++++++++++++
drivers/scsi/libsas/sas_scsi_host.c | 6 ++++--
include/scsi/libsas.h | 1 +
6 files changed, 54 insertions(+), 33 deletions(-)
diff --git a/Documentation/scsi/libsas.txt b/Documentation/scsi/libsas.txt
index aa54f54..3cc9c78 100644
--- a/Documentation/scsi/libsas.txt
+++ b/Documentation/scsi/libsas.txt
@@ -398,21 +398,6 @@ struct sas_task {
task_done -- callback when the task has finished execution
};
-When an external entity, entity other than the LLDD or the
-SAS Layer, wants to work with a struct domain_device, it
-_must_ call kobject_get() when getting a handle on the
-device and kobject_put() when it is done with the device.
-
-This does two things:
- A) implements proper kfree() for the device;
- B) increments/decrements the kref for all players:
- domain_device
- all domain_device's ... (if past an expander)
- port
- host adapter
- pci device
- and up the ladder, etc.
-
DISCOVERY
---------
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 54a5199..4e64930 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -36,8 +36,6 @@
void sas_init_dev(struct domain_device *dev)
{
- INIT_LIST_HEAD(&dev->siblings);
- INIT_LIST_HEAD(&dev->dev_list_node);
switch (dev->dev_type) {
case SAS_END_DEV:
break;
@@ -73,14 +71,14 @@ static int sas_get_port_device(struct asd_sas_port *port)
struct sas_rphy *rphy;
struct domain_device *dev;
- dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ dev = sas_alloc_device();
if (!dev)
return -ENOMEM;
spin_lock_irqsave(&port->phy_list_lock, flags);
if (list_empty(&port->phy_list)) {
spin_unlock_irqrestore(&port->phy_list_lock, flags);
- kfree(dev);
+ sas_put_device(dev);
return -ENODEV;
}
phy = container_of(port->phy_list.next, struct asd_sas_phy, port_phy_el);
@@ -130,7 +128,7 @@ static int sas_get_port_device(struct asd_sas_port *port)
}
if (!rphy) {
- kfree(dev);
+ sas_put_device(dev);
return -ENODEV;
}
rphy->identify.phy_identifier = phy->phy->identify.phy_identifier;
@@ -173,6 +171,7 @@ int sas_notify_lldd_dev_found(struct domain_device *dev)
dev_name(sas_ha->dev),
SAS_ADDR(dev->sas_addr), res);
}
+ kref_get(&dev->kref);
}
return res;
}
@@ -184,8 +183,10 @@ void sas_notify_lldd_dev_gone(struct domain_device *dev)
struct Scsi_Host *shost = sas_ha->core.shost;
struct sas_internal *i = to_sas_internal(shost->transportt);
- if (i->dft->lldd_dev_gone)
+ if (i->dft->lldd_dev_gone) {
i->dft->lldd_dev_gone(dev);
+ sas_put_device(dev);
+ }
}
/* ---------- Common/dispatchers ---------- */
@@ -219,6 +220,20 @@ out_err2:
/* ---------- Device registration and unregistration ---------- */
+void sas_free_device(struct kref *kref)
+{
+ struct domain_device *dev = container_of(kref, typeof(*dev), kref);
+
+ if (dev->parent)
+ sas_put_device(dev->parent);
+
+ /* remove the phys and ports, everything else should be gone */
+ if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV)
+ kfree(dev->ex_dev.ex_phy);
+
+ kfree(dev);
+}
+
static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_device *dev)
{
sas_notify_lldd_dev_gone(dev);
@@ -230,6 +245,8 @@ static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_d
spin_lock_irq(&port->dev_list_lock);
list_del_init(&dev->dev_list_node);
spin_unlock_irq(&port->dev_list_lock);
+
+ sas_put_device(dev);
}
void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
@@ -239,11 +256,6 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
sas_rphy_delete(dev->rphy);
dev->rphy = NULL;
}
- if (dev->dev_type == EDGE_DEV || dev->dev_type == FANOUT_DEV) {
- /* remove the phys and ports, everything else should be gone */
- kfree(dev->ex_dev.ex_phy);
- dev->ex_dev.ex_phy = NULL;
- }
sas_unregister_common_dev(port, dev);
}
@@ -322,7 +334,7 @@ static void sas_discover_domain(struct work_struct *work)
list_del_init(&dev->dev_list_node);
spin_unlock_irq(&port->dev_list_lock);
- kfree(dev); /* not kobject_register-ed yet */
+ sas_put_device(dev);
port->port_dev = NULL;
}
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 1b831c5..15d2239 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -657,10 +657,11 @@ static struct domain_device *sas_ex_discover_end_dev(
if (phy->attached_sata_host || phy->attached_sata_ps)
return NULL;
- child = kzalloc(sizeof(*child), GFP_KERNEL);
+ child = sas_alloc_device();
if (!child)
return NULL;
+ kref_get(&parent->kref);
child->parent = parent;
child->port = parent->port;
child->iproto = phy->attached_iproto;
@@ -762,7 +763,7 @@ static struct domain_device *sas_ex_discover_end_dev(
sas_port_delete(phy->port);
out_err:
phy->port = NULL;
- kfree(child);
+ sas_put_device(child);
return NULL;
}
@@ -809,7 +810,7 @@ static struct domain_device *sas_ex_discover_expander(
phy->attached_phy_id);
return NULL;
}
- child = kzalloc(sizeof(*child), GFP_KERNEL);
+ child = sas_alloc_device();
if (!child)
return NULL;
@@ -835,6 +836,7 @@ static struct domain_device *sas_ex_discover_expander(
child->rphy = rphy;
edev = rphy_to_expander_device(rphy);
child->dev_type = phy->attached_dev_type;
+ kref_get(&parent->kref);
child->parent = parent;
child->port = port;
child->iproto = phy->attached_iproto;
@@ -858,7 +860,7 @@ static struct domain_device *sas_ex_discover_expander(
spin_lock_irq(&parent->port->dev_list_lock);
list_del(&child->dev_list_node);
spin_unlock_irq(&parent->port->dev_list_lock);
- kfree(child);
+ sas_put_device(child);
return NULL;
}
list_add_tail(&child->siblings, &parent->ex_dev.children);
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 14e21b5..0d43408 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -76,6 +76,8 @@ struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy);
void sas_hae_reset(struct work_struct *work);
+void sas_free_device(struct kref *kref);
+
#ifdef CONFIG_SCSI_SAS_HOST_SMP
extern int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req,
struct request *rsp);
@@ -161,4 +163,21 @@ static inline void sas_add_parent_port(struct domain_device *dev, int phy_id)
sas_port_add_phy(ex->parent_port, ex_phy->phy);
}
+static inline struct domain_device *sas_alloc_device(void)
+{
+ struct domain_device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+
+ if (dev) {
+ INIT_LIST_HEAD(&dev->siblings);
+ INIT_LIST_HEAD(&dev->dev_list_node);
+ kref_init(&dev->kref);
+ }
+ return dev;
+}
+
+static inline void sas_put_device(struct domain_device *dev)
+{
+ kref_put(&dev->kref, sas_free_device);
+}
+
#endif /* _SAS_INTERNAL_H_ */
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index e95e5e1..4636453 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -758,6 +758,7 @@ int sas_target_alloc(struct scsi_target *starget)
return res;
}
+ kref_get(&found_dev->kref);
starget->hostdata = found_dev;
return 0;
}
@@ -1047,7 +1048,7 @@ int sas_slave_alloc(struct scsi_device *scsi_dev)
void sas_target_destroy(struct scsi_target *starget)
{
- struct domain_device *found_dev = sas_find_target(starget);
+ struct domain_device *found_dev = starget->hostdata;
if (!found_dev)
return;
@@ -1055,7 +1056,8 @@ void sas_target_destroy(struct scsi_target *starget)
if (dev_is_sata(found_dev))
ata_sas_port_destroy(found_dev->sata_dev.ap);
- return;
+ starget->hostdata = NULL;
+ sas_put_device(found_dev);
}
static void sas_parse_addr(u8 *sas_addr, const char *p)
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 2b14348..7ecb5c1 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -206,6 +206,7 @@ struct domain_device {
void *lldd_dev;
int gone;
+ struct kref kref;
};
struct sas_discovery_event {
next prev parent reply other threads:[~2011-12-17 2:33 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-17 2:33 [PATCH 00/24] libsas: eh reworks (ata-eh vs discovery, races, ...) Dan Williams
2011-12-17 2:33 ` [PATCH 01/24] workqueue: promote workqueue_lock to hard-irq safe Dan Williams
2011-12-17 2:33 ` [PATCH 02/24] workqueue: defer work to a draining queue Dan Williams
2011-12-19 21:49 ` Tejun Heo
2011-12-19 22:01 ` Dan Williams
2011-12-19 22:08 ` Tejun Heo
2011-12-19 22:25 ` Williams, Dan J
2011-12-17 2:33 ` [PATCH 03/24] scsi: use drain_workqueue Dan Williams
2011-12-17 2:33 ` [PATCH 04/24] libsas: remove unused ata_task_resp fields Dan Williams
2011-12-17 12:51 ` Sergei Shtylyov
2011-12-19 1:38 ` Jack Wang
2011-12-17 2:33 ` [PATCH 05/24] libsas: kill sas_slave_destroy Dan Williams
2011-12-17 2:33 ` Dan Williams [this message]
2011-12-19 2:32 ` [PATCH 06/24] libsas: fix domain_device leak Jack Wang
2011-12-17 2:33 ` [PATCH 07/24] libsas: fix leak of dev->sata_dev.identify_[packet_]device Dan Williams
2011-12-19 2:38 ` Jack Wang
2011-12-17 2:33 ` [PATCH 08/24] libsas: replace event locks with atomic bitops Dan Williams
2011-12-17 2:33 ` [PATCH 09/24] libsas: remove ata_port.lock management duties from lldds Dan Williams
2011-12-17 2:33 ` [PATCH 10/24] libsas: prevent domain rediscovery competing with ata error handling Dan Williams
2011-12-17 2:34 ` [PATCH 11/24] libsas: use ->set_dmamode to notify lldds of NCQ parameters Dan Williams
2011-12-17 2:34 ` [PATCH 12/24] libsas: kill invocation of scsi_eh_finish_cmd from sas_ata_task_done Dan Williams
2011-12-17 2:34 ` [PATCH 13/24] libsas: close error handling vs sas_ata_task_done() race Dan Williams
2011-12-17 2:34 ` [PATCH 14/24] libsas: prevent double completion of scmds from eh Dan Williams
2011-12-17 13:08 ` Sergei Shtylyov
2011-12-18 19:19 ` Dan Williams
2011-12-17 13:13 ` Sergei Shtylyov
2011-12-18 19:24 ` Dan Williams
2011-12-17 2:34 ` [PATCH 15/24] libsas: fix timeout vs completion race Dan Williams
2011-12-17 2:34 ` [PATCH 16/24] libsas: let libata handle command timeouts Dan Williams
2011-12-17 2:34 ` [PATCH 17/24] libsas: defer SAS_TASK_NEED_DEV_RESET commands to libata Dan Williams
2011-12-17 2:34 ` [PATCH 18/24] libsas: use libata-eh-reset for sata rediscovery fis transmit failures Dan Williams
2011-12-17 2:34 ` [PATCH 19/24] libsas: execute transport link resets with libata-eh via host workqueue Dan Williams
2011-12-17 2:34 ` [PATCH 20/24] libsas: sas_phy_enable via transport_sas_phy_reset Dan Williams
2011-12-17 2:34 ` [PATCH 21/24] libsas: Remove redundant phy state notification calls Dan Williams
2011-12-17 2:35 ` [PATCH 22/24] libsas: add mutex for SMP task execution Dan Williams
2011-12-17 2:35 ` [PATCH 23/24] libsas: async ata-eh Dan Williams
2011-12-17 2:35 ` [PATCH 24/24] libsas: poll for ata device readiness after reset Dan Williams
2011-12-20 6:18 ` Jack Wang
2011-12-20 7:08 ` Williams, Dan J
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111217023339.15036.16390.stgit@localhost6.localdomain6 \
--to=dan.j.williams@intel.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.