Re: [PATCH v2 2/2] RDMA/cm: Optimise rbtree searching

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, May 12, 2021 at 09:12:08PM +0800, Leizhen (ThunderTown) wrote:
> 
> 
> On 2021/5/12 20:50, Jason Gunthorpe wrote:
> > On Wed, May 12, 2021 at 06:05:37PM +0800, Zhen Lei wrote:
> >>  static struct cm_id_private *cm_find_listen(struct ib_device *device,
> >> @@ -686,22 +687,23 @@ static struct cm_id_private *cm_find_listen(struct ib_device *device,
> >>  
> >>  	while (node) {
> >>  		cm_id_priv = rb_entry(node, struct cm_id_private, service_node);
> >> -		if ((cm_id_priv->id.service_mask & service_id) ==
> >> -		     cm_id_priv->id.service_id &&
> >> -		    (cm_id_priv->id.device == device)) {
> >> -			refcount_inc(&cm_id_priv->refcount);
> >> -			return cm_id_priv;
> >> -		}
> >> +
> >>  		if (device < cm_id_priv->id.device)
> >>  			node = node->rb_left;
> >>  		else if (device > cm_id_priv->id.device)
> >>  			node = node->rb_right;
> >> +		else if ((cm_id_priv->id.service_mask & service_id) == cm_id_priv->id.service_id)
> >> +			goto found;
> >>  		else if (be64_lt(service_id, cm_id_priv->id.service_id))
> >>  			node = node->rb_left;
> >>  		else
> >>  			node = node->rb_right;
> >>  	}
> > 
> > This is not the pattern I showed you. Drop the first patch and rely on
> > the implicit equality in the final else.
> 
> Do you mean treate the "found" process as the else branch?
> 
> But ((cm_id_priv->id.service_mask & service_id) ==
> cm_id_priv->id.service_id) is different from (service_id ==
> cm_id_priv->id.service_id),I'm just worried that it might change
> the original logic.

The service_mask is always ~cpu_to_be64(0), it is some non-working
dead code that has been left in here.

If you really want to touch this then you should have a prep patch to
remove that entire API facet, then the above will make sense.

Jason