From: Leon Romanovsky <leonro@nvidia.com>
To: Mark Zhang <markzhang@nvidia.com>
Cc: <jgg@nvidia.com>, <dledford@redhat.com>,
<jiapeng.chong@linux.alibaba.com>, <cgel.zte@gmail.com>,
<linux-rdma@vger.kernel.org>
Subject: Re: [PATCH rdma-next 2/3] IB/cm: remove cm_id_priv->id.service_mask and service_mask parameter of cm_init_listen()
Date: Sun, 21 Aug 2022 14:34:52 +0300 [thread overview]
Message-ID: <YwIYXI9xTSpw4Vtj@unreal> (raw)
In-Reply-To: <20220819090859.957943-3-markzhang@nvidia.com>
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
>
next prev parent reply other threads:[~2022-08-21 11:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-19 9:08 [PATCH rdma-next 0/3] IB/cm refactors Mark Zhang
2022-08-19 9:08 ` [PATCH rdma-next 1/3] IB/cm: Remove the service_mask parameter from ib_cm_listen() Mark Zhang
2022-08-19 9:08 ` [PATCH rdma-next 2/3] IB/cm: remove cm_id_priv->id.service_mask and service_mask parameter of cm_init_listen() Mark Zhang
2022-08-21 11:34 ` Leon Romanovsky [this message]
2022-08-21 12:50 ` Mark Zhang
2022-08-21 13:47 ` Leon Romanovsky
2022-08-21 14:08 ` Mark Zhang
2022-08-21 14:14 ` Leon Romanovsky
2022-08-19 9:08 ` [PATCH rdma-next 3/3] IB/cm: Refactor cm_insert_listen() and cm_find_listen() Mark Zhang
2022-08-29 8:21 ` [PATCH rdma-next 0/3] IB/cm refactors Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YwIYXI9xTSpw4Vtj@unreal \
--to=leonro@nvidia.com \
--cc=cgel.zte@gmail.com \
--cc=dledford@redhat.com \
--cc=jgg@nvidia.com \
--cc=jiapeng.chong@linux.alibaba.com \
--cc=linux-rdma@vger.kernel.org \
--cc=markzhang@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.