patch for 2.4.22 sbp2 hang when loaded with devices already connected

patch for 2.4.22 sbp2 hang when loaded with devices already connected

[Alexandre Oliva]

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

We were free()ing a packet before sbp2_agent_reset() had a chance to
wake up on its semaphore, which would often cause sbp2 to hang in
`initializing' state if it was loaded when firewire devices were
already connected to the host.  Details of the symptoms at
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=103821

This patch fixes it.  I guess Ben likes it because he sent me a very
similar patch to test just as I was finishing testing this one :-)

Thanks, Ben, for helping me figure out what was going on!

Please Cc: me on replies, as I'm not subscribed.  In fact, I'm not
even sure my posting will make it to the lists.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sbp2-init.patch --]
[-- Type: text/x-patch, Size: 2285 bytes --]

--- /usr/src/linux-2.4/drivers/ieee1394/sbp2.c	2003-08-25 08:44:42.000000000 -0300
+++ drivers/ieee1394/sbp2.c	2003-10-20 22:16:17.000000000 -0200
@@ -372,8 +372,10 @@ static int sbp2util_down_timeout(atomic_
 }
 
 /* Free's an allocated packet */
-static void sbp2_free_packet(struct hpsb_packet *packet)
+static void sbp2_free_packet(void *packet_)
 {
+	struct hpsb_packet *packet = packet_;
+
 	hpsb_free_tlabel(packet);
 	free_hpsb_packet(packet);
 }
@@ -389,7 +391,8 @@ static struct hpsb_packet *
 sbp2util_allocate_write_packet(struct sbp2scsi_host_info *hi,
 			       struct node_entry *ne, u64 addr,
 			       size_t data_size,
-			       quadlet_t *data)
+			       quadlet_t *data,
+			       int keep)
 {
 	struct hpsb_packet *packet;
 
@@ -399,8 +402,10 @@ sbp2util_allocate_write_packet(struct sb
         if (!packet)
                 return NULL;
 
-	hpsb_set_packet_complete_task(packet, (void (*)(void*))sbp2_free_packet,
-				      packet);
+	if (!keep)
+		hpsb_set_packet_complete_task(packet,
+					      sbp2_free_packet,
+					      packet);
 
 	hpsb_node_fill_packet(ne, packet);
 
@@ -1763,7 +1768,7 @@ static int sbp2_agent_reset(struct scsi_
 	packet = sbp2util_allocate_write_packet(hi, scsi_id->ne,
 						scsi_id->sbp2_command_block_agent_addr +
 						SBP2_AGENT_RESET_OFFSET,
-						4, &data);
+						4, &data, wait);
 
 	if (!packet) {
 		SBP2_ERR("sbp2util_allocate_write_packet failed");
@@ -1779,6 +1784,7 @@ static int sbp2_agent_reset(struct scsi_
 	if (wait) {
 		down(&packet->state_change);
 		down(&packet->state_change);
+		sbp2_free_packet(packet); 
 	}
 
 	/*
@@ -2073,7 +2079,7 @@ static int sbp2_link_orb_command(struct 
 
 			packet = sbp2util_allocate_write_packet(hi, scsi_id->ne,
 								scsi_id->sbp2_command_block_agent_addr +
-								SBP2_ORB_POINTER_OFFSET, 8, NULL);
+								SBP2_ORB_POINTER_OFFSET, 8, NULL, 0);
 		
 			if (!packet) {
 				SBP2_ERR("sbp2util_allocate_write_packet failed");
@@ -2123,7 +2129,7 @@ static int sbp2_link_orb_command(struct 
 
 			packet = sbp2util_allocate_write_packet(hi, scsi_id->ne,
 					scsi_id->sbp2_command_block_agent_addr +
-					SBP2_DOORBELL_OFFSET, 4, &data);
+					SBP2_DOORBELL_OFFSET, 4, &data, 0);
 	
 			if (!packet) {
 				SBP2_ERR("sbp2util_allocate_write_packet failed");

[-- Attachment #3: Type: text/plain, Size: 289 bytes --]


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: patch for 2.4.22 sbp2 hang when loaded with devices already connected

[Ben Collins]

On Mon, Oct 20, 2003 at 10:40:23PM -0200, Alexandre Oliva wrote:
> We were free()ing a packet before sbp2_agent_reset() had a chance to
> wake up on its semaphore, which would often cause sbp2 to hang in
> `initializing' state if it was loaded when firewire devices were
> already connected to the host.  Details of the symptoms at
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=103821
> 
> This patch fixes it.  I guess Ben likes it because he sent me a very
> similar patch to test just as I was finishing testing this one :-)
> 
> Thanks, Ben, for helping me figure out what was going on!

Please let me send the patch to Marcelo. I have to get this into our
repo, and merge things around back to Marcelo, else it makes things
harder later.

Thanks for following through with the debug to track this down.

-- 
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
WatchGuard - http://www.watchguard.com/

Re: patch for 2.4.22 sbp2 hang when loaded with devices already connected

[Alexandre Oliva]

On Oct 20, 2003, Ben Collins <bcollins@debian.org> wrote:

> Please let me send the patch to Marcelo. I have to get this into our
> repo, and merge things around back to Marcelo, else it makes things
> harder later.

Sure, do however you prefer.  Sorry that I wasn't aware of the
procedure, and this ws the first patch I've ever submitted for the
kernel, woohoo! :-)

I just asked around where patches for 2.4 were supposed to be sent,
and got Marcelo's e-mail.  Then I noticed another firewire-related
patch mentioned in the same bug report in Red Hat's bugzilla, saw it
had been posted to these two mailing lists, and thought I'd do that as
well.

Thanks again, your help was extremely useful in nailing the problem
down.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

Re: patch for 2.4.22 sbp2 hang when loaded with devices already connected

[Ben Collins]

On Mon, Oct 20, 2003 at 11:37:36PM -0200, Alexandre Oliva wrote:
> On Oct 20, 2003, Ben Collins <bcollins@debian.org> wrote:
> 
> > Please let me send the patch to Marcelo. I have to get this into our
> > repo, and merge things around back to Marcelo, else it makes things
> > harder later.
> 
> Sure, do however you prefer.  Sorry that I wasn't aware of the
> procedure, and this ws the first patch I've ever submitted for the
> kernel, woohoo! :-)
> 
> I just asked around where patches for 2.4 were supposed to be sent,
> and got Marcelo's e-mail.  Then I noticed another firewire-related
> patch mentioned in the same bug report in Red Hat's bugzilla, saw it
> had been posted to these two mailing lists, and thought I'd do that as
> well.
> 
> Thanks again, your help was extremely useful in nailing the problem
> down.

No problem. Generally, Marcelo is the right place for 2.4, but sometimes
it's better to filter things through the subsystem maintainer if at all
possible. For something trivial, I wouldn't have bothered asking to
redirect through me, but for something like this I want to make sure we
get on the same page, and that I test it out a little bit more (I hadn't
even tested this patch myself yet :)

Definitely thanks again for the help. This is the second bug recently
that I could not reproduce and it takes feedback and effort like you
gave me in order to track things down.

FYI, I checked, and this is a non-issue with our current 2.6 code (not
sure it's pushed to Linus yet) because we removed several points of
problems like this.

-- 
Debian     - http://www.debian.org/
Linux 1394 - http://www.linux1394.org/
Subversion - http://subversion.tigris.org/
WatchGuard - http://www.watchguard.com/