<br><font size=2 face="sans-serif">Hi Michael,</font>
<br>
<br><font size=2><tt>> Exactly, you got it. If you look at mcast_send
you'll see that it creates new<br>
> queries, creates broad cast group and adds entries to the list.</tt></font>
<br>
<br><font size=2><tt>Correct, but even with the lock, it would create those
(once, and that is true for</tt></font>
<br><font size=2><tt>whether lock is held or not). The only thing stopping
creation of those is setting</tt></font>
<br><font size=2><tt>the bit (but only for a. the non-race part or b. the
race part where the stop_thread</tt></font>
<br><font size=2><tt>executes before the mcast_send(), but not the race
part where the mcast_send()</tt></font>
<br><font size=2><tt>wins over the stop_thread), but holding the lock for
the setting/testing of that bit</tt></font>
<br><font size=2><tt>will not stop creation of those entries in the b.
case.</tt></font>
<br>
<br><font size=2><tt>> This cancels the query and waits on mcast "done"
completion.<br>
> completion is called and "done" is set.</tt></font>
<br><font size=2><tt>> Meanwhile, ipoib_mcast_send arrives and starts
a new query,<br>
> re-initializing "done".</tt></font>
<br>
<br><font size=2><tt>Isn't all that managed by clearing/testing the bit
? Because holding the lock doesn't solve</tt></font>
<br><font size=2><tt>it. To give an example :</tt></font>
<br>
<br><font size=2><tt>stop_thread()</tt></font>
<br><font size=2><tt>{</tt></font>
<br><font size=2><tt>        lock();</tt></font>
<br><font size=2><tt>        clear();</tt></font>
<br><font size=2><tt>        unlock();</tt></font>
<br><font size=2><tt>        ...</tt></font>
<br><font size=2><tt>        wait_for_completion(mcast);</tt></font>
<br><font size=2><tt>}</tt></font>
<br>
<br><font size=2><tt>mcast_send()</tt></font>
<br><font size=2><tt>{</tt></font>
<br><font size=2><tt>        lock();</tt></font>
<br><font size=2><tt>        test();</tt></font>
<br><font size=2><tt>        results_in_creation_of_entries_done_etc();;</tt></font>
<br><font size=2><tt>        unlock();</tt></font>
<br><font size=2><tt>}</tt></font>
<br>
<br><font size=2 face="sans-serif">In this case, if mcast_send() gets the
lock first and proceeds while the stop_thread is spinning</font>
<br><font size=2 face="sans-serif">on the lock, the entries are created
and then the stop_thread() clears the bit and at this point</font>
<br><font size=2 face="sans-serif">in time, no more entries can be ever
created. Now if the lock were removed, the behavior</font>
<br><font size=2 face="sans-serif">is identical - the mcast_send() would
test the bit, and get the lock() while the stop_thread()</font>
<br><font size=2 face="sans-serif">clears the bit (without a lock) and
waits for completion, while *no more* mcast_sends() would</font>
<br><font size=2 face="sans-serif">ever continue beyond this time.</font>
<br>
<br><font size=2 face="sans-serif">thanks,</font>
<br>
<br><font size=2 face="sans-serif">- KK</font>
<br>
<br><font size=2><tt>"Michael S. Tsirkin" <mst@mellanox.co.il>
wrote on 12/12/2005 11:42:13 AM:<br>
<br>
> Quoting Krishna Kumar2 <krkumar2@in.ibm.com>:<br>
> > Subject: Re: [openib-general] [PATCH fixed] was Re: [PATCH]?<br>
> ipoib_multicast/ipoib_mcast_send race<br>
> > <br>
> > <br>
> > Hi Micheal, <br>
> > <br>
> > But the lock doesn't help that case. The only difference with
having the<br>
> > <br>
> > lock is that in case of a race, the mcast_send() will complete
*before* <br>
> > the flag is set, while without the lock the mcast_send() could
be in the<br>
> > <br>
> > *middle* of execution when the flag is set.<br>
> <br>
> Exactly, you got it. If you look at mcast_send you'll see that it
creates new<br>
> queries, creates broad cast group and adds entries to the list.<br>
> <br>
> So here's why the lock helps:<br>
> <br>
> > > > > > Fix the following race scenario:<br>
> > > > > > device is up.<br>
> > > > > > port event or set mcast list triggers ipoib_mcast_stop_thread,
<br>
> > > > > > This cancels the query and waits on mcast
"done" completion.<br>
> > > > > > completion is called and "done"
is set.<br>
> > > > > > Meanwhile, ipoib_mcast_send arrives and starts
a new query,<br>
> > > > > > re-initializing "done".<br>
> <br>
> Clear now?<br>
> <br>
> -- <br>
> MST<br>
</tt></font>