Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

Introduce libibverbs atomic write #2085

Merged
merged 5 commits into from
Jan 3, 2023
Merged

Conversation

yangx-jy
Copy link
Contributor

@yangx-jy yangx-jy commented Dec 8, 2022

This feature is based on my atomic write patch sets:
kernel: [[PATCH v7 0/8] RDMA/rxe: Add atomic write operation] (it has been merged into rdma for-next branch)
rdma-core: linux-rdma/rdma-core#1179 (it has been merged into rdma-core master)


This change is Reviewable

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: 0 of 9 files reviewed, all discussions resolved

@yangx-jy
Copy link
Contributor Author

@ldorau @grom72

The whole libibverbs atomic write patch set has been merged into kernel and rdma-core repo. I think it is time to review this PR. ^_^

Best Regards,
Xiao Yang

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @yangx-jy)

a discussion (no related file):
The 07-atomic-write example fails with the unexpected wc.wr_id value (0x0 != 0xF01D) error.


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code coverage is OK: https://app.codecov.io/gh/pmem/rpma/pull/2085

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @yangx-jy)

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions (waiting on @yangx-jy)

a discussion (no related file):
On Mellanox ConnectX-5 RNIC (Mellanox Technologies MT27800 Family) all examples fail with the following error:

Dec 13 13:33:41.367207 [1374172] *ERROR* peer.c: 208: rpma_peer_setup_qp: rdma_create_qp_ex(max_send_wr=10, max_recv_wr=10, max_send/recv_sge=1, max_inline_data=8, qp_type=IBV_QPT_RC, sq_sig_all=0) failed: Operation not supported

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions (waiting on @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

The 07-atomic-write example fails with the unexpected wc.wr_id value (0x0 != 0xF01D) error.

(tested on SoftRoCE)


Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions (waiting on @ldorau and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

On Mellanox ConnectX-5 RNIC (Mellanox Technologies MT27800 Family) all examples fail with the following error:

Dec 13 13:33:41.367207 [1374172] *ERROR* peer.c: 208: rpma_peer_setup_qp: rdma_create_qp_ex(max_send_wr=10, max_recv_wr=10, max_send/recv_sge=1, max_inline_data=8, qp_type=IBV_QPT_RC, sq_sig_all=0) failed: Operation not supported

To use the atomic__write operation to conditions must be fulfilled:

  • libibverbs must support ibv_wr_atomic_write()
  • RDMA driver must support this operation
    The first item is checked by function(is_ibv_wr_atomic_write_supported var).
    The second condition must be verified during peer setup and later appropriate API shall be used (new one or legacy one)

@ldorau ldorau requested a review from grom72 December 13, 2022 15:42
Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, grom72 (Tomasz Gromadzki) wrote…

To use the atomic__write operation to conditions must be fulfilled:

  • libibverbs must support ibv_wr_atomic_write()
  • RDMA driver must support this operation
    The first item is checked by function(is_ibv_wr_atomic_write_supported var).
    The second condition must be verified during peer setup and later appropriate API shall be used (new one or legacy one)

OK, thanks @grom72 !


Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yangx-jy)

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yangx-jy)

Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ldorau)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

(tested on SoftRoCE)

@ldorau

It seems that your kernel doesn't support atomic write operation (Did you apply my kernel patch set into your kernel?). In this case, a completion with IB_WC_LOC_QP_OP_ERR was always generated.


a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

OK, thanks @grom72 !

