> +static void queue_join(struct mcast_member *member)
> +{
> + struct mcast_group *group = member->group;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&group->lock, flags);
> + list_add(&member->list, &group->pending_list);
> + if (group->state == MCAST_IDLE) {
> + group->state = MCAST_BUSY;
> + spin_unlock_irqrestore(&group->lock, flags);
> + atomic_inc(&group->refcount);
> + queue_work(mcast_wq, &group->work);
> + } else
> + spin_unlock_irqrestore(&group->lock, flags);
> +}
should the atomic_inc() be outside the lock here? It seems that
leaves a window for things to go bad. It might be simpler to just do
something like:
spin_lock_irqsave(&group->lock, flags);
list_add(&member->list, &group->pending_list);
if (group->state == MCAST_IDLE) {
group->state = MCAST_BUSY;
atomic_inc(&group->refcount);
queue_work(mcast_wq, &group->work);
}
spin_unlock_irqrestore(&group->lock, flags);
> +static void adjust_membership(struct mcast_group *group, u8 join_state, int
> inc)
> +{
> + int i;
> +
> + for (i = 0; i < 3; i++, join_state >>= 1)
> + if (join_state & 0x1)
> + group->members[i] += inc;
> +}
> +
> +static u8 get_leave_state(struct mcast_group *group)
> +{
> + u8 leave_state = 0;
> + int i;
> +
> + for (i = 0; i < 3; i++)
> + if (!group->members[i])
> + leave_state |= (0x1 << i);
> +
> + return leave_state & group->rec.join_state;
> +}
These look rather magical -- perhaps a comment to make them understandable?
> + case MCAST_JOINING:
> + list_del_init(&member->list);
> + break;
Why not just list_del()? I don't see anywhere that this list_head is
used after this.
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
