Skip to content

Commit

Permalink
Fix segfault in XDD when buffer file is smaller than reqsize with -da…
Browse files Browse the repository at this point in the history 
…tapattern wholefile (#38)

This commit rewrites the logic for handling the -datapattern wholefile
option to ensure that even when the blocksize (bs) is smaller than the
request size (reqsize), the code runs without segmentation faults. The
changes include calculating the offset within the data pattern buffer,
allocating a buffer if not already allocated, and copying data in chunks
from the data pattern to the buffer until the entire transfer size is
copied. The offset is correctly calculated and reset after the first
iteration. In addition, a test case also added to verify that the code
correctly handles cases where the blocksize is smaller than the request
size. The test compares the input file and output file to ensure they
match and that the offset is correctly calculated with different numbers
of queue depth (qd).

Signed-off-by: Cade Gore <79346293+cadegore@users.noreply.github.com>
  • Loading branch information
@cadegore
cadegore committed Oct 4, 2024
1 parent 3790d66 commit 9814ff0
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 98 deletions.
153 changes: 87 additions & 66 deletions src/common/datapatterns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <assert.h>
#include "xint.h"
#include <sys/sysinfo.h>
#include <sys/param.h>

#define WHOLEFILE_MAX_SIZE_RAM 0.5 // Currently only allow for 50% of RAM

Expand Down Expand Up @@ -46,22 +47,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
tdp = wdp->wd_tdp;
dpp = tdp->td_dpp;
if (dpp->data_pattern_options & DP_WHOLEFILE_PATTERN) { // Using the whole contents of the file
if (tdp->td_dpp->data_pattern_length < ((size_t)tdp->td_xfer_size)) {
memset(wdp->wd_task.task_datap,'\0',tdp->td_xfer_size);
ucp = (unsigned char *)wdp->wd_task.task_datap;
if (dpp->data_pattern_options & DP_REPLICATE_PATTERN) { // Replicate the pattern throughout the buffer
remaining_length = tdp->td_xfer_size;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,dpp->data_pattern,pattern_length);
remaining_length -= pattern_length;
ucp += pattern_length;
}
}
} else if (tdp->td_dpp->data_pattern_length % tdp->td_xfer_size) {
/*
if (tdp->td_dpp->data_pattern_length % tdp->td_xfer_size) {
/*
* We will be assign data using each worker_data_t's task_byte_offset during target passes,
* so we just need to check if transfer size is evenly divisable by the
* the target file size here.
Expand All @@ -70,13 +57,13 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
* require periodic memcpy's to occur during target passes and effect measurements.
*/
fprintf(xgp->errout, "%s: WARNING file %s's size %lu is not evenly divisable by target transfer size %u this will effect performance measurements\n",
xgp->progname,
xgp->progname,
tdp->td_dpp->data_pattern_filename,
tdp->td_dpp->data_pattern_length,
tdp->td_xfer_size);
// Since the whole file size is greater than the td_xfer_size we will remove
// the replicate flag.
tdp->td_dpp->data_pattern_options =
tdp->td_dpp->data_pattern_options =
tdp->td_dpp->data_pattern_options & ~DP_REPLICATE_PATTERN;
}
} else if (dpp->data_pattern_options & DP_RANDOM_PATTERN) { // A nice random pattern
Expand All @@ -91,7 +78,7 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
} else if (dpp->data_pattern_options & DP_RANDOM_BY_TARGET_PATTERN) { // A nice random pattern, unique by target number
lp = (uint32_t *)wdp->wd_task.task_datap;
xgp->random_initialized = 0;
xgp->random_init_seed = (tdp->td_target_number+1);
xgp->random_init_seed = (tdp->td_target_number+1);
/* Set each four-byte field in the I/O buffer to a random integer */
for(i = 0; i < (int32_t)(tdp->td_xfer_size / sizeof(int32_t)); i++ ) {
*lp=xdd_random_int();
Expand All @@ -104,17 +91,17 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
if (dpp->data_pattern_options & DP_REPLICATE_PATTERN) { // Replicate the pattern throughout the buffer
ucp = (unsigned char *)wdp->wd_task.task_datap;
remaining_length = tdp->td_xfer_size;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;

memcpy(ucp,dpp->data_pattern,pattern_length);
remaining_length -= pattern_length;
ucp += pattern_length;
}
} else { // Just put the pattern at the beginning of the buffer once
if (dpp->data_pattern_length < (size_t)tdp->td_xfer_size)
} else { // Just put the pattern at the beginning of the buffer once
if (dpp->data_pattern_length < (size_t)tdp->td_xfer_size)
pattern_length = dpp->data_pattern_length;
else pattern_length = tdp->td_xfer_size;
memcpy(wdp->wd_task.task_datap,dpp->data_pattern,pattern_length);
Expand All @@ -126,8 +113,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
memset(wdp->wd_task.task_datap,0x00,tdp->td_xfer_size);
remaining_length = tdp->td_xfer_size;
ucp = (unsigned char *)wdp->wd_task.task_datap;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,lfpat,pattern_length);
Expand All @@ -141,8 +128,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
memset(wdp->wd_task.task_datap,0x00,tdp->td_xfer_size);
remaining_length = tdp->td_xfer_size;
ucp = (unsigned char *)wdp->wd_task.task_datap;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,ltpat,pattern_length);
Expand All @@ -156,8 +143,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
memset(wdp->wd_task.task_datap,0x00,tdp->td_xfer_size);
remaining_length = tdp->td_xfer_size;
ucp = (unsigned char *)wdp->wd_task.task_datap;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,cjtpat,pattern_length);
Expand All @@ -171,8 +158,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
memset(wdp->wd_task.task_datap,0x00,tdp->td_xfer_size);
remaining_length = tdp->td_xfer_size;
ucp = (unsigned char *)wdp->wd_task.task_datap;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,crpat,pattern_length);
Expand All @@ -186,8 +173,8 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
memset(wdp->wd_task.task_datap,0x00,tdp->td_xfer_size);
remaining_length = tdp->td_xfer_size;
ucp = (unsigned char *)wdp->wd_task.task_datap;
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
while (remaining_length) {
if (dpp->data_pattern_length < remaining_length)
pattern_length = dpp->data_pattern_length;
else pattern_length = remaining_length;
memcpy(ucp,cspat,pattern_length);
Expand Down Expand Up @@ -216,20 +203,20 @@ xdd_datapattern_buffer_init(worker_data_t *wdp) {
} else { // Otherwise set the entire buffer to the character in "dpp->data_pattern"
memset(wdp->wd_task.task_datap,*(dpp->data_pattern),tdp->td_xfer_size);
}

} // end of xdd_datapattern_buffer_init()

