On 2021/5/12 22:26, Jason Gunthorpe wrote:
> 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.
Okay, I'll try.
>
> Jason
>
> .
>