The above error indicated that Mellanox-5 provider (rdma-core/providers/mlx5/*) doesn't support atomic write operation.

Perhaps we need to check the atomic write support for both libibverbs and driver.


@ldorau ldorau requested a review from grom72 December 15, 2022 08:37
Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

@ldorau

It seems that your kernel doesn't support atomic write operation (Did you apply my kernel patch set into your kernel?). In this case, a completion with IB_WC_LOC_QP_OP_ERR was always generated.

What version of kernel is required?


a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

The above error indicated that Mellanox-5 provider (rdma-core/providers/mlx5/*) doesn't support atomic write operation.

Perhaps we need to check the atomic write support for both libibverbs and driver.

Yes, we have to check the atomic write support for both libibverbs and driver.


@yangx-jy
Copy link
Contributor Author

Previously, ldorau (Lukasz Dorau) wrote…

What version of kernel is required?

There is no released kernel version supporting atomic write for now.
You have to build the latest kernel source from main line and then verify this PR.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

There is no released kernel version supporting atomic write for now.
You have to build the latest kernel source from main line and then verify this PR.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

What version of kernel is it going to be released in?


a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Yes, we have to check the atomic write support for both libibverbs and driver.

So please add checking if there is the support for the atomic write in the driver.


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

What version of kernel is it going to be released in?

  1. What version of kernel is it going to be released in?
  2. What exactly patches are required?

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…
  1. What version of kernel is it going to be released in?
  2. What exactly patches are required?

I have built linux kernel 6.1 with the following patches applied:

4cd9f1d320f9 RDMA/rxe: Enable atomic write capability for rxe device
3aec427bb149 RDMA/rxe: Implement atomic write completion
034e285f8b99 RDMA/rxe: Make responder support atomic write on RC service
abb633cf2804 RDMA/rxe: Make requester support atomic write on RC service
5c7af6c79384 RDMA/rxe: Extend rxe packet format to support atomic write
c2d939002934 RDMA/rxe: Extend rxe user ABI to support atomic write
3ff81e827b8d RDMA: Extend RDMA kernel ABI to support atomic write
efa2afc3969e RDMA: Extend RDMA user ABI to support atomic write

and now all examples fail with the following error (on SoftRoCE):

*ERROR* peer.c: 203: rpma_peer_setup_qp: rdma_create_qp_ex(max_send_wr=10, max_recv_wr=10, max_send/recv_sge=1, max_inline_data=8, qp_type=IBV_QPT_RC, sq_sig_all=0) failed: Operation not supported

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

I have built linux kernel 6.1 with the following patches applied:

4cd9f1d320f9 RDMA/rxe: Enable atomic write capability for rxe device
3aec427bb149 RDMA/rxe: Implement atomic write completion
034e285f8b99 RDMA/rxe: Make responder support atomic write on RC service
abb633cf2804 RDMA/rxe: Make requester support atomic write on RC service
5c7af6c79384 RDMA/rxe: Extend rxe packet format to support atomic write
c2d939002934 RDMA/rxe: Extend rxe user ABI to support atomic write
3ff81e827b8d RDMA: Extend RDMA kernel ABI to support atomic write
efa2afc3969e RDMA: Extend RDMA user ABI to support atomic write

and now all examples fail with the following error (on SoftRoCE):

*ERROR* peer.c: 203: rpma_peer_setup_qp: rdma_create_qp_ex(max_send_wr=10, max_recv_wr=10, max_send/recv_sge=1, max_inline_data=8, qp_type=IBV_QPT_RC, sq_sig_all=0) failed: Operation not supported

No, it was my mistake. Something was wrong with the build or the environment.

I have double-checked and I confirm that this PR with linux kernel 6.1 and the following patches applied:

4cd9f1d320f9 RDMA/rxe: Enable atomic write capability for rxe device
3aec427bb149 RDMA/rxe: Implement atomic write completion
034e285f8b99 RDMA/rxe: Make responder support atomic write on RC service
abb633cf2804 RDMA/rxe: Make requester support atomic write on RC service
5c7af6c79384 RDMA/rxe: Extend rxe packet format to support atomic write
c2d939002934 RDMA/rxe: Extend rxe user ABI to support atomic write
3ff81e827b8d RDMA: Extend RDMA kernel ABI to support atomic write
efa2afc3969e RDMA: Extend RDMA user ABI to support atomic write

WORKS FINE :-)


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

No, it was my mistake. Something was wrong with the build or the environment.

I have double-checked and I confirm that this PR with linux kernel 6.1 and the following patches applied:

4cd9f1d320f9 RDMA/rxe: Enable atomic write capability for rxe device
3aec427bb149 RDMA/rxe: Implement atomic write completion
034e285f8b99 RDMA/rxe: Make responder support atomic write on RC service
abb633cf2804 RDMA/rxe: Make requester support atomic write on RC service
5c7af6c79384 RDMA/rxe: Extend rxe packet format to support atomic write
c2d939002934 RDMA/rxe: Extend rxe user ABI to support atomic write
3ff81e827b8d RDMA: Extend RDMA kernel ABI to support atomic write
efa2afc3969e RDMA: Extend RDMA user ABI to support atomic write

WORKS FINE :-)

(only on SoftRoCE of course)


@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #2085 (53922b3) into main (2d26292) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main     #2085   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1580      1590   +10     
=========================================
+ Hits          1580      1590   +10     

@yangx-jy yangx-jy force-pushed the libibverbs_atomic_write branch 2 times, most recently from 2d5831f to fe5d109 Compare December 16, 2022 08:39
Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 21 files reviewed, 1 unresolved discussion (waiting on @grom72 and @ldorau)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

(only on SoftRoCE of course)

Great.


a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

So please add checking if there is the support for the atomic write in the driver.

I added rpma_utils_ibv_context_is_atomic_write_capable() to check if kernel supports native atomic write.
In this case, I think the atomic write example can work well on all environment.


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 21 files reviewed, all discussions resolved (waiting on @grom72)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

I added rpma_utils_ibv_context_is_atomic_write_capable() to check if kernel supports native atomic write.
In this case, I think the atomic write example can work well on all environment.

It works now :-) Thanks!


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 9 files at r2, 2 of 19 files at r3.
Reviewable status: 4 of 21 files reviewed, all discussions resolved (waiting on @grom72)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 21 files reviewed, 2 unresolved discussions (waiting on @ldorau and @yangx-jy)


src/peer.c line 36 at r3 (raw file):

	int is_odp_supported; /* is On-Demand Paging supported */

	int is_atomic_write_supported; /* is atomic write supported */

is_atomic_write_supported_natively
or
is_atomic_write_supported_by_ibv_device

Code quote:

is_atomic_write_supported

src/peer.c line 350 at r3 (raw file):

	peer->pd = pd;
	peer->is_odp_supported = is_odp_supported;
	peer->is_atomic_write_supported = is_atomic_write_supported;

Please add RPMA_LOG_INFO message when native atomic write is not supported

Code quote:

peer->is_atomic_write_supported = is_atomic_write_supported;

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 9 files at r2, 1 of 19 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yangx-jy)


