| Message ID | 20220819090859.957943-3-markzhang@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | IB/cm refactors | expand |
On Fri, Aug 19, 2022 at 12:08:58PM +0300, Mark Zhang wrote: > The service_mask is always ~cpu_to_be64(0), so the result is always > a NOP when it is &'d with a service_id. Remove it for simplicity. > > Signed-off-by: Mark Zhang <markzhang@nvidia.com> > --- > drivers/infiniband/core/cm.c | 28 ++++++++-------------------- > include/rdma/ib_cm.h | 1 - > 2 files changed, 8 insertions(+), 21 deletions(-) > > diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c > index b59f864b3d79..84bb10799467 100644 > --- a/drivers/infiniband/core/cm.c > +++ b/drivers/infiniband/core/cm.c > @@ -617,7 +617,6 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, > struct rb_node *parent = NULL; > struct cm_id_private *cur_cm_id_priv; > __be64 service_id = cm_id_priv->id.service_id; > - __be64 service_mask = cm_id_priv->id.service_mask; > unsigned long flags; > > spin_lock_irqsave(&cm.lock, flags); > @@ -625,8 +624,7 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, > parent = *link; > cur_cm_id_priv = rb_entry(parent, struct cm_id_private, > service_node); > - if ((cur_cm_id_priv->id.service_mask & service_id) == > - (service_mask & cur_cm_id_priv->id.service_id) && > + if ((service_id == cur_cm_id_priv->id.service_id) && > (cm_id_priv->id.device == cur_cm_id_priv->id.device)) { > /* > * Sharing an ib_cm_id with different handlers is not > @@ -670,8 +668,7 @@ 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 && > + if ((service_id == cm_id_priv->id.service_id) && > (cm_id_priv->id.device == device)) { > refcount_inc(&cm_id_priv->refcount); > return cm_id_priv; > @@ -1158,22 +1155,17 @@ void ib_destroy_cm_id(struct ib_cm_id *cm_id) > } > EXPORT_SYMBOL(ib_destroy_cm_id); > > -static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id, > - __be64 service_mask) > +static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id) > { > - service_mask = service_mask ? service_mask : ~cpu_to_be64(0); > - service_id &= service_mask; > if ((service_id & IB_SERVICE_ID_AGN_MASK) == IB_CM_ASSIGN_SERVICE_ID && > (service_id != IB_CM_ASSIGN_SERVICE_ID)) > return -EINVAL; > > - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { > + if (service_id == IB_CM_ASSIGN_SERVICE_ID) > cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); > - cm_id_priv->id.service_mask = ~cpu_to_be64(0); > - } else { > + else > cm_id_priv->id.service_id = service_id; > - cm_id_priv->id.service_mask = service_mask; > - } If service_id != IB_CM_ASSIGN_SERVICE_ID, we had zero as service_mask and not FFF... like you wrote. It puts in question all cm_id_priv->id.service_mask & service_id => service_id conversions in this patch. Thanks > + > return 0; > } > > @@ -1199,7 +1191,7 @@ int ib_cm_listen(struct ib_cm_id *cm_id, __be64 service_id) > goto out; > } > > - ret = cm_init_listen(cm_id_priv, service_id, 0); > + ret = cm_init_listen(cm_id_priv, service_id); > if (ret) > goto out; > > @@ -1247,7 +1239,7 @@ struct ib_cm_id *ib_cm_insert_listen(struct ib_device *device, > if (IS_ERR(cm_id_priv)) > return ERR_CAST(cm_id_priv); > > - err = cm_init_listen(cm_id_priv, service_id, 0); > + err = cm_init_listen(cm_id_priv, service_id); > if (err) { > ib_destroy_cm_id(&cm_id_priv->id); > return ERR_PTR(err); > @@ -1518,7 +1510,6 @@ int ib_send_cm_req(struct ib_cm_id *cm_id, > } > } > cm_id->service_id = param->service_id; > - cm_id->service_mask = ~cpu_to_be64(0); > cm_id_priv->timeout_ms = cm_convert_to_ms( > param->primary_path->packet_life_time) * 2 + > cm_convert_to_ms( > @@ -2075,7 +2066,6 @@ static int cm_req_handler(struct cm_work *work) > cpu_to_be32(IBA_GET(CM_REQ_LOCAL_COMM_ID, req_msg)); > cm_id_priv->id.service_id = > cpu_to_be64(IBA_GET(CM_REQ_SERVICE_ID, req_msg)); > - cm_id_priv->id.service_mask = ~cpu_to_be64(0); > cm_id_priv->tid = req_msg->hdr.tid; > cm_id_priv->timeout_ms = cm_convert_to_ms( > IBA_GET(CM_REQ_LOCAL_CM_RESPONSE_TIMEOUT, req_msg)); > @@ -3482,7 +3472,6 @@ int ib_send_cm_sidr_req(struct ib_cm_id *cm_id, > spin_lock_irqsave(&cm_id_priv->lock, flags); > cm_move_av_from_path(&cm_id_priv->av, &av); > cm_id->service_id = param->service_id; > - cm_id->service_mask = ~cpu_to_be64(0); > cm_id_priv->timeout_ms = param->timeout_ms; > cm_id_priv->max_cm_retries = param->max_cm_retries; > if (cm_id->state != IB_CM_IDLE) { > @@ -3557,7 +3546,6 @@ static int cm_sidr_req_handler(struct cm_work *work) > cpu_to_be32(IBA_GET(CM_SIDR_REQ_REQUESTID, sidr_req_msg)); > cm_id_priv->id.service_id = > cpu_to_be64(IBA_GET(CM_SIDR_REQ_SERVICEID, sidr_req_msg)); > - cm_id_priv->id.service_mask = ~cpu_to_be64(0); > cm_id_priv->tid = sidr_req_msg->hdr.tid; > > wc = work->mad_recv_wc->wc; > diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h > index fbf260c1b1df..8dae5847020a 100644 > --- a/include/rdma/ib_cm.h > +++ b/include/rdma/ib_cm.h > @@ -294,7 +294,6 @@ struct ib_cm_id { > void *context; > struct ib_device *device; > __be64 service_id; > - __be64 service_mask; > enum ib_cm_state state; /* internal CM/debug use */ > enum ib_cm_lap_state lap_state; /* internal CM/debug use */ > __be32 local_id; > -- > 2.26.3 >
On 8/21/2022 7:34 PM, Leon Romanovsky wrote: > On Fri, Aug 19, 2022 at 12:08:58PM +0300, Mark Zhang wrote: >> The service_mask is always ~cpu_to_be64(0), so the result is always >> a NOP when it is &'d with a service_id. Remove it for simplicity. >> >> Signed-off-by: Mark Zhang <markzhang@nvidia.com> >> --- >> drivers/infiniband/core/cm.c | 28 ++++++++-------------------- >> include/rdma/ib_cm.h | 1 - >> 2 files changed, 8 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c >> index b59f864b3d79..84bb10799467 100644 >> --- a/drivers/infiniband/core/cm.c >> +++ b/drivers/infiniband/core/cm.c >> @@ -617,7 +617,6 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, >> struct rb_node *parent = NULL; >> struct cm_id_private *cur_cm_id_priv; >> __be64 service_id = cm_id_priv->id.service_id; >> - __be64 service_mask = cm_id_priv->id.service_mask; >> unsigned long flags; >> >> spin_lock_irqsave(&cm.lock, flags); >> @@ -625,8 +624,7 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, >> parent = *link; >> cur_cm_id_priv = rb_entry(parent, struct cm_id_private, >> service_node); >> - if ((cur_cm_id_priv->id.service_mask & service_id) == >> - (service_mask & cur_cm_id_priv->id.service_id) && >> + if ((service_id == cur_cm_id_priv->id.service_id) && >> (cm_id_priv->id.device == cur_cm_id_priv->id.device)) { >> /* >> * Sharing an ib_cm_id with different handlers is not >> @@ -670,8 +668,7 @@ 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 && >> + if ((service_id == cm_id_priv->id.service_id) && >> (cm_id_priv->id.device == device)) { >> refcount_inc(&cm_id_priv->refcount); >> return cm_id_priv; >> @@ -1158,22 +1155,17 @@ void ib_destroy_cm_id(struct ib_cm_id *cm_id) >> } >> EXPORT_SYMBOL(ib_destroy_cm_id); >> >> -static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id, >> - __be64 service_mask) >> +static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id) >> { >> - service_mask = service_mask ? service_mask : ~cpu_to_be64(0); >> - service_id &= service_mask; >> if ((service_id & IB_SERVICE_ID_AGN_MASK) == IB_CM_ASSIGN_SERVICE_ID && >> (service_id != IB_CM_ASSIGN_SERVICE_ID)) >> return -EINVAL; >> >> - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { >> + if (service_id == IB_CM_ASSIGN_SERVICE_ID) >> cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); >> - cm_id_priv->id.service_mask = ~cpu_to_be64(0); >> - } else { >> + else >> cm_id_priv->id.service_id = service_id; >> - cm_id_priv->id.service_mask = service_mask; >> - } > > If service_id != IB_CM_ASSIGN_SERVICE_ID, we had zero as service_mask > and not FFF... like you wrote. It puts in question all > cm_id_priv->id.service_mask & service_id => service_id conversions in > this patch. The id.service_mask field is removed in this patch, check the change in include/rdma/ib_cm.h > >> + >> return 0; >> } >> >> @@ -1199,7 +1191,7 @@ int ib_cm_listen(struct ib_cm_id *cm_id, __be64 service_id) >> goto out; >> } >> >> - ret = cm_init_listen(cm_id_priv, service_id, 0); >> + ret = cm_init_listen(cm_id_priv, service_id); >> if (ret) >> goto out; >> >> @@ -1247,7 +1239,7 @@ struct ib_cm_id *ib_cm_insert_listen(struct ib_device *device, >> if (IS_ERR(cm_id_priv)) >> return ERR_CAST(cm_id_priv); >> >> - err = cm_init_listen(cm_id_priv, service_id, 0); >> + err = cm_init_listen(cm_id_priv, service_id); >> if (err) { >> ib_destroy_cm_id(&cm_id_priv->id); >> return ERR_PTR(err); >> @@ -1518,7 +1510,6 @@ int ib_send_cm_req(struct ib_cm_id *cm_id, >> } >> } >> cm_id->service_id = param->service_id; >> - cm_id->service_mask = ~cpu_to_be64(0); >> cm_id_priv->timeout_ms = cm_convert_to_ms( >> param->primary_path->packet_life_time) * 2 + >> cm_convert_to_ms( >> @@ -2075,7 +2066,6 @@ static int cm_req_handler(struct cm_work *work) >> cpu_to_be32(IBA_GET(CM_REQ_LOCAL_COMM_ID, req_msg)); >> cm_id_priv->id.service_id = >> cpu_to_be64(IBA_GET(CM_REQ_SERVICE_ID, req_msg)); >> - cm_id_priv->id.service_mask = ~cpu_to_be64(0); >> cm_id_priv->tid = req_msg->hdr.tid; >> cm_id_priv->timeout_ms = cm_convert_to_ms( >> IBA_GET(CM_REQ_LOCAL_CM_RESPONSE_TIMEOUT, req_msg)); >> @@ -3482,7 +3472,6 @@ int ib_send_cm_sidr_req(struct ib_cm_id *cm_id, >> spin_lock_irqsave(&cm_id_priv->lock, flags); >> cm_move_av_from_path(&cm_id_priv->av, &av); >> cm_id->service_id = param->service_id; >> - cm_id->service_mask = ~cpu_to_be64(0); >> cm_id_priv->timeout_ms = param->timeout_ms; >> cm_id_priv->max_cm_retries = param->max_cm_retries; >> if (cm_id->state != IB_CM_IDLE) { >> @@ -3557,7 +3546,6 @@ static int cm_sidr_req_handler(struct cm_work *work) >> cpu_to_be32(IBA_GET(CM_SIDR_REQ_REQUESTID, sidr_req_msg)); >> cm_id_priv->id.service_id = >> cpu_to_be64(IBA_GET(CM_SIDR_REQ_SERVICEID, sidr_req_msg)); >> - cm_id_priv->id.service_mask = ~cpu_to_be64(0); >> cm_id_priv->tid = sidr_req_msg->hdr.tid; >> >> wc = work->mad_recv_wc->wc; >> diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h >> index fbf260c1b1df..8dae5847020a 100644 >> --- a/include/rdma/ib_cm.h >> +++ b/include/rdma/ib_cm.h >> @@ -294,7 +294,6 @@ struct ib_cm_id { >> void *context; >> struct ib_device *device; >> __be64 service_id; >> - __be64 service_mask; >> enum ib_cm_state state; /* internal CM/debug use */ >> enum ib_cm_lap_state lap_state; /* internal CM/debug use */ >> __be32 local_id; >> -- >> 2.26.3 >>
On Sun, Aug 21, 2022 at 08:50:08PM +0800, Mark Zhang wrote: > On 8/21/2022 7:34 PM, Leon Romanovsky wrote: > > On Fri, Aug 19, 2022 at 12:08:58PM +0300, Mark Zhang wrote: > > > The service_mask is always ~cpu_to_be64(0), so the result is always > > > a NOP when it is &'d with a service_id. Remove it for simplicity. > > > > > > Signed-off-by: Mark Zhang <markzhang@nvidia.com> > > > --- > > > drivers/infiniband/core/cm.c | 28 ++++++++-------------------- > > > include/rdma/ib_cm.h | 1 - > > > 2 files changed, 8 insertions(+), 21 deletions(-) <...> > > > - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { > > > + if (service_id == IB_CM_ASSIGN_SERVICE_ID) > > > cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); > > > - cm_id_priv->id.service_mask = ~cpu_to_be64(0); > > > - } else { > > > + else > > > cm_id_priv->id.service_id = service_id; > > > - cm_id_priv->id.service_mask = service_mask; > > > - } > > > > If service_id != IB_CM_ASSIGN_SERVICE_ID, we had zero as service_mask > > and not FFF... like you wrote. It puts in question all > > cm_id_priv->id.service_mask & service_id => service_id conversions in > > this patch. > > The id.service_mask field is removed in this patch, check the change in > include/rdma/ib_cm.h Right, you removed service_mask and described it "is always ~cpu_to_be64(0)". As far as I can tell, this is not true and in this "if (service_id == IB_CM_ASSIGN_SERVICE_ID)" sometimes we set cm_id_priv->id.service_mask to be 0 and sometimes 0xFF.... Why is it correct to remove cm_id_priv->id.service_mask in this hunk? Thanks
On 8/21/2022 9:47 PM, Leon Romanovsky wrote: > On Sun, Aug 21, 2022 at 08:50:08PM +0800, Mark Zhang wrote: >> On 8/21/2022 7:34 PM, Leon Romanovsky wrote: >>> On Fri, Aug 19, 2022 at 12:08:58PM +0300, Mark Zhang wrote: >>>> The service_mask is always ~cpu_to_be64(0), so the result is always >>>> a NOP when it is &'d with a service_id. Remove it for simplicity. >>>> >>>> Signed-off-by: Mark Zhang <markzhang@nvidia.com> >>>> --- >>>> drivers/infiniband/core/cm.c | 28 ++++++++-------------------- >>>> include/rdma/ib_cm.h | 1 - >>>> 2 files changed, 8 insertions(+), 21 deletions(-) > > <...> > >>>> - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { >>>> + if (service_id == IB_CM_ASSIGN_SERVICE_ID) >>>> cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); >>>> - cm_id_priv->id.service_mask = ~cpu_to_be64(0); >>>> - } else { >>>> + else >>>> cm_id_priv->id.service_id = service_id; >>>> - cm_id_priv->id.service_mask = service_mask; >>>> - } >>> >>> If service_id != IB_CM_ASSIGN_SERVICE_ID, we had zero as service_mask >>> and not FFF... like you wrote. It puts in question all >>> cm_id_priv->id.service_mask & service_id => service_id conversions in >>> this patch. >> >> The id.service_mask field is removed in this patch, check the change in >> include/rdma/ib_cm.h > > Right, you removed service_mask and described it "is always ~cpu_to_be64(0)". > As far as I can tell, this is not true and in this "if (service_id == IB_CM_ASSIGN_SERVICE_ID)" > sometimes we set cm_id_priv->id.service_mask to be 0 and sometimes 0xFF.... > > Why is it correct to remove cm_id_priv->id.service_mask in this hunk? 1. service_id == IB_CM_ASSIGN_SERVICE_ID: cm_id_priv->id.service_mask = ~cpu_to_be64(0); 2. service_id != IB_CM_ASSIGN_SERVICE_ID: cm_id_priv->id.service_mask = service_mask; It's also ~cpu_to_be64(0), because cm_init_listen() is always called with service_mask = 0: ib_cm_listen(..., 0) -> cm_init_listen(..., 0) ib_cm_insert_listen() -> cm_init_listen(..., 0) So it's always ~cpu_to_be64(0).. Thanks
On Sun, Aug 21, 2022 at 10:08:54PM +0800, Mark Zhang wrote: > On 8/21/2022 9:47 PM, Leon Romanovsky wrote: > > On Sun, Aug 21, 2022 at 08:50:08PM +0800, Mark Zhang wrote: > > > On 8/21/2022 7:34 PM, Leon Romanovsky wrote: > > > > On Fri, Aug 19, 2022 at 12:08:58PM +0300, Mark Zhang wrote: > > > > > The service_mask is always ~cpu_to_be64(0), so the result is always > > > > > a NOP when it is &'d with a service_id. Remove it for simplicity. > > > > > > > > > > Signed-off-by: Mark Zhang <markzhang@nvidia.com> > > > > > --- > > > > > drivers/infiniband/core/cm.c | 28 ++++++++-------------------- > > > > > include/rdma/ib_cm.h | 1 - > > > > > 2 files changed, 8 insertions(+), 21 deletions(-) > > > > <...> > > > > > > > - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { > > > > > + if (service_id == IB_CM_ASSIGN_SERVICE_ID) > > > > > cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); > > > > > - cm_id_priv->id.service_mask = ~cpu_to_be64(0); > > > > > - } else { > > > > > + else > > > > > cm_id_priv->id.service_id = service_id; > > > > > - cm_id_priv->id.service_mask = service_mask; > > > > > - } > > > > > > > > If service_id != IB_CM_ASSIGN_SERVICE_ID, we had zero as service_mask > > > > and not FFF... like you wrote. It puts in question all > > > > cm_id_priv->id.service_mask & service_id => service_id conversions in > > > > this patch. > > > > > > The id.service_mask field is removed in this patch, check the change in > > > include/rdma/ib_cm.h > > > > Right, you removed service_mask and described it "is always ~cpu_to_be64(0)". > > As far as I can tell, this is not true and in this "if (service_id == IB_CM_ASSIGN_SERVICE_ID)" > > sometimes we set cm_id_priv->id.service_mask to be 0 and sometimes 0xFF.... > > > > Why is it correct to remove cm_id_priv->id.service_mask in this hunk? > > 1. service_id == IB_CM_ASSIGN_SERVICE_ID: > cm_id_priv->id.service_mask = ~cpu_to_be64(0); > > 2. service_id != IB_CM_ASSIGN_SERVICE_ID: > cm_id_priv->id.service_mask = service_mask; > It's also ~cpu_to_be64(0), because cm_init_listen() is always called > with service_mask = 0: > ib_cm_listen(..., 0) -> cm_init_listen(..., 0) > ib_cm_insert_listen() -> cm_init_listen(..., 0) > > So it's always ~cpu_to_be64(0).. I see it now, thanks. > > Thanks > > > > >
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index b59f864b3d79..84bb10799467 100644 --- a/drivers/infiniband/core/cm.c +++ b/drivers/infiniband/core/cm.c @@ -617,7 +617,6 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, struct rb_node *parent = NULL; struct cm_id_private *cur_cm_id_priv; __be64 service_id = cm_id_priv->id.service_id; - __be64 service_mask = cm_id_priv->id.service_mask; unsigned long flags; spin_lock_irqsave(&cm.lock, flags); @@ -625,8 +624,7 @@ static struct cm_id_private *cm_insert_listen(struct cm_id_private *cm_id_priv, parent = *link; cur_cm_id_priv = rb_entry(parent, struct cm_id_private, service_node); - if ((cur_cm_id_priv->id.service_mask & service_id) == - (service_mask & cur_cm_id_priv->id.service_id) && + if ((service_id == cur_cm_id_priv->id.service_id) && (cm_id_priv->id.device == cur_cm_id_priv->id.device)) { /* * Sharing an ib_cm_id with different handlers is not @@ -670,8 +668,7 @@ 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 && + if ((service_id == cm_id_priv->id.service_id) && (cm_id_priv->id.device == device)) { refcount_inc(&cm_id_priv->refcount); return cm_id_priv; @@ -1158,22 +1155,17 @@ void ib_destroy_cm_id(struct ib_cm_id *cm_id) } EXPORT_SYMBOL(ib_destroy_cm_id); -static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id, - __be64 service_mask) +static int cm_init_listen(struct cm_id_private *cm_id_priv, __be64 service_id) { - service_mask = service_mask ? service_mask : ~cpu_to_be64(0); - service_id &= service_mask; if ((service_id & IB_SERVICE_ID_AGN_MASK) == IB_CM_ASSIGN_SERVICE_ID && (service_id != IB_CM_ASSIGN_SERVICE_ID)) return -EINVAL; - if (service_id == IB_CM_ASSIGN_SERVICE_ID) { + if (service_id == IB_CM_ASSIGN_SERVICE_ID) cm_id_priv->id.service_id = cpu_to_be64(cm.listen_service_id++); - cm_id_priv->id.service_mask = ~cpu_to_be64(0); - } else { + else cm_id_priv->id.service_id = service_id; - cm_id_priv->id.service_mask = service_mask; - } + return 0; } @@ -1199,7 +1191,7 @@ int ib_cm_listen(struct ib_cm_id *cm_id, __be64 service_id) goto out; } - ret = cm_init_listen(cm_id_priv, service_id, 0); + ret = cm_init_listen(cm_id_priv, service_id); if (ret) goto out; @@ -1247,7 +1239,7 @@ struct ib_cm_id *ib_cm_insert_listen(struct ib_device *device, if (IS_ERR(cm_id_priv)) return ERR_CAST(cm_id_priv); - err = cm_init_listen(cm_id_priv, service_id, 0); + err = cm_init_listen(cm_id_priv, service_id); if (err) { ib_destroy_cm_id(&cm_id_priv->id); return ERR_PTR(err); @@ -1518,7 +1510,6 @@ int ib_send_cm_req(struct ib_cm_id *cm_id, } } cm_id->service_id = param->service_id; - cm_id->service_mask = ~cpu_to_be64(0); cm_id_priv->timeout_ms = cm_convert_to_ms( param->primary_path->packet_life_time) * 2 + cm_convert_to_ms( @@ -2075,7 +2066,6 @@ static int cm_req_handler(struct cm_work *work) cpu_to_be32(IBA_GET(CM_REQ_LOCAL_COMM_ID, req_msg)); cm_id_priv->id.service_id = cpu_to_be64(IBA_GET(CM_REQ_SERVICE_ID, req_msg)); - cm_id_priv->id.service_mask = ~cpu_to_be64(0); cm_id_priv->tid = req_msg->hdr.tid; cm_id_priv->timeout_ms = cm_convert_to_ms( IBA_GET(CM_REQ_LOCAL_CM_RESPONSE_TIMEOUT, req_msg)); @@ -3482,7 +3472,6 @@ int ib_send_cm_sidr_req(struct ib_cm_id *cm_id, spin_lock_irqsave(&cm_id_priv->lock, flags); cm_move_av_from_path(&cm_id_priv->av, &av); cm_id->service_id = param->service_id; - cm_id->service_mask = ~cpu_to_be64(0); cm_id_priv->timeout_ms = param->timeout_ms; cm_id_priv->max_cm_retries = param->max_cm_retries; if (cm_id->state != IB_CM_IDLE) { @@ -3557,7 +3546,6 @@ static int cm_sidr_req_handler(struct cm_work *work) cpu_to_be32(IBA_GET(CM_SIDR_REQ_REQUESTID, sidr_req_msg)); cm_id_priv->id.service_id = cpu_to_be64(IBA_GET(CM_SIDR_REQ_SERVICEID, sidr_req_msg)); - cm_id_priv->id.service_mask = ~cpu_to_be64(0); cm_id_priv->tid = sidr_req_msg->hdr.tid; wc = work->mad_recv_wc->wc; diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h index fbf260c1b1df..8dae5847020a 100644 --- a/include/rdma/ib_cm.h +++ b/include/rdma/ib_cm.h @@ -294,7 +294,6 @@ struct ib_cm_id { void *context; struct ib_device *device; __be64 service_id; - __be64 service_mask; enum ib_cm_state state; /* internal CM/debug use */ enum ib_cm_lap_state lap_state; /* internal CM/debug use */ __be32 local_id;
The service_mask is always ~cpu_to_be64(0), so the result is always a NOP when it is &'d with a service_id. Remove it for simplicity. Signed-off-by: Mark Zhang <markzhang@nvidia.com> --- drivers/infiniband/core/cm.c | 28 ++++++++-------------------- include/rdma/ib_cm.h | 1 - 2 files changed, 8 insertions(+), 21 deletions(-)