[patch 2.6.27.y 0/6] firewire: proposed patches for -stable

[patch 2.6.27.y 0/6] firewire: proposed patches for -stable

[Stefan Richter]

Hi -stable folks,

please consider to add the following fixes for the 2.6.27.y series.
Coming up as replies:

1/6  firewire: fix setting tag and sy in iso transmission
2/6  firewire: fix ioctl() return code
3/6  firewire: Survive more than 256 bus resets
4/6  firewire: fix struct fw_node memory leak
5/6  firewire: fw-sbp2: delay first login to avoid retries
6/6  firewire: fw-sbp2: fix races

 drivers/firewire/fw-cdev.c        |    6 ++--
 drivers/firewire/fw-sbp2.c        |   38 +++++++++++++++++++++---------
 drivers/firewire/fw-topology.c    |    6 +++-
 drivers/firewire/fw-transaction.h |    2 -
 4 files changed, 35 insertions(+), 17 deletions(-)

Thanks,
-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 1/6] firewire: fix setting tag and sy in iso transmission

[Stefan Richter]

Date: Fri, 12 Sep 2008 18:09:55 +0200 (CEST)
From: Stefan Richter <stefanr@s5r6.in-berlin.de>

Reported by Jay Fenlason:
The iso packet control accessors in fw-cdev.c had bogus masks.

Same as commit 7a1003449c693f0d57443c8786bbf19717921ae0.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -720,8 +720,8 @@ static int ioctl_create_iso_context(stru
 #define GET_PAYLOAD_LENGTH(v)	((v) & 0xffff)
 #define GET_INTERRUPT(v)	(((v) >> 16) & 0x01)
 #define GET_SKIP(v)		(((v) >> 17) & 0x01)
-#define GET_TAG(v)		(((v) >> 18) & 0x02)
-#define GET_SY(v)		(((v) >> 20) & 0x04)
+#define GET_TAG(v)		(((v) >> 18) & 0x03)
+#define GET_SY(v)		(((v) >> 20) & 0x0f)
 #define GET_HEADER_LENGTH(v)	(((v) >> 24) & 0xff)
 
 static int ioctl_queue_iso(struct client *client, void *buffer)

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 2/6] firewire: fix ioctl() return code

[Stefan Richter]

Date: Fri, 12 Sep 2008 18:20:16 +0200 (CEST)
From: Stefan Richter <stefanr@s5r6.in-berlin.de>

Reported by Jay Fenlason:  ioctl() did not return as intended
  - the size of data read into ioctl_send_request,
  - the number of datagrams enqueued by ioctl_queue_iso.

Same as commit 99692f71ee04c6f249d0bf6a581359f32f409a38.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-cdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/firewire/fw-cdev.c
===================================================================
--- linux.orig/drivers/firewire/fw-cdev.c
+++ linux/drivers/firewire/fw-cdev.c
@@ -913,7 +913,7 @@ dispatch_ioctl(struct client *client, un
 			return -EFAULT;
 	}
 
-	return 0;
+	return retval;
 }
 
 static long

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 3/6] firewire: Survive more than 256 bus resets

[Stefan Richter]

Date: Thu, 16 Oct 2008 15:51:59 -0400
From: Jay Fenlason <fenlason@redhat.com>

The "color" is used during the topology building after a bus reset,
hovever in "struct fw_node"s it is stored in a u8, but in struct fw_card
it is stored in an int.  When the value wraps in one struct, but not
the other, disaster strikes.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>

Fixes http://bugzilla.kernel.org/show_bug.cgi?id=10922 -
machine locks up solid if a series of bus resets occurs.

Same as commit 4f9740d4f5a17fa6a1b097fa3ccdfb7246660307.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-transaction.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/firewire/fw-transaction.h
===================================================================
--- linux.orig/drivers/firewire/fw-transaction.h
+++ linux/drivers/firewire/fw-transaction.h
@@ -248,7 +248,7 @@ struct fw_card {
 	struct fw_node *local_node;
 	struct fw_node *root_node;
 	struct fw_node *irm_node;
-	int color;
+	u8 color; /* must be u8 to match the definition in struct fw_node */
 	int gap_count;
 	bool beta_repeaters_present;
 

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 4/6] firewire: fix struct fw_node memory leak

[Stefan Richter]

Date: Thu, 16 Oct 2008 18:00:15 -0400
From: Jay Fenlason <fenlason@redhat.com>

With the bus_resets patch applied, it is easy to see this memory leak
by repeatedly resetting the firewire bus while running slabtop in
another window.  Just watch kmalloc-32 grow and grow...

Same as commit 77e557191701afa55ae7320d42ad6458a2ad292e.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-topology.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux/drivers/firewire/fw-topology.c
===================================================================
--- linux.orig/drivers/firewire/fw-topology.c
+++ linux/drivers/firewire/fw-topology.c
@@ -413,7 +413,7 @@ static void
 update_tree(struct fw_card *card, struct fw_node *root)
 {
 	struct list_head list0, list1;
-	struct fw_node *node0, *node1;
+	struct fw_node *node0, *node1, *next1;
 	int i, event;
 
 	INIT_LIST_HEAD(&list0);
@@ -485,7 +485,9 @@ update_tree(struct fw_card *card, struct
 		}
 
 		node0 = fw_node(node0->link.next);
-		node1 = fw_node(node1->link.next);
+		next1 = fw_node(node1->link.next);
+		fw_node_put(node1);
+		node1 = next1;
 	}
 }
 

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 5/6] firewire: fw-sbp2: delay first login to avoid retries

[Stefan Richter]

