[ofa-general] [PATCH 1/2] IB/core: handle race between elements in qork queues after event

[Moni Shoua]

Roland Dreier wrote:
> By the way:
> 
>  > +		struct ib_sa_sm_ah *sm_ah;
>  > +
>  > +		spin_lock_irqsave(&port->ah_lock, flags);
>  > +		sm_ah = port->sm_ah;
>  > +		port->sm_ah = NULL;
>  > +		spin_unlock_irqrestore(&port->ah_lock, flags);
>  > +
>  > +		if (sm_ah)
>  > +			kref_put(&sm_ah->ref, free_sm_ah);
> 
> Is there some reason why this can't be simpler like:
> 
> 		spin_lock_irqsave(&port->ah_lock, flags);
> 		if (port->sm_ah)
> 			kref_put(&port->sm_ah->ref, free_sm_ah);
> 		port->sm_ah = NULL;
> 		spin_unlock_irqrestore(&port->ah_lock, flags);
> 
What happens if this happens

# |         CPU-0					|	CPU-1
  |      						|
1 | if (port->sm_ah)					|
  |      kref_put(&port->sm_ah->ref, free_sm_ah);	|
--+-----------------------------------------------------+-----------------------
2 |							| alloc_mad() 
--+-----------------------------------------------------+-----------------------
3 | port->sm_ah = NULL;					|

As I see it, process on CPU-1 gets a garbage sm_ah
Do you agree?
> I guess the same cleanup applies to update_sm_ah(), except after your
> patch I don't see any way that update_sm_ah() could be called with sm_ah
> anything but NULL, so we could drop the old_ah stuff completely there.
I agree. The cleanup code can be completely removed.
> 
>  - R.
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>