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

Update compact dataset I/O routines to handle driver-level memory copy #1054

Merged
merged 3 commits into from
Sep 30, 2021

Conversation

jhendersonHDF
Copy link
Collaborator

@jhendersonHDF jhendersonHDF commented Sep 29, 2021

These changes are needed to be compatible with the GPU VFD. Since the VFD uses buffers that are allocated via cudaMalloc, which aren't compatible with memcpy from the standard library, HDF5 needs to be able to request that a driver handle a memory copy if necessary.

src/H5Dcompact.c Outdated Show resolved Hide resolved
src/H5Dcompact.c Outdated
FUNC_ENTER_STATIC

/* Retrieve lower-level file handle for I/O */
if (H5F_shared_get_file_handle(udata->f_sh, &file_handle) < 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, we need a handle to the shared file structure's H5FD_t pointer. However, the H5F package hides this behind an incomplete typedef, so we can't just directly access it through udata->f_sh->lf. We have to create a routine in H5Fquery.c to give this back to us.

@@ -185,6 +193,9 @@
#define H5FD_CTL__GET_MPI_COMMUNICATOR_OPCODE 2
#define H5FD_CTL__GET_MPI_RANK_OPCODE 3
#define H5FD_CTL__GET_MPI_SIZE_OPCODE 4
#define H5FD_CTL__MEM_ALLOC 5
#define H5FD_CTL__MEM_FREE 6
#define H5FD_CTL__MEM_COPY 7
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here, I only reserved the op codes for driver-level memory allocations and frees. I haven't added any argument structs for them since there isn't a use case yet, but can do so if it's useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree -- wait until we have use cases.

/* Private headers needed by this file */
#include "H5MMprivate.h" /* Memory management */
#include "H5FDprivate.h" /* File drivers */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since H5F_shared_get_file_handles needs the definition for the H5FD_t type, and since H5FDprivate.h includes H5FDpublic.h, just include the private header rather than both here.

@jhendersonHDF jhendersonHDF force-pushed the gpu_vfd_updates branch 2 times, most recently from c56b30f to 1b2fc54 Compare September 29, 2021 21:12
Rename CTL memory copy flag and H5Fquery routine to get file driver
structure
@byrnHDF
Copy link
Contributor

byrnHDF commented Sep 29, 2021

Should I assume all these changes to develop have had someone add release.txt entries?

@jhendersonHDF
Copy link
Collaborator Author

Should I assume all these changes to develop have had someone add release.txt entries?

It looks like the VFD plugins PR didn't have a RELEASE.txt entry associated with it. Let me create that in a separate PR and then add a RELEASE.txt entry for this PR here.

Copy link
Contributor

@jrmainzer jrmainzer left a comment

Choose a reason for hiding this comment

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

VFD code looks good.
No obvious issues with H5D code, but then I'm not as familiar with it as I would like.

@lrknox lrknox merged commit 5b5f0c0 into HDFGroup:develop Sep 30, 2021
@jhendersonHDF jhendersonHDF deleted the gpu_vfd_updates branch January 12, 2022 23:19
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.

7 participants