src/utils.h line 13 at r7 (raw file):

#include "librpma.h"

#define RPMA_ATOMIC_WRITE_CAP_FLAG	(1UL << 40)

Not needed as we should use libibverbs const.

Code quote:

#define RPMA_ATOMIC_WRITE_CAP_FLAG	(1UL << 40)

src/utils.c line 42 at r7 (raw file):

	/* check whether native atomic write is supported in kernel */
	if (attr.device_cap_flags_ex & RPMA_ATOMIC_WRITE_CAP_FLAG)

We should use here the flag defined by libibverbs.

Suggestion:

IB_UVERBS_DEVICE_ATOMIC_WRITE

@ldorau ldorau requested a review from grom72 December 30, 2022 11:18
Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)


src/utils.c line 42 at r7 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

We should use here the flag defined by libibverbs.

And this constant IB_UVERBS_DEVICE_ATOMIC_WRITE should be checked in the CMake script checking support

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @yangx-jy)


cmake/functions.cmake line 298 at r7 (raw file):

		/* check if ibv_wr_atomic_write() is defined */
		int main() {
			return !ibv_wr_atomic_write;

Please check also for IB_UVERBS_DEVICE_ATOMIC_WRITE

Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 19 of 23 files reviewed, 3 unresolved discussions (waiting on @grom72 and @ldorau)


cmake/functions.cmake line 298 at r7 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Please check also for IB_UVERBS_DEVICE_ATOMIC_WRITE

Done. I also check IBV_QP_EX_WITH_ATOMIC_WRITE which is used by rdma_create_qp_ex().


src/utils.h line 13 at r7 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Not needed as we should use libibverbs const.

Done.


src/utils.c line 42 at r7 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

And this constant IB_UVERBS_DEVICE_ATOMIC_WRITE should be checked in the CMake script checking support

Done.

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):
Please update CHANGELOG.md


Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yangx-jy)

