[Openib-windows] SRP __ioc_query_sa error flow

[Fabian Tillier]

Hi Yossi,

On 9/6/06, Fabian Tillier <ftillier at silverstorm.com> wrote:
> Hi Yossi,
>
> On 9/6/06, Yossi Leybovich <sleybo at mellanox.co.il> wrote:
> >
> > Fab
> >
> > In the begging of __ioc_query_sa function you check
> > if( p_svc->h_node_query )
> >   return IB_NOT_DONE;
> >
> >  if( p_svc->h_path_query )
> >   return IB_NOT_DONE;
> >  I think that when ib_query fail we should set p_svc->h_node_query = NULL so
> > in the next time the sweep will try to query this mad_svc (this port).
> > The current code does not do  that
> >
> >
> >  status = ib_query( gh_al, &query, &p_svc->h_node_query );
> >  if( status != IB_SUCCESS )
> >  {
> >   cl_atomic_dec( &p_svc->query_cnt );
> >   deref_al_obj( &p_svc->obj );
> >   AL_PRINT_EXIT( TRACE_LEVEL_WARNING, AL_DBG_PNP,
> >    ("ib_query returned %s\n", ib_get_err_str( status )) );
> >   return status;
> >  }
> >
> > What do you think ?
>
> There's no need to clear the handle - if ib_query fails, the input
> handles is not modified.  As that handles is NULL when the call is
> made, the handle is still NULL when after the call fails.

There was some code to set the handle before the status was known, and
it didn't clear the handle in case of failure.  This was done to allow
synchronous callers to cancel the query, but there's a race between
the call returning a failure status (after the structure was freed)
and the user calling cancel with the (now freed) handle.  I moved the
code that sets the query handle to only set in the case of success.

Committed in revision 489.

Thanks for reporting this.

- Fab