<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META http-equiv=Content-Type content="text/html; charset=us-ascii">
<META content="MSHTML 6.00.2900.2802" name=GENERATOR></HEAD>
<BODY>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006>Hi
Fab,</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006>I think that the
code in the function __get_mad_element has a bug.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006>Here is the
problematic code:</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006> /* Obtain a
MAD item from the stack. */<BR> cl_spinlock_acquire(
&h_pool->obj.lock );<BR> p_mad_item = cl_qlist_remove_head(
&h_pool->mad_stack );<BR> p_mad_element = PARENT_STRUCT( p_mad_item,
al_mad_element_t, list_item );<BR> p_mad = PARENT_STRUCT( p_mad_element,
mad_item_t, al_mad_element );<BR> if( p_mad_item == cl_qlist_end(
&h_pool->mad_stack ) )<BR> {<BR> /* The stack was
empty. Grow the pool and obtain a new item.
*/<BR> cl_spinlock_release( &h_pool->obj.lock
);<BR> status = __grow_mad_pool( h_pool, &p_mad
);<BR> if( status != IB_SUCCESS
)<BR> {<BR> CL_TRACE_EXIT( AL_DBG_ERROR,
g_al_dbg_lvl,<BR> ("grow_mad_pool failed with status
%s.\n",<BR> ib_get_err_str(status))
);<BR> return
status;<BR> }<BR> }<BR> else<BR> {<BR> cl_spinlock_release(
&h_pool->obj.lock );<BR> }<BR></SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006>As you can see, if
there are no items in the pool, the condition in if( p_mad_item == cl_qlist_end(
&h_pool->mad_stack ) )<BR>will be true, and we will try to enter the if.
The comment there says that /* The stack was empty. Grow the
pool and obtain a new item. */. However, IMHO, this is not the correct
implementation since after the pool has grown, we don't ask for a new item
from the pool. </SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN class=634322113-04042006>I also think, that
the approach that cl_qlist_remove_head returns one item passed the end, and if
we enlarge the pool this object will be valid is also not correct since the lock
is being freed and only later taken inside __grow_mad_pool which might already
be too late.</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006></SPAN></FONT> </DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006>Thanks</SPAN></FONT></DIV>
<DIV><FONT face=Arial size=2><SPAN
class=634322113-04042006>Tzachi</SPAN></FONT></DIV></BODY></HTML>