All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] libsas: export sas_alloc_task()
@ 2011-07-30  0:26 Dan Williams
  2011-08-31 13:44 ` [PATCH] [SCSI]: Allow expander T-T attachments Mark Salyzyn
  0 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2011-07-30  0:26 UTC (permalink / raw)
  To: JBottomley; +Cc: Lindar Liu, Xiangliang Yu, Ankit Jain, linux-scsi, Jack Wang

Now that isci has added a 3rd open coded user of this functionality just
share the libsas version.

Acked-by: Jack Wang <jack_wang@usish.com>
Cc: Lindar Liu <lindar_liu@usish.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Ankit Jain <jankit@suse.de>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

Updated with Jack's ack, and now includes the removal of the open-coded
sas_alloc_task in isci.

 drivers/scsi/isci/task.c         |   29 +++--------------------------
 drivers/scsi/libsas/sas_init.c   |   30 +++++++++++++++++++++++++++---
 drivers/scsi/mvsas/mv_sas.c      |   29 +++--------------------------
 drivers/scsi/pm8001/pm8001_sas.c |   38 ++++++--------------------------------
 include/scsi/libsas.h            |   26 ++------------------------
 5 files changed, 41 insertions(+), 111 deletions(-)

diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index d6bcdd0..e2d9418 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1345,29 +1345,6 @@ static void isci_smp_task_done(struct sas_task *task)
 	complete(&task->completion);
 }
 
-static struct sas_task *isci_alloc_task(void)
-{
-	struct sas_task *task = kzalloc(sizeof(*task), GFP_KERNEL);
-
-	if (task) {
-		INIT_LIST_HEAD(&task->list);
-		spin_lock_init(&task->task_state_lock);
-		task->task_state_flags = SAS_TASK_STATE_PENDING;
-		init_timer(&task->timer);
-		init_completion(&task->completion);
-	}
-
-	return task;
-}
-
-static void isci_free_task(struct isci_host *ihost, struct sas_task  *task)
-{
-	if (task) {
-		BUG_ON(!list_empty(&task->list));
-		kfree(task);
-	}
-}
-
 static int isci_smp_execute_task(struct isci_host *ihost,
 				 struct domain_device *dev, void *req,
 				 int req_size, void *resp, int resp_size)
@@ -1376,7 +1353,7 @@ static int isci_smp_execute_task(struct isci_host *ihost,
 	struct sas_task *task = NULL;
 
 	for (retry = 0; retry < 3; retry++) {
-		task = isci_alloc_task();
+		task = sas_alloc_task(GFP_KERNEL);
 		if (!task)
 			return -ENOMEM;
 
@@ -1439,13 +1416,13 @@ static int isci_smp_execute_task(struct isci_host *ihost,
 				SAS_ADDR(dev->sas_addr),
 				task->task_status.resp,
 				task->task_status.stat);
-			isci_free_task(ihost, task);
+			sas_free_task(task);
 			task = NULL;
 		}
 	}
 ex_err:
 	BUG_ON(retry == 3 && task != NULL);
-	isci_free_task(ihost, task);
+	sas_free_task(task);
 	return res;
 }
 
diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c
index 2dc5534..dd56ea8 100644
--- a/drivers/scsi/libsas/sas_init.c
+++ b/drivers/scsi/libsas/sas_init.c
@@ -37,7 +37,32 @@
 
 #include "../scsi_sas_internal.h"
 
-struct kmem_cache *sas_task_cache;
+static struct kmem_cache *sas_task_cache;
+
+struct sas_task *sas_alloc_task(gfp_t flags)
+{
+	struct sas_task *task = kmem_cache_zalloc(sas_task_cache, flags);
+
+	if (task) {
+		INIT_LIST_HEAD(&task->list);
+		spin_lock_init(&task->task_state_lock);
+		task->task_state_flags = SAS_TASK_STATE_PENDING;
+		init_timer(&task->timer);
+		init_completion(&task->completion);
+	}
+
+	return task;
+}
+EXPORT_SYMBOL_GPL(sas_alloc_task);
+
+void sas_free_task(struct sas_task *task)
+{
+	if (task) {
+		BUG_ON(!list_empty(&task->list));
+		kmem_cache_free(sas_task_cache, task);
+	}
+}
+EXPORT_SYMBOL_GPL(sas_free_task);
 
 /*------------ SAS addr hash -----------*/
 void sas_hash_addr(u8 *hashed, const u8 *sas_addr)
@@ -293,8 +318,7 @@ EXPORT_SYMBOL_GPL(sas_domain_release_transport);
 
 static int __init sas_class_init(void)
 {
-	sas_task_cache = kmem_cache_create("sas_task", sizeof(struct sas_task),
-					   0, SLAB_HWCACHE_ALIGN, NULL);
+	sas_task_cache = KMEM_CACHE(sas_task, SLAB_HWCACHE_ALIGN);
 	if (!sas_task_cache)
 		return -ENOMEM;
 
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 0ef2742..7921b61 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -1518,28 +1518,6 @@ void mvs_dev_gone(struct domain_device *dev)
 	mvs_dev_gone_notify(dev);
 }
 
-static  struct sas_task *mvs_alloc_task(void)
-{
-	struct sas_task *task = kzalloc(sizeof(struct sas_task), GFP_KERNEL);
-
-	if (task) {
-		INIT_LIST_HEAD(&task->list);
-		spin_lock_init(&task->task_state_lock);
-		task->task_state_flags = SAS_TASK_STATE_PENDING;
-		init_timer(&task->timer);
-		init_completion(&task->completion);
-	}
-	return task;
-}
-
-static  void mvs_free_task(struct sas_task *task)
-{
-	if (task) {
-		BUG_ON(!list_empty(&task->list));
-		kfree(task);
-	}
-}
-
 static void mvs_task_done(struct sas_task *task)
 {
 	if (!del_timer(&task->timer))
@@ -1564,7 +1542,7 @@ static int mvs_exec_internal_tmf_task(struct domain_device *dev,
 	struct sas_task *task = NULL;
 
 	for (retry = 0; retry < 3; retry++) {
-		task = mvs_alloc_task();
+		task = sas_alloc_task(GFP_KERNEL);
 		if (!task)
 			return -ENOMEM;
 
@@ -1622,15 +1600,14 @@ static int mvs_exec_internal_tmf_task(struct domain_device *dev,
 				    SAS_ADDR(dev->sas_addr),
 				    task->task_status.resp,
 				    task->task_status.stat);
-			mvs_free_task(task);
+			sas_free_task(task);
 			task = NULL;
 
 		}
 	}
 ex_err:
 	BUG_ON(retry == 3 && task != NULL);
-	if (task != NULL)
-		mvs_free_task(task);
+	sas_free_task(task);
 	return res;
 }
 
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 6ae059e..7dbbf8b 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -669,30 +669,6 @@ int pm8001_dev_found(struct domain_device *dev)
 	return pm8001_dev_found_notify(dev);
 }
 
-/**
-  * pm8001_alloc_task - allocate a task structure for TMF
-  */
-static struct sas_task *pm8001_alloc_task(void)
-{
-	struct sas_task *task = kzalloc(sizeof(*task), GFP_KERNEL);
-	if (task) {
-		INIT_LIST_HEAD(&task->list);
-		spin_lock_init(&task->task_state_lock);
-		task->task_state_flags = SAS_TASK_STATE_PENDING;
-		init_timer(&task->timer);
-		init_completion(&task->completion);
-	}
-	return task;
-}
-
-static void pm8001_free_task(struct sas_task *task)
-{
-	if (task) {
-		BUG_ON(!list_empty(&task->list));
-		kfree(task);
-	}
-}
-
 static void pm8001_task_done(struct sas_task *task)
 {
 	if (!del_timer(&task->timer))
@@ -728,7 +704,7 @@ static int pm8001_exec_internal_tmf_task(struct domain_device *dev,
 	struct pm8001_hba_info *pm8001_ha = pm8001_find_ha_by_dev(dev);
 
 	for (retry = 0; retry < 3; retry++) {
-		task = pm8001_alloc_task();
+		task = sas_alloc_task(GFP_KERNEL);
 		if (!task)
 			return -ENOMEM;
 
@@ -789,14 +765,13 @@ static int pm8001_exec_internal_tmf_task(struct domain_device *dev,
 				SAS_ADDR(dev->sas_addr),
 				task->task_status.resp,
 				task->task_status.stat));
-			pm8001_free_task(task);
+			sas_free_task(task);
 			task = NULL;
 		}
 	}
 ex_err:
 	BUG_ON(retry == 3 && task != NULL);
-	if (task != NULL)
-		pm8001_free_task(task);
+	sas_free_task(task);
 	return res;
 }
 
@@ -811,7 +786,7 @@ pm8001_exec_internal_task_abort(struct pm8001_hba_info *pm8001_ha,
 	struct sas_task *task = NULL;
 
 	for (retry = 0; retry < 3; retry++) {
-		task = pm8001_alloc_task();
+		task = sas_alloc_task(GFP_KERNEL);
 		if (!task)
 			return -ENOMEM;
 
@@ -864,14 +839,13 @@ pm8001_exec_internal_task_abort(struct pm8001_hba_info *pm8001_ha,
 				SAS_ADDR(dev->sas_addr),
 				task->task_status.resp,
 				task->task_status.stat));
-			pm8001_free_task(task);
+			sas_free_task(task);
 			task = NULL;
 		}
 	}
 ex_err:
 	BUG_ON(retry == 3 && task != NULL);
-	if (task != NULL)
-		pm8001_free_task(task);
+	sas_free_task(task);
 	return res;
 }
 
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ee86606..2517254 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -555,36 +555,14 @@ struct sas_task {
 	struct work_struct abort_work;
 };
 
-extern struct kmem_cache *sas_task_cache;
-
 #define SAS_TASK_STATE_PENDING      1
 #define SAS_TASK_STATE_DONE         2
 #define SAS_TASK_STATE_ABORTED      4
 #define SAS_TASK_NEED_DEV_RESET     8
 #define SAS_TASK_AT_INITIATOR       16
 
-static inline struct sas_task *sas_alloc_task(gfp_t flags)
-{
-	struct sas_task *task = kmem_cache_zalloc(sas_task_cache, flags);
-
-	if (task) {
-		INIT_LIST_HEAD(&task->list);
-		spin_lock_init(&task->task_state_lock);
-		task->task_state_flags = SAS_TASK_STATE_PENDING;
-		init_timer(&task->timer);
-		init_completion(&task->completion);
-	}
-
-	return task;
-}
-
-static inline void sas_free_task(struct sas_task *task)
-{
-	if (task) {
-		BUG_ON(!list_empty(&task->list));
-		kmem_cache_free(sas_task_cache, task);
-	}
-}
+extern struct sas_task *sas_alloc_task(gfp_t flags);
+extern void sas_free_task(struct sas_task *task);
 
 struct sas_domain_function_template {
 	/* The class calls these to notify the LLDD of an event. */


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH] [SCSI]: libsas failure to revalidate domain for anything but the first expander child.
@ 2011-09-19 16:55 Mark Salyzyn
  2011-09-20  5:30 ` Jack Wang
  2011-09-20  6:17 ` Xiangliang Yu
  0 siblings, 2 replies; 16+ messages in thread
From: Mark Salyzyn @ 2011-09-19 16:55 UTC (permalink / raw)
  To: linux-scsi
  Cc: Luben Tuikov, Darrick J Wong, James Bottomley, Xiangliang Yu,
	Jack Wang, Dan Williams

[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]

Three weeks, and this simple and obvious patch has had two positive
reviews and no propagation. Who do I have to buy a Pint of Guinness for
to get this moving upstream? ;-}

Sincerely -- Mark Salyzyn

-----Original Message-----
From: dan.j.williams@gmail.com [mailto:dan.j.williams@gmail.com] On
Behalf Of Dan Williams
Sent: Wednesday, September 07, 2011 3:26 PM
To: Mark Salyzyn
Cc: linux-scsi@vger.kernel.org; Luben Tuikov; Darrick J Wong; James
Bottomley; Xiangliang Yu; Jack Wang
Subject: Re: [PATCH] [SCSI]: libsas failure to revalidate domain for
anything but the first expander child.

. . .

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

. . .

Reviewed-by: Luben Tuikov <ltuikov@yahoo.com>
I don't mind this patch.

______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
 
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
 
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
 
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
 
\r

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: expander_bug.patch --]
[-- Type: text/x-patch; name="expander_bug.patch", Size: 1867 bytes --]

From e3569cf6e71f9435af8476c6729624cf32870326 Mon Sep 17 00:00:00 2001
From: Mark Salyzyn <mark_salyzyn@us.xyratex.com>
Date: Thu, 1 Sep 2011 06:11:17 -0700
Subject: [PATCH] libsas: failure to revalidate domain for anything but the
 first expander child

In an enclosure model where there are chaining expanders to a large body
of storage, it was discovered that libsas, responding to a broadcast
event change, would only revalidate the domain of first child expander
in the list.

The issue is that the pointer value to the discovered source device was
used to break out of the loop, rather than the content of the pointer.

This still remains non-compliant as the revalidate domain code is
supposed to loop through all child expanders, and not stop at the first
one it finds that reports a change count. However, the design of this
routine does not allow multiple device discoveries and that would be a
more complicated set of patches reserved for another day. We are fixing
the glaring bug rather than refactoring the code.

Cc: Luben Tuikov <tuikov@yahoo.com>
Cc: Darrick J Wong <djwong@us.ibm.com>
Cc: James Bottomley <jbottomley@parallels.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Mark Salyzyn <msalyzyn@us.xyratex.com>
---
 drivers/scsi/libsas/sas_expander.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
index 874e29d..7d68517 100644
--- a/drivers/scsi/libsas/sas_expander.c
+++ b/drivers/scsi/libsas/sas_expander.c
@@ -1718,7 +1718,7 @@ static int sas_find_bcast_dev(struct domain_device *dev,
 	list_for_each_entry(ch, &ex->children, siblings) {
 		if (ch->dev_type == EDGE_DEV || ch->dev_type == FANOUT_DEV) {
 			res = sas_find_bcast_dev(ch, src_dev);
-			if (src_dev)
+			if (*src_dev)
 				return res;
 		}
 	}
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-09-20  6:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-30  0:26 [PATCH v3] libsas: export sas_alloc_task() Dan Williams
2011-08-31 13:44 ` [PATCH] [SCSI]: Allow expander T-T attachments Mark Salyzyn
2011-08-31 17:43   ` Luben Tuikov
2011-08-31 17:51     ` Mark Salyzyn
2011-09-01 15:34       ` James Bottomley
2011-09-01 15:58         ` Mark Salyzyn
2011-09-06 16:47         ` Luben Tuikov
2011-09-01 17:25       ` Luben Tuikov
2011-09-01 13:11   ` [PATCH] [SCSI]: libsas failure to revalidate domain for anything but the first expander child Mark Salyzyn
2011-09-01 17:18     ` Luben Tuikov
2011-09-06 14:56       ` Mark Salyzyn
2011-09-07 17:03         ` Luben Tuikov
2011-09-07 19:26     ` Dan Williams
2011-09-19 16:55 Mark Salyzyn
2011-09-20  5:30 ` Jack Wang
2011-09-20  6:17 ` Xiangliang Yu

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.