Re: [PATCH net-next 4/4] RDMA/rxe: Set queue pair cur_qp_state when being queried

[Zhu Yanjun <yanjun.zhu@xxxxxxxxx>]

在 2024/8/9 16:31, Liu Jian 写道:
> Same with commit e375b9c92985 ("RDMA/cxgb4: Set queue pair state when
>   being queried"). The API for ib_query_qp requires the driver to set
> cur_qp_state on return, add the missing set.

Add the following?
Cc: stable@xxxxxxxxxxxxxxx

> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Liu Jian <liujian56@xxxxxxxxxx>
> ---
>   drivers/infiniband/sw/rxe/rxe_verbs.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 5c18f7e342f2..699b4b315336 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -634,6 +634,8 @@ static int rxe_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>   	rxe_qp_to_init(qp, init);
>   	rxe_qp_to_attr(qp, attr, mask);
>   
> +	attr->cur_qp_state = qp->attr.qp_state;

I am fine with this commit.

But I think this "attr->cur_qp_state = qp->attr.qp_state;" should be put 
into this function rxe_qp_to_attr.

And the access to qp->attr.qp_state should be protected by spin lock 
qp->state_lock.

So the following is better.
Any way, thanks.

Reviewed-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c 
b/drivers/infiniband/sw/rxe/rxe_qp.c
index d2f7b5195c19..da723b9690e5 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -782,6 +782,10 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct 
ib_qp_attr *attr, int mask)
                spin_unlock_irqrestore(&qp->state_lock, flags);
        }

+       spin_lock_irqsave(&qp->state_lock, flags);
+       attr->cur_qp_state = qp_state(qp);
+       spin_unlock_irqrestore(&qp->state_lock, flags);
+
        return 0;
 }

Best Regards,
Zhu Yanjun

> +
>   	return 0;
>   }
>