[openib-general] [PATCH 3/3] iWARP CM

[Tom Tucker]

On Sun, 2006-03-12 at 21:24 -0800, Sean Hefty wrote:
> >> > +	if (cm_id->device == NULL || cm_id->device->iwcm == NULL)
> >> > +		return -EINVAL;
> >>
> >> Can these be removed?
> >
> >I am concerned about someone calling disconnect when the object has
> >already been disconnected or on an idle cm_id.
> 
> The problem is that there's no protection around this check.  If a caller is
> calling disconnect twice or on an idle cm_id, then it sounds like the caller is
> broken.  If another thread can change the device or iwcm pointer, then we're
> missing needed protection.
> 
> This isn't a big deal, but it seems more likely to mask actual bugs.  I would
> rather just see synchronized checks against some sort of state value, which I
> believe that this function has.
> 
I think you're right, actually. A related problem (both peers close at
the same time) is what the additional state was created to fix
(CLOSING). The problem is that it was thrown in at the last minute and
it really needs to be thought through and done right -- this is the
exercise I'll have done by tomorrow and will repost a patch for. 

BTW, I think a state diagram would help -- where should I post that --
and how...

> - Sean
>