<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=US-ASCII">
<META NAME="Generator" CONTENT="MS Exchange Server version 5.5.2654.45">
<TITLE>RE: [PATCHv2] OpenSM: OpenIB vendor layer: Implement osm_vendor_delete</TITLE>
</HEAD>
<BODY>
<P><FONT SIZE=2>Hi Hal,</FONT>
<BR><FONT SIZE=2>There is a problem with the patch.</FONT>
<BR><FONT SIZE=2>1. In osm_vendor_unbind: You used free(p_bind), when the pointer was allocated using cl_zalloc.</FONT>
<BR><FONT SIZE=2> You need to use cl_free.</FONT>
<BR><FONT SIZE=2>2. There is a race between the cl_free and the receiver thread. We get a segmentation fault due to the fact that the thread isn't destroyed before freeing the p_bind object. There should be a way to signal the reciever thread to exit, and the unbind should wait for that thread to join.</FONT></P>
<P><FONT SIZE=2>Thanks,</FONT>
<BR><FONT SIZE=2>Yael</FONT>
</P>
<P><FONT SIZE=2>-----Original Message-----</FONT>
<BR><FONT SIZE=2>From: Hal Rosenstock [<A HREF="mailto:halr@voltaire.com">mailto:halr@voltaire.com</A>]</FONT>
<BR><FONT SIZE=2>Sent: Tuesday, September 06, 2005 5:27 PM</FONT>
<BR><FONT SIZE=2>To: Yael Kalka; Eitan Zahavi</FONT>
<BR><FONT SIZE=2>Cc: openib-general@openib.org</FONT>
<BR><FONT SIZE=2>Subject: [PATCHv2] OpenSM: OpenIB vendor layer: Implement</FONT>
<BR><FONT SIZE=2>osm_vendor_delete</FONT>
</P>
<BR>
<P><FONT SIZE=2>[same patch just generated with diff -up]</FONT>
</P>
<P><FONT SIZE=2>OpenSM: OpenIB vendor layer: Implement osm_vendor_delete</FONT>
</P>
<P><FONT SIZE=2>[I've done some testing of this; are there any regressions for this ?]</FONT>
</P>
<P><FONT SIZE=2>Signed-off-by: Hal Rosenstock <halr@voltaire.com></FONT>
</P>
<P><FONT SIZE=2>--- osm_vendor_ibumad.c.1 2005-08-31 12:26:03.000000000 -0400</FONT>
<BR><FONT SIZE=2>+++ osm_vendor_ibumad.c 2005-09-06 09:35:27.000000000 -0400</FONT>
<BR><FONT SIZE=2>@@ -483,14 +483,8 @@ void</FONT>
<BR><FONT SIZE=2> osm_vendor_delete(</FONT>
<BR><FONT SIZE=2> IN osm_vendor_t** const pp_vend )</FONT>
<BR><FONT SIZE=2> {</FONT>
<BR><FONT SIZE=2>- int agent_id;</FONT>
<BR><FONT SIZE=2>-</FONT>
<BR><FONT SIZE=2>- /* unregister UMAD agents */</FONT>
<BR><FONT SIZE=2>- for (agent_id = 0; agent_id < UMAD_CA_MAX_AGENTS; agent_id++)</FONT>
<BR><FONT SIZE=2>- if ( (*pp_vend)->agents[agent_id] )</FONT>
<BR><FONT SIZE=2>- umad_unregister( (*pp_vend)->umad_port_id, agent_id );</FONT>
<BR><FONT SIZE=2> clear_madw( *pp_vend );</FONT>
<BR><FONT SIZE=2>- /* make sure all ports are closed? */</FONT>
<BR><FONT SIZE=2>+ /* make sure all ports are closed */</FONT>
<BR><FONT SIZE=2> umad_done();</FONT>
<BR><FONT SIZE=2> cl_free( *pp_vend );</FONT>
<BR><FONT SIZE=2> *pp_vend = NULL;</FONT>
<BR><FONT SIZE=2>@@ -593,7 +587,7 @@ Exit:</FONT>
<BR><FONT SIZE=2> </FONT>
<BR><FONT SIZE=2> /**********************************************************************</FONT>
<BR><FONT SIZE=2> **********************************************************************/</FONT>
<BR><FONT SIZE=2>-int</FONT>
<BR><FONT SIZE=2>+static int</FONT>
<BR><FONT SIZE=2> osm_vendor_open_port(</FONT>
<BR><FONT SIZE=2> IN osm_vendor_t* const p_vend,</FONT>
<BR><FONT SIZE=2> IN const ib_net64_t port_guid )</FONT>
<BR><FONT SIZE=2>@@ -828,11 +822,27 @@ osm_vendor_unbind(</FONT>
<BR><FONT SIZE=2> IN osm_bind_handle_t h_bind)</FONT>
<BR><FONT SIZE=2> {</FONT>
<BR><FONT SIZE=2> osm_umad_bind_info_t *p_bind = ( osm_umad_bind_info_t * ) h_bind;</FONT>
<BR><FONT SIZE=2>- osm_vendor_t *p_vend = p_bind->p_vend;</FONT>
<BR><FONT SIZE=2>+ osm_vendor_t *p_vend;</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ if (p_bind) {</FONT>
<BR><FONT SIZE=2>+ p_vend = p_bind->p_vend;</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ OSM_LOG_ENTER( p_vend->p_log, osm_vendor_unbind );</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ /* Unregister UMAD agents */</FONT>
<BR><FONT SIZE=2>+ if (p_vend->agents[p_bind->agent_id1])</FONT>
<BR><FONT SIZE=2>+ umad_unregister(p_bind->port_id, p_bind->agent_id1);</FONT>
<BR><FONT SIZE=2>+ if (p_vend->agents[p_bind->agent_id])</FONT>
<BR><FONT SIZE=2>+ umad_unregister(p_bind->port_id, p_bind->agent_id);</FONT>
<BR><FONT SIZE=2> </FONT>
<BR><FONT SIZE=2>- OSM_LOG_ENTER( p_vend->p_log, osm_vendor_unbind );</FONT>
<BR><FONT SIZE=2>+ /* close port ??? */</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ free(p_bind);</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ OSM_LOG_EXIT( p_vend->p_log);</FONT>
<BR><FONT SIZE=2>+</FONT>
<BR><FONT SIZE=2>+ }</FONT>
<BR><FONT SIZE=2> </FONT>
<BR><FONT SIZE=2>- OSM_LOG_EXIT( p_vend->p_log);</FONT>
<BR><FONT SIZE=2> }</FONT>
<BR><FONT SIZE=2> </FONT>
<BR><FONT SIZE=2> /**********************************************************************</FONT>
</P>
</BODY>
</HTML>