/*----------------------------------------------------------------------------*/
/* xdd_datapattern_fill() - This subroutine will fill the buffer with a
* specific pattern.
/* xdd_datapattern_fill() - This subroutine will fill the buffer with a
* specific pattern.
* This routine is called within the inner I/O loop for every I/O if the data
* pattern changes from IO to IO
*/
void
xdd_datapattern_fill(worker_data_t *wdp) {
target_data_t *tdp;
size_t j; // random variables
uint64_t *posp; // Position Pointer
size_t j; // random variables
uint64_t *posp; // Position Pointer
nclk_t start_time; // Used for calculating elapsed times of ops
nclk_t end_time; // Used for calculating elapsed times of ops

Expand All @@ -249,7 +236,7 @@ xdd_datapattern_fill(worker_data_t *wdp) {
nclk_now(&end_time);
// FIXME ???? wdp->wd_accumulated_pattern_fill_time = (end_time - start_time);
}
} // End of xdd_datapattern_fill()
} // End of xdd_datapattern_fill()

/*----------------------------------------------------------------------------*/
/* xdd_datapattern_wholefile_enough_ram() - This subroutine will verify that
Expand Down Expand Up @@ -280,7 +267,7 @@ xdd_datapattern_wholefile_enough_ram(target_data_t *tdp, const char *filename) {
filename);

return FALSE;
} // End of xdd_datapattern_wholefile_enough_ram()
} // End of xdd_datapattern_wholefile_enough_ram()

/*----------------------------------------------------------------------------*/
/* xdd_set_datapattern_from_filename() - This subroutine will set the targets
Expand All @@ -303,7 +290,7 @@ xdd_set_datapattern_from_filename(target_data_t *tdp, char *filename) {
close(fd);

return ret;
} // End of xdd_set_datapattern_from_filename()
} // End of xdd_set_datapattern_from_filename()

/*----------------------------------------------------------------------------*/
/* xdd_set_datapattern_from_file_descriptor() - This subroutine will set the
Expand Down Expand Up @@ -341,7 +328,27 @@ xdd_set_datapattern_from_file_descriptor(target_data_t *tdp, int fd, char *filen
error:
free(dp->data_pattern);
return 1;
} // End of xdd_set_datapattern_from_file_descriptor()
} // End of xdd_set_datapattern_from_file_descriptor()

/*----------------------------------------------------------------------------*/
/* xdd_datapattern_copy_data_to_buffer() - This function copies data from the
* target's data pattern buffer to the worker's buffer.
*/
static void
xdd_datapattern_copy_data_to_buffer(unsigned char *buff_ptr,
const unsigned char *data_pattern,
size_t buffer_size, off_t offset,
size_t remaining_size) {
size_t memcpy_size;

while (remaining_size > 0) {
memcpy_size = MIN(remaining_size, buffer_size - offset);
memcpy(buff_ptr, &(data_pattern[offset]), memcpy_size);
buff_ptr += memcpy_size;
remaining_size -= memcpy_size;
offset = 0;
}
} // End of xdd_datapattern_copy_data_to_buffer()

