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.

Sign up for GitHub

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

Jump to bottom

UCT/GDR_COPY: Fix gdr_copy registration issues #3044

Merged
merged 3 commits into from
Nov 18, 2018
Merged

UCT/GDR_COPY: Fix gdr_copy registration issues #3044

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57c8744
UCT/GDR_COPY: align address to GPU PAGE SIZE when rcache is turned off
bureddy Nov 15, 2018
3b7f72e
UCT/GDR_COPY: avoid registering whole allocation addr range
bureddy Nov 15, 2018
bbd0058
UCT/GDR_COPY: Fix review commnets
bureddy Nov 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions src/uct/cuda/gdr_copy/gdr_copy_md.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <limits.h>
#include <ucs/debug/log.h>
#include <ucs/sys/sys.h>
#include <ucs/sys/math.h>
#include <ucs/debug/memtrack.h>
#include <ucs/type/class.h>
#include <ucm/api/ucm.h>
Expand Down Expand Up @@ -178,8 +179,7 @@ static ucs_status_t uct_gdr_copy_mem_reg(uct_md_h uct_md, void *address, size_t
unsigned flags, uct_mem_h *memh_p)
{
uct_gdr_copy_mem_t *mem_hndl = NULL;
size_t reg_size;
void *ptr;
void *start, *end;
ucs_status_t status;

mem_hndl = ucs_malloc(sizeof(uct_gdr_copy_mem_t), "gdr_copy handle");
Expand All @@ -188,10 +188,11 @@ static ucs_status_t uct_gdr_copy_mem_reg(uct_md_h uct_md, void *address, size_t
return UCS_ERR_NO_MEMORY;
}

reg_size = (length + GPU_PAGE_SIZE - 1) & GPU_PAGE_MASK;
ptr = (void *) ((uintptr_t)address & GPU_PAGE_MASK);
start = ucs_align_down_pow2_ptr(address, GPU_PAGE_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Generic question - is this safe to use compile time GPU_PAGE_SIZE ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@shamisp Do you mean this in the context of #if HAVE_CUDA?
If so, GPU_PAGE_SIZE is defined in gdrapi.h and gets included #if HAVE_CUDA. It should be safe, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is NVIDIA's gdr_copy constant https://github.com/NVIDIA/gdrcopy/blob/master/gdrapi.h#L35
Looks like it is safe for current nvidia architectures.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just wanted to make sure that it is the same value (64K) across all architectures.

end = ucs_align_up_pow2_ptr(address + length, GPU_PAGE_SIZE);
ucs_assert_always(start <= end);

status = uct_gdr_copy_mem_reg_internal(uct_md, ptr, reg_size, 0, mem_hndl);
status = uct_gdr_copy_mem_reg_internal(uct_md, start, end - start, 0, mem_hndl);
if (status != UCS_OK) {
ucs_free(mem_hndl);
return status;
Expand Down Expand Up @@ -283,17 +284,8 @@ uct_gdr_copy_mem_rcache_reg(uct_md_h uct_md, void *address, size_t length,
ucs_rcache_region_t *rregion;
ucs_status_t status;
uct_gdr_copy_mem_t *memh;
CUdeviceptr d_ptr;
size_t d_length;


status = UCT_CUDADRV_FUNC(cuMemGetAddressRange(&d_ptr, &d_length,
(CUdeviceptr)address));
if (status != UCS_OK) {
return status;
}

status = ucs_rcache_get(md->rcache, (void *)d_ptr, d_length, PROT_READ|PROT_WRITE,
status = ucs_rcache_get(md->rcache, (void *)address, length, PROT_READ|PROT_WRITE,
&flags, &rregion);
if (status != UCS_OK) {
return status;
Expand Down