Date: Wed, 22 Oct 2008 00:28:36 +0200 (CEST)
From: Stefan Richter <stefanr@s5r6.in-berlin.de>

This optimizes firewire-sbp2's device probe for the case that the local
node and the SBP-2 node were discovered at the same time.  In this case,
fw-core's bus management work and fw-sbp2's login and SCSI probe work
are scheduled in parallel (in the globally shared workqueue and in
fw-sbp2's workqueue, respectively).  The bus reset from fw-core may then
disturb and extremely delay the login and SCSI probe because the latter
fails with several command timeouts and retries and has to be retried
from scratch.

We avoid this particular situation of sbp2_login() and fw_card_bm_work()
running in parallel by delaying the first sbp2_login() a little bit.

This is meant to be a short-term fix for
https://bugzilla.redhat.com/show_bug.cgi?id=466679.  In the long run,
the SCSI probe, i.e. fw-sbp2's call of __scsi_add_device(), should be
parallelized with sbp2_reconnect().

Problem reported and fix tested and confirmed by Alex Kanavin.

Same as commit 0dcfeb7e3c8695c5aa3677dda8efb9bef2e7e64d.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-sbp2.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/firewire/fw-sbp2.c
===================================================================
--- linux.orig/drivers/firewire/fw-sbp2.c
+++ linux/drivers/firewire/fw-sbp2.c
@@ -1158,7 +1158,7 @@ static int sbp2_probe(struct device *dev
 
 	/* Do the login in a workqueue so we can easily reschedule retries. */
 	list_for_each_entry(lu, &tgt->lu_list, link)
-		sbp2_queue_work(lu, 0);
+		sbp2_queue_work(lu, DIV_ROUND_UP(HZ, 5));
 	return 0;
 
  fail_tgt_put:

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/

[patch 2.6.27.y 6/6] firewire: fw-sbp2: fix races

[Stefan Richter]

Date: Fri, 24 Oct 2008 15:26:20 -0400
From: Jay Fenlason <fenlason@redhat.com>

1: There is a small race between queue_delayed_work() and its
   corresponding kref_get().  Do the kref_get first, and _put it again
   if the queue_delayed_work() failed, so there is no chance of the
   kref going to zero while the work is scheduled.
2: An SBP2_LOGOUT_REQUEST could be sent out with a login_id full of
   garbage.  Initialize it to an invalid value so we can tell if we
   ever got a valid login_id.
3: The node ID and generation may have changed but the new values may
   not yet have been recorded in lu and tgt when the final logout is
   attempted.  Use the latest values from the device in
   sbp2_release_target().

Same as commit cd1f70fdb4823c97328a1f151f328eb36fafd579.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-sbp2.c |   36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

Index: linux/drivers/firewire/fw-sbp2.c
===================================================================
--- linux.orig/drivers/firewire/fw-sbp2.c
+++ linux/drivers/firewire/fw-sbp2.c
@@ -172,6 +172,9 @@ struct sbp2_target {
 	int blocked;	/* ditto */
 };
 
+/* Impossible login_id, to detect logout attempt before successful login */
+#define INVALID_LOGIN_ID 0x10000
+
 /*
  * Per section 7.4.8 of the SBP-2 spec, a mgt_ORB_timeout value can be
  * provided in the config rom. Most devices do provide a value, which
@@ -791,9 +794,20 @@ static void sbp2_release_target(struct k
 			scsi_remove_device(sdev);
 			scsi_device_put(sdev);
 		}
-		sbp2_send_management_orb(lu, tgt->node_id, lu->generation,
-				SBP2_LOGOUT_REQUEST, lu->login_id, NULL);
-
+		if (lu->login_id != INVALID_LOGIN_ID) {
+			int generation, node_id;
+			/*
+			 * tgt->node_id may be obsolete here if we failed
+			 * during initial login or after a bus reset where
+			 * the topology changed.
+			 */
+			generation = device->generation;
+			smp_rmb(); /* node_id vs. generation */
+			node_id    = device->node_id;
+			sbp2_send_management_orb(lu, node_id, generation,
+						 SBP2_LOGOUT_REQUEST,
+						 lu->login_id, NULL);
+		}
 		fw_core_remove_address_handler(&lu->address_handler);
 		list_del(&lu->link);
 		kfree(lu);
@@ -808,19 +822,20 @@ static void sbp2_release_target(struct k
 
 static struct workqueue_struct *sbp2_wq;
 
+static void sbp2_target_put(struct sbp2_target *tgt)
+{
+	kref_put(&tgt->kref, sbp2_release_target);
+}
+
 /*
  * Always get the target's kref when scheduling work on one its units.
  * Each workqueue job is responsible to call sbp2_target_put() upon return.
  */
 static void sbp2_queue_work(struct sbp2_logical_unit *lu, unsigned long delay)
 {
-	if (queue_delayed_work(sbp2_wq, &lu->work, delay))
-		kref_get(&lu->tgt->kref);
-}
-
-static void sbp2_target_put(struct sbp2_target *tgt)
-{
-	kref_put(&tgt->kref, sbp2_release_target);
+	kref_get(&lu->tgt->kref);
+	if (!queue_delayed_work(sbp2_wq, &lu->work, delay))
+		sbp2_target_put(lu->tgt);
 }
 
 static void
@@ -993,6 +1008,7 @@ static int sbp2_add_logical_unit(struct 
 
 	lu->tgt      = tgt;
 	lu->lun      = lun_entry & 0xffff;
+	lu->login_id = INVALID_LOGIN_ID;
 	lu->retries  = 0;
 	lu->has_sdev = false;
 	lu->blocked  = false;

-- 
Stefan Richter
-=====-==--- =-=- ==-==
http://arcgraph.de/sr/