[openib-general] [PATCH fixed] was Re: [PATCH] ipoib_multicast/ipoib_mcast_send race

[Krishna Kumar2]

Hi Michael,

I see what you are doing with that lock :-) But isn't the lock a hack ? 
Eg, I could instead
do this (If I can make sure the redundant lock/unlock is not optimized 
out) and it would
still work :

stop_thread()
{
        clear_bit();
        lock(); /* empty lock/unlock to synchronize with the mcast_send() 
*/
        unlock(); /* make the other routine FINISH before we start other 
activity */
        ...
        ...
}
mcast_send()
{
        lock();
        if (test_bit)
        ...
        ...
        unlock();
}

So basically, the lock is not required for clearing (and absolutely not 
required for
setting) the bit, but a lock is required before we start waiting, to 
enable us to
synchronize with any sends. The lock is being used as a signalling 
mechanism
between two processes (in this case, lock/unlock is a mechanism for the 
mcast_send()
to finish if running).

Thanks,

- KK
 
"Michael S. Tsirkin" <mst at mellanox.co.il> wrote on 12/12/2005 12:26:23 PM:

> Quoting r. Krishna Kumar2 <krkumar2 at in.ibm.com>:
> > Isn't all that managed by clearing/testing the bit ? Because holding 
the
> > lock doesn't solve 
> > it.
> > To give an example : 
> > 
> > stop_thread() 
> > { 
> >         lock(); 
> >         clear(); 
> >         unlock(); 
> >         ... 
> >         wait_for_completion(mcast); 
> > } 
> > 
> > mcast_send() 
> > { 
> >         lock(); 
> >         test(); 
> >         results_in_creation_of_entries_done_etc();; 
> >         unlock(); 
> > } 
> > 
> > In this case, if mcast_send() gets the lock first and proceeds while 
the
> > stop_thread is spinning 
> > on the lock, the entries are created and then the stop_thread() clears
> > the bit and at this point 
> > in time, no more entries can be ever created. Now if the lock were
> > removed, the behavior 
> > is identical - the mcast_send() would test the bit, and get the lock()
> > while the stop_thread() 
> > clears the bit (without a lock) and waits for completion, while *no
> > more* mcast_sends() would 
> > ever continue beyond this time. 
> 
> Now mcast_send can call init_completion *after* stop_thread does wait 
for
> completion.
> It could also call list_add while mcast_stop_thread walks the list.
> 
> Thats what I am trying to prevent.
> 
> 
> -- 
> MST
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20051212/5fc33f8f/attachment.html>