a discussion (no related file):
mtt-mr-rpma_mr_remote_from_descriptor_0_none fails all the time for this PR:

236/295 Test #236: mtt-mr-rpma_mr_remote_from_descriptor_0_none .................***Failed 0.03 sec

https://app.circleci.com/pipelines/github/pmem/rpma


Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 23 of 24 files reviewed, 2 unresolved discussions (waiting on @grom72 and @ldorau)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Please update CHANGELOG.md

Done.


a discussion (no related file):

Previously, grom72 (Tomasz Gromadzki) wrote…

mtt-mr-rpma_mr_remote_from_descriptor_0_none fails all the time for this PR:

236/295 Test #236: mtt-mr-rpma_mr_remote_from_descriptor_0_none .................***Failed 0.03 sec

https://app.circleci.com/pipelines/github/pmem/rpma

It seems not be caused by this PR, all unit tests passed on my environment:

...
235/295 Test #235: mtt-mr-rpma_mr_reg_0_helgrind ................................   Passed    1.09 sec
        Start 236: mtt-mr-rpma_mr_remote_from_descriptor_0_none
236/295 Test #236: mtt-mr-rpma_mr_remote_from_descriptor_0_none .................   Passed    0.15 sec
        Start 237: mtt-mr-rpma_mr_remote_from_descriptor_0_memcheck
237/295 Test #237: mtt-mr-rpma_mr_remote_from_descriptor_0_memcheck .............   Passed    3.05 sec
...
293/295 Test #293: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_memcheck ...   Passed    2.37 sec
        Start 294: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_drd
294/295 Test #294: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_drd ........   Passed    1.19 sec
        Start 295: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_helgrind
295/295 Test #295: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_helgrind ...   Passed    1.16 sec

100% tests passed, 0 tests failed out of 295

Total Test time (real) = 271.63 sec

Did I miss something?


@ldorau ldorau requested a review from grom72 January 3, 2023 08:55
Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

It seems not be caused by this PR, all unit tests passed on my environment:

...
235/295 Test #235: mtt-mr-rpma_mr_reg_0_helgrind ................................   Passed    1.09 sec
        Start 236: mtt-mr-rpma_mr_remote_from_descriptor_0_none
236/295 Test #236: mtt-mr-rpma_mr_remote_from_descriptor_0_none .................   Passed    0.15 sec
        Start 237: mtt-mr-rpma_mr_remote_from_descriptor_0_memcheck
237/295 Test #237: mtt-mr-rpma_mr_remote_from_descriptor_0_memcheck .............   Passed    3.05 sec
...
293/295 Test #293: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_memcheck ...   Passed    2.37 sec
        Start 294: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_drd
294/295 Test #294: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_drd ........   Passed    1.19 sec
        Start 295: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_helgrind
295/295 Test #295: mtt-utils-rpma_utils_ibv_context_is_odp_capable_0_helgrind ...   Passed    1.16 sec

100% tests passed, 0 tests failed out of 295

Total Test time (real) = 271.63 sec

Did I miss something?

These errors have nothing to do with this PR. They occurs only on Circle CI.



CHANGELOG.md line 21 at r9 (raw file):

