Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UCT/IB/MLX5/DV: device memory allocation debug logs #10495

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roiedanino
Copy link
Contributor

What?

Adding missing debug logs to IB device memory allocations.

Why?

Getting a better indication of whether device memory was used in runtime and the cause.

@roiedanino roiedanino self-assigned this Feb 16, 2025
@roiedanino roiedanino requested a review from yosefe February 16, 2025 21:10
Comment on lines +2074 to +2075
uct_ib_device_name(&md->dev), alloc_name, address,
UCS_PTR_BYTE_OFFSET(address, dm_attr.length), dm_attr.length, dm,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need to print both end address and length? IMO length is enough

@@ -2068,6 +2068,13 @@ uct_ib_mlx5_devx_device_mem_alloc(uct_md_h uct_md, size_t *length_p,
*length_p = dm_attr.length;
*address_p = address;
*memh_p = memh;

ucs_debug("%s: allocated device memory (%s) %p..%p length %zu, dm %p, mkey "
Copy link
Contributor

Choose a reason for hiding this comment

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

pls remove the ,

uct_ib_device_name(&md->super.dev));

#else
ucs_debug("UCX was compiled without --with-dm configuration");
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this debug log is not needed as we don't usually have such messages, and we can know the compilation defines from ucx_info -b

Comment on lines +2177 to +2178
ucs_debug("%s: KSM over device memory is supported",
uct_ib_device_name(&md->super.dev));
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. "%s: device memory allocation is supported"
  2. move this log after line 2167? we could get here also if uct_ib_mlx5_devx_reg_atomic_key failed

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

Successfully merging this pull request may close these issues.

3 participants