Subject: Re: [ofa-general] [PATCH 09/13] QLogic VNIC: IB Multicast for Ethernet broadcast/multicast

From: Roland Dreier <[email-masked]>
Date: Thu, 15 May 2008 15:38:00 -0700

> +#define SET_MCAST_STATE_INVALID \
 > +do { \
 > +	viport->mc_info.state = MCAST_STATE_INVALID; \
 > +	viport->mc_info.mc = NULL; \
 > +	memset(&viport->mc_info.mgid, 0, sizeof(union ib_gid)); \
 > +} while (0);

Seems like this could be profitably implemented in C instead of CPP.

 > +		spin_lock_irqsave(&viport->mc_info.lock, flags);
 > +		viport->mc_info.state = MCAST_STATE_INVALID;
 > +		spin_unlock_irqrestore(&viport->mc_info.lock, flags);

This pattern makes me uneasy about the locking... setting the state
member will already be atomic, so what do you think you're protecting
against here by taking the lock?

 - R.