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@xxxxxxxxxx>
---
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
