On Thu, Sep 23, 2021 at 08:49:06AM +0300, Leon Romanovsky wrote:
> On Wed, Sep 22, 2021 at 11:41:19AM -0300, Jason Gunthorpe wrote:
> > On Wed, Sep 22, 2021 at 11:01:39AM +0300, Leon Romanovsky wrote:
> >
> > > > + /* The FSM can return back to RDMA_CM_ADDR_BOUND after
> > > > + * rdma_resolve_ip() is called, eg through the error
> > > > + * path in addr_handler. If this happens the existing
> > > > + * request must be canceled before issuing a new one.
> > > > + */
> > > > + if (id_priv->used_resolve_ip)
> > > > + rdma_addr_cancel(&id->route.addr.dev_addr);
> > > > + else
> > > > + id_priv->used_resolve_ip = 1;
> > >
> > > Why don't you never clear this field?
> >
> > The only case where it can be cleared is if we have called
> > rdma_addr_cancel(), and since this is the only place that does it and
> > immediately calls rdma_resolve_ip() again, there is no reason to ever
> > clear it.
>
> IMHO, it is better to clear instead to rely on "the only place" semantic.
Then the code looks really silly:
if (id_priv->used_resolve_ip) {
rdma_addr_cancel(&id->route.addr.dev_addr);
id_priv->used_resolve_ip = 0;
}
id_priv->used_resolve_ip = 1;
Jason