/*----------------------------------------------------------------------------*/
/* xdd_datapattern_get_datap_from_offset() - This subroutine will return the
Expand All @@ -354,27 +361,31 @@ xdd_datapattern_get_datap_from_offset(worker_data_t *wdp) {
unsigned char *buff = NULL;
off_t offset = 0;
size_t memcpy_size = 0;
size_t remaining_size = 0;
size_t buffer_size = 0;

tdp = wdp->wd_tdp;

// Get the size of the datapattern buffer
buffer_size = tdp->td_dpp->data_pattern_length;

if (tdp->td_dpp->data_pattern_options & DP_WHOLEFILE_PATTERN) {
/*
* In the event of a whole file data pattern we can wind up with offsets that
* are not correctly aligned with the target data buffer size. See Below:
*
*
* tdp->td_dpp->data_pattern - TDP
* wdp->wd_task.task_datap - WDP
*
*
* Aligned:
* TDP
* |-----------------------------------------------------------|
* | ------------ | ------------ | ------------ | ------------ |
* | ------------ | ------------ | ------------ | ------------ |
* WDP1 WDP2 WDP3 WDP4
*
* Unaligned:
* TDP
* |-----------------------------------------------------------|
* | ---------- | ---------- | ---------- | ---------- | ---------- |
* | ---------- | ---------- | ---------- | ---------- | ---------- |
* WDP1 WDP2 WDP3 WDP4 WDP5
*
* In the aligned case, we can just simply modulo the work_data_t's byte offset
Expand All @@ -388,22 +399,32 @@ xdd_datapattern_get_datap_from_offset(worker_data_t *wdp) {
*
* In the unaligned case we will allocate the IO buffer for the worker and copy
* the correct data to it.
*/
offset = wdp->wd_task.task_byte_offset % tdp->td_dpp->data_pattern_length;
if ((tdp->td_dpp->data_pattern_length - offset) < (size_t)tdp->td_xfer_size) {
// The target's xfer_size does not evenly divide into the whole file's
// size, so we have to allocate a buffer (if not already allocated) to
// hold the data an copy it over.
if (!wdp->wd_bufp_allocated) {
buff = xdd_init_io_buffers(wdp);
wdp->wd_task.task_datap = buff;
} else {
buff = wdp->wd_task.task_datap;
}
memcpy_size = tdp->td_dpp->data_pattern_length - offset;
memcpy(buff, &(tdp->td_dpp->data_pattern[offset]), memcpy_size);
memcpy(&buff[memcpy_size], tdp->td_dpp->data_pattern, (tdp->td_xfer_size - memcpy_size));
} else {
*/

// Calculate the offset within the datapattern buffer
offset = wdp->wd_task.task_byte_offset % buffer_size;
// Set the remaining size to the transfer size
remaining_size = tdp->td_xfer_size;

// If the buffer has already allocated, we can copy the data
// into buffer. We also handle the offset to ensure the data
// is being copied correctly
if (wdp->wd_bufp_allocated) {
buff = wdp->wd_task.task_datap;
unsigned char *buff_ptr = buff;
xdd_datapattern_copy_data_to_buffer(buff_ptr, tdp->td_dpp->data_pattern, buffer_size, offset, remaining_size);
}
// The target's xfer_size does not evenly divide into the whole file's
// size, so we have to allocate a buffer (if not already allocated) to
// hold the data an copy it over.
else if ((buffer_size - offset) < remaining_size) {
buff = xdd_init_io_buffers(wdp);
wdp->wd_task.task_datap = buff;
unsigned char *buff_ptr = buff;
xdd_datapattern_copy_data_to_buffer(buff_ptr, tdp->td_dpp->data_pattern, buffer_size, offset, remaining_size);
}
// Data can be directly used
else {
buff = &(tdp->td_dpp->data_pattern[offset]);
}
} else {
Expand All @@ -413,8 +434,8 @@ xdd_datapattern_get_datap_from_offset(worker_data_t *wdp) {
}

return buff;
} // End of xdd_datapattern_get_datap_from_offset()
} // End of xdd_datapattern_get_datap_from_offset()

/*
* Local variables:
* indent-tabs-mode: t
Expand Down
Loading

0 comments on commit 9814ff0

Please sign in to comment.