diff mbox series

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

Message ID 20240809083148.1989912-5-liujian56@huawei.com (mailing list archive)
State Not Applicable
Headers show
Series Make SMC-R can work with rxe devices | expand

Commit Message

liujian (CE) Aug. 9, 2024, 8:31 a.m. UTC
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.

Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
 drivers/infiniband/sw/rxe/rxe_verbs.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Zhu Yanjun Aug. 9, 2024, 11:06 a.m. UTC | #1
在 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@vger.kernel.org

> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
>   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@linux.dev>

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;
>   }
>
Leon Romanovsky Aug. 11, 2024, 8:31 a.m. UTC | #2
On Fri, Aug 09, 2024 at 07:06:34PM +0800, Zhu Yanjun wrote:
> 在 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@vger.kernel.org

There is no need to add stable tag for RXE driver. Distros are not
supporting this driver, which is used for development and not for
production.

Thanks

> 
> > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > ---
> >   drivers/infiniband/sw/rxe/rxe_verbs.c | 2 ++
> >   1 file changed, 2 insertions(+)
Zhu Yanjun Aug. 11, 2024, 10:24 a.m. UTC | #3
在 2024/8/11 18:22, Zhu Yanjun 写道:
>
>
> 在 2024/8/11 16:31, Leon Romanovsky 写道:
>> On Fri, Aug 09, 2024 at 07:06:34PM +0800, Zhu Yanjun wrote:
>>> 在 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@vger.kernel.org
>> There is no need to add stable tag for RXE driver. Distros are not
>> supporting this driver, which is used for development and not for
>> production.
>
> I do not mean that this driver is supported in Distros.
>
> I mean that QP cur_qp_state is not set when being queried in rxe. In 
> other rdma drivers, this is set.
>
> This should be a problem in RXE. So I suggest to add "CC: 
> stable@vger.kernel.org".
>
> As such, the stable branch will backport this commit to fix this 
> problem in RXE.
>

Sorry. My mistakes.

No need to add this "CC: stable@vger.kernel.org"

Zhu Yanjun


> Anyway, thanks a lot for your reporting and fixing this problem in RXE.
>
> Best Regards,
>
> Zhu Yanjun
>
>> Thanks
>>
>>>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>>>> Signed-off-by: Liu Jian<liujian56@huawei.com>
>>>> ---
>>>>    drivers/infiniband/sw/rxe/rxe_verbs.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
> -- 
> Best Regards,
> Yanjun.Zhu
diff mbox series

Patch

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;
+
 	return 0;
 }