Re: [PATCH rdma-next 07/11] RDMA/erdma: Add verbs implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/22/21 8:46 PM, Bernard Metzler wrote:
-----Original Message-----
From: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>
Sent: Wednesday, 22 December 2021 04:11
To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>; Leon Romanovsky
<leon@xxxxxxxxxx>
Cc: jgg@xxxxxxxx; dledford@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
KaiShen@xxxxxxxxxxxxxxxxx; tonylu@xxxxxxxxxxxxxxxxx
Subject: [EXTERNAL] Re: [PATCH rdma-next 07/11] RDMA/erdma: Add verbs
implementation



On 12/21/21 11:20 PM, Bernard Metzler wrote:
-----Original Message-----
From: Leon Romanovsky <leon@xxxxxxxxxx>
Sent: Tuesday, 21 December 2021 14:32
To: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>
Cc: jgg@xxxxxxxx; dledford@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
KaiShen@xxxxxxxxxxxxxxxxx; tonylu@xxxxxxxxxxxxxxxxx
Subject: [EXTERNAL] Re: [PATCH rdma-next 07/11] RDMA/erdma: Add verbs
implementation

On Tue, Dec 21, 2021 at 10:48:54AM +0800, Cheng Xu wrote:
The RDMA verbs implementation of erdma is divided into three files:
erdma_qp.c, erdma_cq.c, and erdma_verbs.c. Internal used functions and
datapath functions of QP/CQ are put in erdma_qp.c and erdma_cq.c, the
reset
is in erdma_verbs.c.

Signed-off-by: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>
---
   drivers/infiniband/hw/erdma/erdma_cq.c    |  201 +++
   drivers/infiniband/hw/erdma/erdma_qp.c    |  624 +++++++++
   drivers/infiniband/hw/erdma/erdma_verbs.c | 1477
+++++++++++++++++++++
   3 files changed, 2302 insertions(+)
   create mode 100644 drivers/infiniband/hw/erdma/erdma_cq.c
   create mode 100644 drivers/infiniband/hw/erdma/erdma_qp.c
   create mode 100644 drivers/infiniband/hw/erdma/erdma_verbs.c


Please no inline functions in .c files and no void casting for the
return values of functions.

<...>

diff --git a/drivers/infiniband/hw/erdma/erdma_qp.c
b/drivers/infiniband/hw/erdma/erdma_qp.c
new file mode 100644
index 000000000000..8c02215cee04
--- /dev/null
+++ b/drivers/infiniband/hw/erdma/erdma_qp.c
@@ -0,0 +1,624 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Authors: Cheng Xu <chengyou@xxxxxxxxxxxxxxxxx>
+ *          Kai Shen <kaishen@xxxxxxxxxxxxxxxxx>
+ * Copyright (c) 2020-2021, Alibaba Group.
+ *
+ * Authors: Bernard Metzler <bmt@xxxxxxxxxxxxxx>
+ *          Fredy Neeser <nfd@xxxxxxxxxxxxxx>
+ * Copyright (c) 2008-2016, IBM Corporation

What does it mean?


Significant parts of the driver have been taken from siw it seems.
Probably really from an old version of it.
In that case I would have recommended to take the upstream siw
code, which has been cleaned from those issues we now see again
(including debugfs code, extern definitions, inline in .c code,
casting issues, etc etc.). Why starting in 2020 with
code from 2016, if better code is available?

Bernard.

First of all, thank you for developing siw, Bernard and Fredy, so we
can build our erdma based on your work.

You are welcome.
You probably got the code from https://github.com/zrlio/softiwarp
where I stopped pushing updates 4 years ago. By then, I started working
on making it acceptable for upstream. As said, I highly recommend taking
it from there, since the community already invested time and effort to
make the code better, and finally acceptable. If you do so, please also
update the copyright notice.

Thank you, and I will follow your recommendation.

Fredy isn't part of it since almost 10 years, and is not reachable
via the email provided. And, by 2016, his contributions were limited to
the siw_cm.c code only.



At the beginning, we started developing erdma driver in kernel
4.9/4.19/5.10, and didn't know the upstream siw version since it is in


siw is in the Linux kernel since v5.3


Yes, I got it wrong.

Thanks,
Cheng Xu

the newer kernel version. As a result, we develop erdma based on the
older version.
Thank you for your recommendation. We will check the differences and
take the upstream siw code if needed.

Thanks,
Cheng Xu



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux