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 <[email protected]>
  • Loading branch information
@cadegore
cadegore committed Oct 4, 2024
1 parent 3790d66 commit 7d0fd05
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 67 deletions.
91 changes: 56 additions & 35 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 Down Expand Up @@ -216,7 +203,7 @@ 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()

/*----------------------------------------------------------------------------*/
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,9 +361,13 @@ 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
Expand Down Expand Up @@ -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 Down
133 changes: 101 additions & 32 deletions tests/functional/test_xdd_file_datapattern.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
#
# Acceptance test for XDD.
#
# Validate the funtionality of -datapattern file/wholefile option by creating a file, setting the datapattern
# from that file and then verifying the contents match
# Validate the funtionality of -datapattern file/wholefile option by creating a file,
# setting the datapattern from that file and then verifying the contents match.
# Additionally, ensure that when using the wholefile option, XDD does not crash (segfault)
# if the buffer file is smaller than the requested size (reqsize)
#
# Description - terminates XDD after a given amount of seconds have passed
# Description - Verifies XDD's -datapattern wholefile functionality.
#
# Get absolute path to script
SCRIPT=${BASH_SOURCE[0]}
SCRIPT="${BASH_SOURCE[0]}"
SCRIPTPATH=$(dirname "${SCRIPT}")

# Source the test configuration environment
source "${SCRIPTPATH}"/../test_config
source "${SCRIPTPATH}"/../common.sh
source "${SCRIPTPATH}/../test_config"
source "${SCRIPTPATH}/../common.sh"

# Perform pre-test
initialize_test
Expand All @@ -23,34 +25,101 @@ log_file="$(get_log_file)"
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
# Function to test XDD with file-based datapattern
# Args:
# $1: blocksize to create the input file
# $2: count (number of blocks to write)
# $3: reqsize (size of each I/O request)
# $4: numreqs (total number of I/O requests)
# $5: queuedepth (number of concurrent requests)
test_xdd_file_datapattern() {
local bs=$1
local count=$2
local reqsize=$3
local numreqs=$4
local qd=$5

# 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 ! 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
# 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 ! 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}"
# Create datapattern input file using dd
dd if=/dev/urandom of="${input_file}" bs="${bs}" count="${count}"

# Write out file using xdd with -datapattern file
"${XDDTEST_XDD_EXE}" -op write -reqsize "${reqsize}" -numreqs "${numreqs}" \
-datapattern wholefile "${input_file}" -targets 1 "${output_file}" \
-looseordering -qd "${qd}" -passes 1

# Check if the input file is smaller than the output file
input_size=$(stat -c%s "${input_file}")
output_size=$(stat -c%s "${output_file}")

# If the input_file is less than output_file
# Append the difference to input_file
# so when it compares both files, they have the same size
# If the input and output files differ in size, they are guaranteed not to match.
if ((input_size < output_size)); then
dd if="${input_file}" of="${input_file}" \
bs=1k \
count=$(((output_size - input_size) / 1024)) \
oflag=append conv=notrunc
elif ((input_size > output_size)); then
# Log an error if the input file is already larger in size
echo "Error: Input file (${input_size} bytes) is larger than output file. No appending needed." >"${log_file}"
finalize_test 1 "Failure: Input file is too large for comparison with the output file."
rm "${input_file}" "${output_file}"
return
fi

# Verify the contents of the file match
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 "${input_file}" "${output_file}"
finalize_test 1 "Issue with setting datapattern from file with -datapattern file. Contents don't match."
fi
rm "${input_file}" "${output_file}"
}

#
# Tests below test when blocksize is the same size as the request size
#
# Verfies that XDD handles -datapattern wholefile correctly with
# Using dd: bs=4k, count=2 to create a 8KB input file
# Using XDD: reqsize=8 blocks, numreqs=1, qd=1 (one thread)
test_xdd_file_datapattern 4k 2 8 1 1

# Verifies that XDD handles -datapattern wholefile correctly with
# Using dd: bs=4k, count=2 to create a 8KB input file
# Using XDD: reqsize=4 blocks, numreqs=2m, qd=2 (two threads)
test_xdd_file_datapattern 4k 2 4 2 2

#
# Tests below test when blocksize is smaller than request size
#
# Verifies that XDD handles cases where reqsize > blocksize
# This test confirms that XDD doesn't encounter issues when the request size
# is larger than the input file size. (#38)
# Using dd: bs=6k, count=1 to create a 6KB input file.
# Using XDD: reqsize=7 blocks, numreqs=1, qd=1
test_xdd_file_datapattern 6k 1 7 1 1

# Verifies correct behavior when reqsize is not a multiple of blocksize
# This test uses two threads with loose ordering to check proper data
# distribution across threads.
# Using dd: bs=5k, count=2 to create a 10KB input file.
# Using XDD: reqsize=7 blocks, numreqs=2, qd=2 (two threads).
test_xdd_file_datapattern 5k 2 7 2 2

# Verifies correct behavior when reqsize is a multiple of blocksize
# This test also uses two threads with loose ordering for overlapping I/O operations.
# Using dd: bs=6k, count=4 to create a 24KB input file.
# Using XDD: reqsize=8 blocks, numreqs=3, qd=2.
test_xdd_file_datapattern 6k 4 8 3 2

# Verifies that XDD handles very large reqsize without segmentation faults
# Specifically tests when reqsize is significantly larger than blocksize.
# Using dd: bs=1k, count=1 to create a 1KB input file.
# Using XDD: reqsize=256 blocks (1MB), numreqs=4, qd=3.
# This ensures XDD can handle writing large data even if the input file is small.
test_xdd_file_datapattern 1k 1 256 4 3

# Test passed
finalize_test 0

0 comments on commit 7d0fd05

Please sign in to comment.