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

Fix a bug in Subfiling VFD vector I/O setup #4821

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Fix a bug in Subfiling VFD vector I/O setup #4821

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a3ccd2
Fix a bug in Subfiling VFD vector I/O setup
jhendersonHDF Sep 10, 2024
a1f4d43
Add a release note
jhendersonHDF Sep 11, 2024
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
117 changes: 71 additions & 46 deletions src/H5FDsubfiling/H5FDsubfiling.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ static herr_t H5FD__subfiling_mirror_writes_to_stub(H5FD_subfiling_t *file, uint
const void *bufs[]);
static herr_t H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_count,
H5FD_mem_t types[], haddr_t file_offsets[],
size_t nelemts[], H5_flexible_const_ptr_t bufs[],
H5FD_subfiling_io_type_t io_type, size_t *ioreq_count,
uint32_t *iovec_len, H5FD_mem_t **io_types,
haddr_t **io_addrs, size_t **io_sizes,
H5_flexible_const_ptr_t **io_bufs);
static void H5FD__subfiling_get_iovec_sizes(subfiling_context_t *sf_context, size_t in_count,
haddr_t file_offsets[], size_t nelemts[], size_t *max_iovec_depth,
size_t *max_num_subfiles);
size_t io_sizes[], H5_flexible_const_ptr_t bufs[],
H5FD_subfiling_io_type_t io_type, size_t *ioreq_count_out,
uint32_t *iovec_len_out, H5FD_mem_t **io_types_out,
haddr_t **io_addrs_out, size_t **io_sizes_out,
H5_flexible_const_ptr_t **io_bufs_out);
static herr_t H5FD__subfiling_get_iovec_sizes(subfiling_context_t *sf_context, size_t in_count,
haddr_t file_offsets[], size_t io_sizes[],
size_t *max_iovec_depth, size_t *max_num_subfiles);
static herr_t H5FD__subfiling_translate_io_req_to_iovec(
subfiling_context_t *sf_context, size_t iovec_idx, size_t iovec_len, size_t iovec_count, H5FD_mem_t type,
haddr_t addr, size_t io_size, H5_flexible_const_ptr_t io_buf, H5FD_subfiling_io_type_t io_type,
Expand Down Expand Up @@ -2023,7 +2023,7 @@ H5FD__subfiling_io_helper(H5FD_subfiling_t *file, size_t io_count, H5FD_mem_t ty
io_count, /* IN: Number of entries in `types`, `addrs`, `sizes` and `bufs` */
types, /* IN: Array of memory types */
addrs, /* IN: Array of starting file offsets */
sizes, /* IN: Array of I/O sizes (in terms of elements) */
sizes, /* IN: Array of I/O sizes */
bufs, /* IN: Array of I/O buffers */
io_type, /* IN: Type of I/O being performed (IO_TYPE_WRITE or IO_TYPE_READ) */
&ioreq_count, /* OUT: Number of I/O requests to be made */
Expand Down Expand Up @@ -2335,30 +2335,30 @@ H5FD__subfiling_mirror_writes_to_stub(H5FD_subfiling_t *file, uint32_t count, H5
* - the type of I/O being performed (IO_TYPE_WRITE or
* IO_TYPE_READ)
*
* ioreq_count (OUT)
* ioreq_count_out (OUT)
* - the number of I/O requests needed to fully satisfy the
* I/O operation
*
* iovec_len (OUT)
* iovec_len_out (OUT)
* - the size of each I/O vector (in terms of array elements)
* for each I/O request to be made
*
* io_types (OUT)
* io_types_out (OUT)
* - I/O vector of memory types for the I/O operation.
* Allocated by this function and must be freed by the
* caller.
*
* io_addrs (OUT)
* io_addrs_out (OUT)
* - I/O vector of file addresses for the I/O operation.
* Allocated by this function and must be freed by the
* caller.
*
* io_sizes (OUT)
* io_sizes_out (OUT)
* - I/O vector of the I/O sizes for the I/O operation.
* Allocated by this function and must be freed by the
* caller.
*
* io_bufs (OUT)
* io_bufs_out (OUT)
* - I/O vector of the I/O buffers for the I/O operation.
* Allocated by this function and must be freed by the
* caller.
Expand All @@ -2368,10 +2368,11 @@ H5FD__subfiling_mirror_writes_to_stub(H5FD_subfiling_t *file, uint32_t count, H5
*/
static herr_t
H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_count, H5FD_mem_t types[],
haddr_t file_offsets[], size_t nelemts[], H5_flexible_const_ptr_t bufs[],
H5FD_subfiling_io_type_t io_type, size_t *ioreq_count,
uint32_t *iovec_len, H5FD_mem_t **io_types, haddr_t **io_addrs,
size_t **io_sizes, H5_flexible_const_ptr_t **io_bufs)
haddr_t file_offsets[], size_t io_sizes[], H5_flexible_const_ptr_t bufs[],
H5FD_subfiling_io_type_t io_type, size_t *ioreq_count_out,
uint32_t *iovec_len_out, H5FD_mem_t **io_types_out,
haddr_t **io_addrs_out, size_t **io_sizes_out,
H5_flexible_const_ptr_t **io_bufs_out)
{
H5_flexible_const_ptr_t *loc_io_bufs = NULL;
H5FD_mem_t *loc_io_types = NULL;
Expand All @@ -2395,18 +2396,18 @@ H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_c
assert(sf_context->topology);
assert(types || in_count == 0);
assert(file_offsets || in_count == 0);
assert(nelemts || in_count == 0);
assert(io_sizes || in_count == 0);
assert(bufs || in_count == 0);
assert(ioreq_count);
assert(iovec_len);
assert(io_types);
assert(io_addrs);
assert(io_sizes);
assert(io_bufs);
assert(ioreq_count_out);
assert(iovec_len_out);
assert(io_types_out);
assert(io_addrs_out);
assert(io_sizes_out);
assert(io_bufs_out);

/* Set some returned values early */
*ioreq_count = 0;
*iovec_len = 0;
*ioreq_count_out = 0;
*iovec_len_out = 0;

/* Nothing to do */
if (in_count == 0)
Expand All @@ -2416,11 +2417,16 @@ H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_c
* Do some initial pre-processing to determine how large of I/O vectors we
* will need to allocate to satisfy the entire I/O request
*/
H5FD__subfiling_get_iovec_sizes(sf_context, in_count, file_offsets, nelemts, &max_iovec_depth,
&max_num_subfiles_touched);
if (H5FD__subfiling_get_iovec_sizes(sf_context, in_count, file_offsets, io_sizes, &max_iovec_depth,
&max_num_subfiles_touched) < 0)
HGOTO_ERROR(H5E_VFL, H5E_CANTGET, FAIL, "can't determine maximum I/O request size");

tot_iovec_len = in_count * max_iovec_depth * max_num_subfiles_touched;

/* Nothing to do */
if (tot_iovec_len == 0)
HGOTO_DONE(SUCCEED);

#ifdef H5_SUBFILING_DEBUG
H5FD__subfiling_log(
sf_context->sf_context_id,
Expand Down Expand Up @@ -2456,10 +2462,10 @@ H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_c
}

if (!extend_sizes) {
if (io_idx > 0 && nelemts[io_idx] == 0)
if (io_idx > 0 && io_sizes[io_idx] == 0)
extend_sizes = true;
else
io_size = nelemts[io_idx];
io_size = io_sizes[io_idx];
}

if (H5FD__subfiling_translate_io_req_to_iovec(sf_context, iovec_idx, max_num_subfiles_touched,
Expand All @@ -2469,13 +2475,13 @@ H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_c
HGOTO_ERROR(H5E_VFL, H5E_CANTINIT, FAIL, "can't translate I/O request to I/O vectors");
}

*ioreq_count = in_count * max_iovec_depth;
*ioreq_count_out = in_count * max_iovec_depth;
H5_CHECK_OVERFLOW(max_num_subfiles_touched, size_t, uint32_t);
*iovec_len = (uint32_t)max_num_subfiles_touched;
*io_types = loc_io_types;
*io_addrs = loc_io_addrs;
*io_sizes = loc_io_sizes;
*io_bufs = loc_io_bufs;
*iovec_len_out = (uint32_t)max_num_subfiles_touched;
*io_types_out = loc_io_types;
*io_addrs_out = loc_io_addrs;
*io_sizes_out = loc_io_sizes;
*io_bufs_out = loc_io_bufs;

done:
if (ret_value < 0) {
Expand All @@ -2497,26 +2503,28 @@ H5FD__subfiling_generate_io_vectors(subfiling_context_t *sf_context, size_t in_c
* info is used to calculate the total size of I/O vectors we
* need to allocate to satisfy an entire I/O request.
*
* Return: Maximum I/O vector depth and maximum number of subfiles
* touched (can't fail)
* Return: Non-negative on success/negative on failure
*
*-------------------------------------------------------------------------
*/
static void
static herr_t
H5FD__subfiling_get_iovec_sizes(subfiling_context_t *sf_context, size_t in_count, haddr_t file_offsets[],
size_t nelemts[], size_t *max_iovec_depth, size_t *max_num_subfiles)
size_t io_sizes[], size_t *max_iovec_depth, size_t *max_num_subfiles)
{
int64_t stripe_size = 0;
int64_t block_size = 0;
size_t loc_max_iovec_depth = 0;
size_t loc_max_num_subfiles = 0;
size_t io_size = 0;
bool extend_sizes = false;
int num_subfiles = 0;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE_NOERR
FUNC_ENTER_PACKAGE

assert(sf_context);
assert(file_offsets);
assert(nelemts);
assert(io_sizes);
assert(max_iovec_depth);
assert(max_num_subfiles);

Expand All @@ -2538,7 +2546,23 @@ H5FD__subfiling_get_iovec_sizes(subfiling_context_t *sf_context, size_t in_count
size_t cur_iovec_depth;

H5_CHECKED_ASSIGN(cur_file_offset, int64_t, file_offsets[io_idx], haddr_t);
H5_CHECKED_ASSIGN(data_size, int64_t, nelemts[io_idx], size_t);

if (!extend_sizes) {
if (io_idx > 0 && io_sizes[io_idx] == 0)
extend_sizes = true;
else
io_size = io_sizes[io_idx];
}

H5_CHECKED_ASSIGN(data_size, int64_t, io_size, size_t);

if (cur_file_offset < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL,
"file offset of %" PRIuHADDR " at index %zu too large; wrapped around",
file_offsets[io_idx], io_idx);
if (data_size < 0)
HGOTO_ERROR(H5E_VFL, H5E_BADVALUE, FAIL, "I/O size of %zu at index %zu too large; wrapped around",
io_size, io_idx);

/*
* Calculate the following from the starting file offset:
Expand Down Expand Up @@ -2645,7 +2669,8 @@ H5FD__subfiling_get_iovec_sizes(subfiling_context_t *sf_context, size_t in_count
*max_iovec_depth = loc_max_iovec_depth;
*max_num_subfiles = loc_max_num_subfiles;

FUNC_LEAVE_NOAPI_VOID
done:
FUNC_LEAVE_NOAPI(ret_value)
}

/*-------------------------------------------------------------------------
Expand Down
116 changes: 116 additions & 0 deletions testpar/t_subfiling_vfd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ static void test_read_different_stripe_size(void);
static void test_subfiling_precreate_rank_0(void);
static void test_subfiling_write_many_read_one(void);
static void test_subfiling_write_many_read_few(void);
static void test_subfiling_vector_io_extension(void);
static void test_subfiling_h5fuse(void);

static test_func tests[] = {
Expand All @@ -118,6 +119,7 @@ static test_func tests[] = {
test_subfiling_precreate_rank_0,
test_subfiling_write_many_read_one,
test_subfiling_write_many_read_few,
test_subfiling_vector_io_extension,
test_subfiling_h5fuse,
};

Expand Down Expand Up @@ -2568,6 +2570,120 @@ test_subfiling_write_many_read_few(void)
#undef SUBF_HDF5_TYPE
#undef SUBF_C_TYPE

/*
* Test to check for a bug where the vector I/O sizes
* array wasn't being extended when an entry in the
* array was 0.
*/
#define SUBF_FILENAME "test_subfiling_vector_io_extension.h5"
#define SUBF_C_TYPE int
static void
test_subfiling_vector_io_extension(void)
{
H5FD_subfiling_params_t cfg;
h5_stat_size_t file_size;
SUBF_C_TYPE *read_buf = NULL;
H5FD_mem_t *types = NULL;
h5_stat_t file_info;
uint32_t count = 64;
haddr_t *addrs = NULL;
haddr_t file_end_addr;
herr_t read_status;
size_t *sizes = NULL;
H5FD_t *file_ptr = NULL;
hid_t file_id = H5I_INVALID_HID;
hid_t fapl_id = H5I_INVALID_HID;
hid_t dxpl_id = H5I_INVALID_HID;
void **bufs = NULL;

curr_nerrors = nerrors;

if (MAINPROCESS)
TESTING_2("I/O vector size extension functionality");

/* Must use at least 2 subfiles to cause generation of
* I/O vectors within the VFD.
*/
cfg.ioc_selection = SELECT_IOC_ONE_PER_NODE;
cfg.stripe_size = (stripe_size_g > 0) ? stripe_size_g : 1048576;
cfg.stripe_count = num_iocs_g > 1 ? num_iocs_g : 2;

fapl_id = create_subfiling_ioc_fapl(comm_g, info_g, true, &cfg, H5FD_IOC_DEFAULT_THREAD_POOL_SIZE);
VRFY((fapl_id >= 0), "FAPL creation succeeded");

file_id = H5Fcreate(SUBF_FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, fapl_id);
VRFY((file_id >= 0), "H5Fcreate succeeded");

VRFY((H5Fclose(file_id) >= 0), "File close succeeded");

/* Re-open file through H5FDopen for direct reads */
file_ptr = H5FDopen(SUBF_FILENAME, H5F_ACC_RDWR, fapl_id, HADDR_UNDEF);
VRFY(file_ptr, "H5FDopen succeeded");

/*
* Get the current file size to see where we can safely
* write to in the file without overwriting the superblock
*/
memset(&file_info, 0, sizeof(h5_stat_t));
VRFY((HDstat(SUBF_FILENAME, &file_info) >= 0), "HDstat succeeded");
file_size = (h5_stat_size_t)file_info.st_size;

H5_CHECK_OVERFLOW(file_size, h5_stat_size_t, haddr_t);
file_end_addr = (haddr_t)file_size;

dxpl_id = H5Pcreate(H5P_DATASET_XFER);
VRFY((dxpl_id >= 0), "DXPL creation succeeded");

/* Set independent I/O on DXPL */
VRFY((H5Pset_dxpl_mpio(dxpl_id, H5FD_MPIO_INDEPENDENT) >= 0), "H5Pset_dxpl_mpio succeeded");

/* Set EOA for following read call */
VRFY((H5FDset_eoa(file_ptr, H5FD_MEM_DEFAULT, file_end_addr + (count * sizeof(int))) >= 0),
"H5FDset_eoa succeeded");

read_buf = malloc(count * sizeof(*read_buf));
types = malloc(count * sizeof(*types));
addrs = malloc(count * sizeof(*addrs));
sizes = malloc(2 * sizeof(size_t));
bufs = malloc(count * sizeof(*bufs));

sizes[0] = sizeof(SUBF_C_TYPE);
sizes[1] = 0;

for (size_t i = 0; i < count; i++) {
types[i] = H5FD_MEM_DRAW;
addrs[i] = file_end_addr + (i * sizeof(SUBF_C_TYPE));
bufs[i] = (void *)&(read_buf[i]);
}

read_status = H5FDread_vector(file_ptr, dxpl_id, count, types, addrs, sizes, bufs);
VRFY((read_status >= 0), "H5FDread_vector succeeded");

VRFY((H5FDclose(file_ptr) >= 0), "H5FDclose succeeded");

mpi_code_g = MPI_Barrier(comm_g);
VRFY((mpi_code_g == MPI_SUCCESS), "MPI_Barrier succeeded");

H5E_BEGIN_TRY
{
H5Fdelete(SUBF_FILENAME, fapl_id);
}
H5E_END_TRY

VRFY((H5Pclose(fapl_id) >= 0), "FAPL close succeeded");
VRFY((H5Pclose(dxpl_id) >= 0), "DXPL close succeeded");

free(bufs);
free(sizes);
free(addrs);
free(types);
free(read_buf);

CHECK_PASSED();
}
#undef SUBF_FILENAME
#undef SUBF_C_TYPE

/*
* Test that the subfiling file can be read with the
* sec2 driver after being fused back together with
Expand Down
Loading