Skip to content

Commit

Permalink
Addressing feedback on PR
Browse files Browse the repository at this point in the history
Addressing issues I had in my initial PR based on review feedback. I
also did some general code clean up for parse_func.c.

I upadted the test_xdd_file_datapattern test for wholefile to be more
stringent in the way it is tested. I wanted to make sure that two
threads were used fully test out the functionailty.
This commit needs to be squashed into the original commit if this PR
is accepted. I will take care of that before the merge.

Signed-off-by: Brian Atkinson <batkinson@lanl.gov>
  • Loading branch information
bwatkinson committed Mar 20, 2024
1 parent daeacbb commit 7274cc3
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 52 deletions.
67 changes: 26 additions & 41 deletions src/client/parse_func.c
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ xddfunc_datapattern(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flag
}
} else if (strcmp(pattern_type, "file") == 0 || strcmp(pattern_type, "wholefile") == 0) {
retval++;
int dp_fd;
int dp_fd = -1;

if (argc <= args+2) {
fprintf(xgp->errout, "%s: not enough arguments specified for the option '-datapattern'\n", xgp->progname);
Expand All @@ -568,63 +568,48 @@ xddfunc_datapattern(xdd_plan_t *planp, int32_t argc, char *argv[], uint32_t flag
(char *)argv[args+2]);
return(0);
}
pattern_length = stat_buf.st_size;

}
if (tdp) { /* set option for specific target */
if (strcmp(pattern_type, "file") == 0) {
tdp->td_dpp->data_pattern_options |= DP_FILE_PATTERN;
} else {// wholefile
tdp->td_dpp->data_pattern_options |= DP_WHOLEFILE_PATTERN;
if (!xdd_datapattern_wholefile_enough_ram(tdp, argv[args+2])) {
return(0);
}
}
tdp->td_dpp->data_pattern_filename = (char *)argv[args+2];
tdp->td_dpp->data_pattern_length = pattern_length;
if (!xdd_datapattern_wholefile_enough_ram(tdp)) {
fprintf(xgp->errout, "%s: file %s can not be loaded into memory because 50%% of RAM is "
"currently unavailable\n",
xgp->progname,
(char *)argv[args+2]);
return(0);
}
dp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY);
if (dp_fd < 0) {
fprintf(xgp->errout, "%s: could not open %s\n", xgp->progname, tdp->td_dpp->data_pattern_filename);
return(0);
tdp->td_dpp->data_pattern_length = stat_buf.st_size;
if(xdd_set_datapattern_from_filename(tdp, argv[args+2])) {
return (0);
}
tdp->td_dpp->data_pattern = (unsigned char *)malloc(sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1));
memset(tdp->td_dpp->data_pattern, '\0', sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1));
pread(dp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0);
close(dp_fd);
} else {// Put this option into all Targets
} else {// Put this option into all Targets
if (flags & XDD_PARSE_PHASE2) {
tdp = planp->target_datap[0];
tdp->td_dpp->data_pattern_length = stat_buf.st_size;
i = 0;
while (tdp) {
if (strcmp(pattern_type, "file") == 0) {
tdp->td_dpp->data_pattern_options |= DP_FILE_PATTERN;
} else {//wholefile
tdp->td_dpp->data_pattern_options |= DP_WHOLEFILE_PATTERN;
}
tdp->td_dpp->data_pattern_filename = (char *)argv[args+2];
tdp->td_dpp->data_pattern_length = pattern_length;
if (!xdd_datapattern_wholefile_enough_ram(tdp)) {
fprintf(xgp->errout, "%s: the file %s is larger than 50%% of total RAM\n",
xgp->progname,
(char *)argv[args+2]);
dp_fd = open(argv[args+2], O_RDONLY);
if (dp_fd < 0) {
fprintf(xgp->errout, "%s: could not open %s\n", xgp->progname, argv[args+2]);
return (0);
}
if (strcmp(pattern_type, "file") == 0) {
tdp->td_dpp->data_pattern_options |= DP_FILE_PATTERN;
} else {//wholefile
tdp->td_dpp->data_pattern_options |= DP_WHOLEFILE_PATTERN;
if (!xdd_datapattern_wholefile_enough_ram(tdp, argv[args+2])) {
return(0);
}
dp_fd = open(tdp->td_dpp->data_pattern_filename, O_RDONLY);
if (dp_fd < 0) {
fprintf(xgp->errout, "%s: could not open %s\n", xgp->progname, tdp->td_dpp->data_pattern_filename);
return(0);
}
while (tdp) {
if (xdd_set_datapattern_from_file_descriptor(tdp, dp_fd, argv[args+2])) {
close(dp_fd);
return (0);
}
tdp->td_dpp->data_pattern_length = pattern_length;
tdp->td_dpp->data_pattern = (unsigned char *)malloc(sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1));
memset(tdp->td_dpp->data_pattern, '\0', sizeof(unsigned char) * (tdp->td_dpp->data_pattern_length + 1));
pread(dp_fd, tdp->td_dpp->data_pattern, tdp->td_dpp->data_pattern_length, 0);
close(dp_fd);
i++;
tdp = planp->target_datap[i];
}
close(dp_fd);
}
}
} else if (strcmp(pattern_type, "sequenced") == 0) {
Expand Down
82 changes: 77 additions & 5 deletions src/common/datapatterns.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,22 +257,94 @@ xdd_datapattern_fill(worker_data_t *wdp) {
* WHOLEFILE_MAX_SIZE_RAM.
*/
int
xdd_datapattern_wholefile_enough_ram(target_data_t *tdp) {
int ret = TRUE;
xdd_datapattern_wholefile_enough_ram(target_data_t *tdp, const char *filename) {
struct sysinfo info;
size_t file_size = tdp->td_dpp->data_pattern_length;

assert(tdp->td_dpp->data_pattern_options & DP_WHOLEFILE_PATTERN);

// Short circuit empty files
if (!file_size)
return FALSE;
goto error;

sysinfo(&info);
if (file_size > (info.freeram * WHOLEFILE_MAX_SIZE_RAM))
ret = FALSE;
goto error;;

return ret;
return TRUE;

error:
fprintf(xgp->errout, "%s: file %s can not be loaded into memory because 50%% of RAM is "
"currently unavailable\n",
xgp->progname,
filename);

return FALSE;
} // End of xdd_datapattern_wholefile_enough_ram()

/*----------------------------------------------------------------------------*/
/* xdd_set_datapattern_from_filename() - This subroutine will set the targets
* data_pattern from a file. The targets td_dpp->data_pattern_length must be
* set before calling this. On error returns 1, and on success returns 0.
*/
int
xdd_set_datapattern_from_filename(target_data_t *tdp, char *filename) {
struct xint_data_pattern *dp = tdp->td_dpp;
int ret = 0;

int fd = open(filename, O_RDONLY);
if (fd < 0 ) {
fprintf(xgp->errout, "%s, could not open %s\n", xgp->progname, filename);
return 1;
}

dp->data_pattern_filename = filename;
ret = xdd_set_datapattern_from_file_descriptor(tdp, fd, filename);
close (fd);

return ret;
} // End of xdd_set_datapattern_from_filename()

/*----------------------------------------------------------------------------*/
/* xdd_set_datapattern_from_file_descriptor() - This subroutine will set the
* targets data_pattern from an open file descriptor. The targets
* td_dpp->data_pattern_length must be set before calling this. On error returns 1,
* and on success returns 0.
*/
int
xdd_set_datapattern_from_file_descriptor(target_data_t *tdp, int fd, char *filename) {
struct xint_data_pattern *dp = tdp->td_dpp;

// Short circuit if the file is not open
if (fd < 0) {
return 1;
}

dp->data_pattern_filename = filename;
dp->data_pattern = (unsigned char *)malloc(sizeof(unsigned char) * dp->data_pattern_length + 1) ;
if (!dp->data_pattern) {
fprintf(xgp->errout, "%s: could not allocate datapattern buffer for target %d",
xgp->progname, tdp->td_target_number);
goto error;
}

memset(dp->data_pattern, '\0', sizeof(unsigned char) * dp->data_pattern_length + 1);
ssize_t bytes = pread(fd, dp->data_pattern, dp->data_pattern_length, 0);
if (bytes != dp->data_pattern_length) {
fprintf(xgp->errout, "%s: short read from file %s when setting target datapattern. "
"Espected %d bytes but got %d",
xgp->progname, filename, dp->data_pattern_length, bytes);
goto error;
}

return 0;
error:
if (dp->data_pattern) {
free(dp->data_pattern);
}
return 1;
} // End of xdd_set_datapattern_from_file_descriptor()

/*----------------------------------------------------------------------------*/
/* xdd_datapattern_get_datap_from_offset() - This subroutine will return the
* beginning of a the buffer from the target's tp_dpp->data_pattern based on
Expand Down
4 changes: 3 additions & 1 deletion src/common/xint_prototypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ int32_t xdd_barrier(struct xdd_barrier *bp, xdd_occupant_t *occupantp, char owne
// datapatterns.c
void xdd_datapattern_buffer_init(worker_data_t *wdp);
void xdd_datapattern_fill(worker_data_t *wdp);
int xdd_datapattern_wholefile_enough_ram(target_data_t *tdp);
int xdd_datapattern_wholefile_enough_ram(target_data_t *tdp, const char *filename);
int xdd_set_datapattern_from_filename(target_data_t *tdp, char *filename);
int xdd_set_datapattern_from_file_descriptor(target_data_t *tdp, int fd, char *filename);
unsigned char *xdd_datapattern_get_datap_from_offset(worker_data_t *wdp);

// debug.c
Expand Down
17 changes: 12 additions & 5 deletions tests/functional/test_xdd_file_datapattern.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,35 @@ source "${SCRIPTPATH}"/../common.sh
# Perform pre-test
initialize_test
test_dir="${XDDTEST_LOCAL_MOUNT}/${TESTNAME}"
log_file="$(get_log_file)"

input_file=$test_dir/data1.dat
output_file=$test_dir/data2.dat
input_file="${test_dir}/data1.dat"
output_file="${test_dir}/data2.dat"

# Create datapattern input file using dd
dd if=/dev/urandom of="${input_file}" bs=4k count=2

# Write out file using xdd with -datapattern file
"${XDDTEST_XDD_EXE}" -op write -reqsize 8 -numreqs 1 -datapattern file "${input_file}" -targets 1 "${output_file}" -qd 1 -passes 1
# Verify the contents of the file match
if ! diff "${input_file}" "${output_file}"
if ! cmp "${input_file}" "${output_file}"
then
echo "Error when comparing files ${input_file} and ${output_file} with -datapattern file" > "${log_file}"
cmp "${input_file}" "${output_file}" >> "${log_file}"
rm "${input_file}" "${output_file}"
finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
fi

rm "${output_file}"
# Write out file using xdd with -datapattern wholefile
"${XDDTEST_XDD_EXE}" -op write -reqsize 8 -numreqs 1 -datapattern wholefile "${input_file}" -targets 1 "${output_file}" -qd 1 -passes 1
# In this caes we will use two threads to write out the data in serial ordering as the
# contents of the file will be distributed evenly between both threads.
"${XDDTEST_XDD_EXE}" -op write -reqsize 4 -numreqs 2 -datapattern wholefile "${input_file}" -targets 1 "${output_file}" -serialordering -qd 2 -passes 1
# Verify the contents of the file match
if ! diff "${input_file}" "${output_file}"
if ! cmp "${input_file}" "${output_file}"
then
echo "Error when comparing files ${input_file} and ${output_file} with -datapattern wholefile" > "${log_file}"
cmp "${input_file}" "${output_file}" >> "${log_file}"
rm "${input_file}" "${output_file}"
finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
fi
Expand Down

0 comments on commit 7274cc3

Please sign in to comment.