- one suppression for Memcheck on Ubuntu 22.04
- CI Coverity build run once a day over the night
- check if libibverbs supports native atomic write

checking ...

(The section "Added" contains "things" (nouns) that have been added)

Code quote:

check if libibverbs supports native atomic write

CHANGELOG.md line 23 at r9 (raw file):

- check if libibverbs supports native atomic write
- internal APIs:
  - rpma_utils_ibv_context_is_atomic_write_capable() - check if kernel supports native atomic write

checks if kernel ...

Code quote:

check if kernel

CHANGELOG.md line 45 at r9 (raw file):

- call rpma_utils_ibv_context_is_atomic_write_capable() in rpma_peer_new()
- enable native atomic write in rpma_peer_setup_qp() if both kernel and libibverbs support it
- use native atomic write in rpma_mr_atomic_write() if the created QP supports it
- rpma_utils_ibv_context_is_atomic_write_capable() called in rpma_peer_new()
- native atomic write enabled in rpma_peer_setup_qp() if both kernel and libibverbs support it
- native atomic write used in rpma_mr_atomic_write() if the created QP supports it

(The section "Changed" contains things that were/have been changed - please use Past/Present Perfect Tense)

Code quote:

- call rpma_utils_ibv_context_is_atomic_write_capable() in rpma_peer_new()
- enable native atomic write in rpma_peer_setup_qp() if both kernel and libibverbs support it
- use native atomic write in rpma_mr_atomic_write() if the created QP supports it

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

These errors have nothing to do with this PR. They occurs only on Circle CI.

This is the fix for this issue: #2087


Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @yangx-jy)

a discussion (no related file):
The #2087 fix merged - rebase please.


@yangx-jy yangx-jy force-pushed the libibverbs_atomic_write branch 2 times, most recently from cd86ee6 to 6257609 Compare January 3, 2023 11:54
Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 23 of 24 files reviewed, 5 unresolved discussions (waiting on @grom72 and @ldorau)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

This is the fix for this issue: #2087

Cool.


a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

The #2087 fix merged - rebase please.

Done.



CHANGELOG.md line 21 at r9 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

checking ...

(The section "Added" contains "things" (nouns) that have been added)

Done.


CHANGELOG.md line 23 at r9 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

checks if kernel ...

Done.


CHANGELOG.md line 45 at r9 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…
- rpma_utils_ibv_context_is_atomic_write_capable() called in rpma_peer_new()
- native atomic write enabled in rpma_peer_setup_qp() if both kernel and libibverbs support it
- native atomic write used in rpma_mr_atomic_write() if the created QP supports it

(The section "Changed" contains things that were/have been changed - please use Past/Present Perfect Tense)

Done.

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @yangx-jy)


CHANGELOG.md line 21 at r9 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Done.

"a check if libibverbs supports native atomic write"
or
"a check for the native atomic write support in libibverbs"

Code quote:

check if libibverbs supports native atomic write

Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
This internal function is to check if kernel supports native
atomic write.

Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
Check if kernel supports native atomic write in rpma_peer_new()
and then save the result into the is_native_atomic_write_supported
member of struct rpma_peer.

Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
rpma_peer_setup_qp() enables native atomic write if
both kernel and libibverbs support it.

Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
rpma_mr_atomic_write() uses native atomic write if
the created QP supports it.

Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
Copy link
Contributor Author

@yangx-jy yangx-jy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 23 of 24 files reviewed, 2 unresolved discussions (waiting on @grom72 and @ldorau)


CHANGELOG.md line 21 at r9 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

"a check if libibverbs supports native atomic write"
or
"a check for the native atomic write support in libibverbs"

Done.

Copy link
Member

@ldorau ldorau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

a discussion (no related file):

Previously, yangx-jy (Xiao Yang) wrote…

Cool.

:lgtm:



CHANGELOG.md line 21 at r9 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Done.

Thanks!

@ldorau ldorau merged commit 7774636 into pmem:main Jan 3, 2023
Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants