Re: [openib-general] Re: [git patch review 1/4] IPoIB: Don't start send-only joins while multicast thread is stopped

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting r. Roland Dreier <[email protected]>:
> Subject: [openib-general] Re: [git patch review 1/4] IPoIB: Don't start send-only joins while multicast thread is stopped
> 
>  > Roland Dreier <[email protected]> wrote:
>  > >
>  > >  +	spin_lock_irq(&priv->lock);
>  > >  +	set_bit(IPOIB_MCAST_STARTED, &priv->flags);
>  > >  +	spin_unlock_irq(&priv->lock);
>  > 
>  > Strange to put a lock around an atomic op like that.
>  > 
>  > Sometimes it's valid.   If another cpu was doing:
>  > 
>  > 	spin_lock(lock);
>  > 
>  > 	if (test_bit(IPOIB_MCAST_STARTED))
>  > 		something();
>  > 	...
>  > 	if (test_bit(IPOIB_MCAST_STARTED))
>  > 		something_else();
>  > 
>  > 	spin_unlock(lock);
>  > 
>  > then the locked set_bit() makes sense.
>  > 
>  > But often it doesn't ;)
> 
> Good point.  Michael, any reason why the lock is there around the
> set_bit()?  (And similarly for the corresponding clear_bit())
> 
> Thanks,
>  Roland

Basically, its as Andrew said: the lock around clear_bit is there to ensure that
ipoib_mcast_send isnt running already when we stop the thread.  Thats why
test_bit has to be inside the lock, too.

This was discussed with Krishna Kumar when I posted the patch originally.
For more detail, please review this thread:
http://www.mail-archive.com/[email protected]/msg13206.html
or here
http://openib.org/pipermail/openib-general/2005-December/014370.html


-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux