Re: [PATCH blktests] create a test for direct io offsets

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

> +static long pagesize;
> +static void *out_buf;
> +static void *in_buf;
> +static int test_fd;
> +

for the sake of completeness see if it makes sense, totally untested :-

diff --git a/src/dio-offsets.c b/src/dio-offsets.c
index 5961232..a8c4d3e 100644
--- a/src/dio-offsets.c
+++ b/src/dio-offsets.c
@@ -157,7 +157,7 @@ static size_t buf_size;
  static long pagesize;
  static void *out_buf;
  static void *in_buf;
-static int test_fd;
+static int test_fd = -1;
  static void init_kernel_version()
@@ -881,6 +881,20 @@ static void run_tests()
         test_unaligned_vectors();
  }
+/*
+ * Cleanup handler for atexit()
+ *
+ * Ensures resources are freed on all exit paths, including when err()/errx()
+ * cause early termination. This prevents file descriptor and memory leaks.
+ */
+static void cleanup(void)
+{
+       if (test_fd >= 0)
+               close(test_fd);
+       free(in_buf);
+       free(out_buf);
+}
+
  int main(int argc, char **argv)
  {
         if (argc < 2)
                 errx(EINVAL, "expect argments: file");
+
+       atexit(cleanup);
         init_args(argv);
         init_buffers();
         run_tests();
         close(test_fd);
         free(out_buf);

> +
> +static void read_queue_attrs(const char *path)
> +{
> +	char attr[PATH_MAX];
> +
> +	if (snprintf(attr, sizeof(attr), "%s/max_segments", path) < 0)
> +		err(errno, "max_segments");
> +	read_sysfs_attr(attr, &max_segments);
> +
> +	if (snprintf(attr, sizeof(attr), "%s/dma_alignment", path) < 0)
> +		err(errno, "dma_alignment");
> +	read_sysfs_attr(attr, &dma_alignment);
> +
> +	if (snprintf(attr, sizeof(attr), "%s/virt_boundary_mask", path) < 0)
> +		err(errno, "virt_boundary_mask");
> +	read_sysfs_attr(attr, &virt_boundary);
> +
> +	if (snprintf(attr, sizeof(attr), "%s/logical_block_size", path) < 0)
> +		err(errno, "logical_block_size");
> +	read_sysfs_attr(attr, &logical_block_size);
> +
> +	if (snprintf(attr, sizeof(attr), "%s/max_sectors_kb", path) < 0)
> +		err(errno, "max_sectors_kb");
> +	read_sysfs_attr(attr, &max_bytes);
> +
> +	max_bytes *= 1024;
> +	dma_alignment++;
> +	virt_boundary++;
> +
> +	/*
> +	printf("logical_block_size:%lu dma_alignment:%lu virt_boundary:%lu max_segments:%lu max_bytes:%lu\n",
> +		logical_block_size, dma_alignment, virt_boundary, max_segments, max_bytes);
> +	*/

debug comment ? either remove of guard with g_debug ?


> +}
> +
> +static void init_args(char **argv)
> +{
> +	char sys_path[PATH_MAX];
> +
> +        test_fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC | O_DIRECT);
> +        if (test_fd < 0)
> +		err(errno, "%s: failed to open %s", __func__, argv[1]);
> +
> +	init_kernel_version();
> +	find_block_device(argv[1], sys_path, sizeof(sys_path));
> +	read_queue_attrs(sys_path);
> +
> +	if (!power_of_2(virt_boundary) ||
> +	    !power_of_2(dma_alignment) ||
> +	    !power_of_2(logical_block_size))
> +		err(EINVAL, "%s: bad parameters", __func__);
> +
> +	if (virt_boundary > 1 && virt_boundary < logical_block_size)
> +		err(EINVAL, "%s: virt_boundary:%lu logical_block_size:%lu", __func__,
> +			virt_boundary, logical_block_size);
> +
> +	if (dma_alignment > logical_block_size)
> +		err(EINVAL, "%s: dma_alignment:%lu logical_block_size:%lu", __func__,
> +			dma_alignment, logical_block_size);
> +
> +	if (max_segments > 4096)
> +		max_segments = 4096;
> +	if (max_bytes > 16384 * 1024)
> +		max_bytes = 16384 * 1024;
> +	if (max_bytes & (logical_block_size - 1))
> +		max_bytes -= max_bytes & (logical_block_size - 1);
> +	pagesize = sysconf(_SC_PAGE_SIZE);
> +}
> +
> +static void init_buffers()
> +{
> +	unsigned long lb_mask = logical_block_size - 1;
> +	int fd, ret;
> +
> +	buf_size = max_bytes * max_segments / 2;
> +	if (buf_size < logical_block_size * max_segments)
> +		err(EINVAL, "%s: logical block size is too big", __func__);
> +
> +	if (buf_size < logical_block_size * 1024 * 4)
> +		buf_size = logical_block_size * 1024 * 4;
> +
> +	if (buf_size & lb_mask)
> +		buf_size = (buf_size + lb_mask) & ~(lb_mask);
> +
> +        ret = posix_memalign((void **)&in_buf, pagesize, buf_size);
> +        if (ret)
> +		err(EINVAL, "%s: failed to allocate in-buf", __func__);
> +
> +        ret = posix_memalign((void **)&out_buf, pagesize, buf_size);
> +        if (ret)
> +		err(EINVAL, "%s: failed to allocate out-buf", __func__);
> +
> +	fd = open("/dev/urandom", O_RDONLY);
> +	if (fd < 0)
> +		err(EINVAL, "%s: failed to open urandom", __func__);
> +
> +	ret = read(fd, out_buf, buf_size);
> +	if (ret < 0)
> +		err(EINVAL, "%s: failed to randomize output buffer", __func__);
> +
> +	close(fd);
> +}
> +
> +static void __compare(void *a, void *b, size_t size, const char *test)
> +{
> +	if (!memcmp(a, b, size))
> +		return;
> +	err(EIO, "%s: data corruption", test);
> +}
> +#define compare(a, b, size) __compare(a, b, size, __func__)
> +
> +/*
> + * Test using page aligned buffers, single source
> + *
> + * Total size is aligned to a logical block size and exceeds the max transfer
> + * size as well as the max segments. This should test the kernel's split bio
> + * construction and bio splitting for exceeding these limits.
> + */
> +static void test_full_size_aligned()
> +{
> +	int ret;
> +
> +	memset(in_buf, 0, buf_size);
> +	ret = pwrite(test_fd, out_buf, buf_size, 0);
> +	if (ret < 0)
> +		err(errno, "%s: failed to write buf", __func__);
> +
> +	ret = pread(test_fd, in_buf, buf_size, 0);
> +	if (ret < 0)
> +		err(errno, "%s: failed to read buf", __func__);
> +
> +	compare(out_buf, in_buf, buf_size);
> +}
> +

do we need to check for partial complete I/O ? something like this totally untested:-

iff --git a/src/dio-offsets.c b/src/dio-offsets.c
index a8c4d3e..b2f3e4a 100644
--- a/src/dio-offsets.c
+++ b/src/dio-offsets.c
@@ -280,6 +280,42 @@ static void init_args(char **argv)
  		err(errno, "%s: failed to open %s", __func__, argv[1]);
  }

+/*
+ * Verify that pread/pwrite transferred the expected number of bytes.
+ *
+ * POSIX allows short transfers even on success (ret >= 0). For direct I/O
+ * testing, we need exact transfers - any short transfer indicates a problem.
+ */
+static void check_io_result(ssize_t ret, size_t expected, const char *op,
+			    const char *func)
+{
+	if (ret < 0)
+		err(errno, "%s: %s failed", func, op);
+	if (ret != (ssize_t)expected)
+		errx(EIO, "%s: short %s: expected %zu bytes, got %zd bytes",
+		     func, op, expected, ret);
+}
+
+/*
+ * Calculate total size of iovec array and verify transfer completed fully.
+ *
+ * For preadv/pwritev, we need to sum all iov_len fields to get the expected
+ * total transfer size, then verify the actual return matches.
+ */
+static void check_iov_result(ssize_t ret, const struct iovec *iov, int iovcnt,
+			     const char *op, const char *func)
+{
+	size_t expected = 0;
+	int i;
+
+	for (i = 0; i < iovcnt; i++)
+		expected += iov[i].iov_len;
+
+	if (ret < 0)
+		err(errno, "%s: %s failed", func, op);
+	if (ret != (ssize_t)expected)
+		errx(EIO, "%s: short %s: expected %zu bytes, got %zd bytes",
+		     func, op, expected, ret);
+}

  static void init_buffers()
  {
@@ -468,10 +504,8 @@ static void test_full_size_aligned()
  	memset(in_buf, 0, buf_size);
  	ret = pwrite(test_fd, out_buf, buf_size, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_io_result(ret, buf_size, "write", __func__);

  	ret = pread(test_fd, in_buf, buf_size, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_io_result(ret, buf_size, "read", __func__);

  	compare(out_buf, in_buf, buf_size);
@@ -489,10 +523,8 @@ static void test_dma_aligned()
  	memset(in_buf, 0, buf_size);
  	ret = pwrite(test_fd, out_buf + dma_alignment, max_bytes, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_io_result(ret, max_bytes, "write", __func__);

  	ret = pread(test_fd, in_buf + dma_alignment, max_bytes, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_io_result(ret, max_bytes, "read", __func__);

  	compare(out_buf + dma_alignment, in_buf + dma_alignment, max_bytes);
@@ -515,8 +547,7 @@ static void test_page_aligned_vectors()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 4;
@@ -525,8 +556,7 @@ static void test_page_aligned_vectors()
  	}

  	ret = preadv(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_iov_result(ret, iov, vecs, "readv", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 4;
@@ -551,8 +581,7 @@ static void test_dma_aligned_vectors()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 8 + dma_alignment * (i + 1);
@@ -561,8 +590,7 @@ static void test_dma_aligned_vectors()
  	}

  	ret = preadv(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_iov_result(ret, iov, vecs, "readv", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 8 + dma_alignment * (i + 1);
@@ -599,11 +627,13 @@ static void test_unaligned_page_vectors()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0) {
-		if (!should_fail)
-			err(errno, "%s: unexpected failure", __func__);
+	if (ret < 0 && should_fail)
  		return;
-	}
+	check_iov_result(ret, iov, vecs, "writev", __func__);

  	if (should_fail)
  		errx(EINVAL, "%s: expected failure, but succeeded", __func__);
@@ -615,8 +639,7 @@ static void test_unaligned_page_vectors()
  	}

  	ret = preadv(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_iov_result(ret, iov, vecs, "readv", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 8;
@@ -653,11 +676,13 @@ static void test_unaligned_vectors()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0) {
-		if (!should_fail)
-			err(errno, "%s: unexpected failure", __func__);
+	if (ret < 0 && should_fail)
  		return;
-	}
+	check_iov_result(ret, iov, vecs, "writev", __func__);

  	if (should_fail)
  		errx(EINVAL, "%s: expected failure, but succeeded", __func__);
@@ -669,8 +688,7 @@ static void test_unaligned_vectors()
  	}

  	ret = preadv(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_iov_result(ret, iov, vecs, "readv", __func__);

  	for (i = 0; i < vecs; i++) {
  		offset = logical_block_size * i * 4 + logical_block_size / 2;
@@ -693,8 +711,7 @@ static void test_virt_boundary_vectors()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);

  	for (i = 0; i < vecs; i++)
  		iov[i].iov_base = in_buf + (virt_boundary + 1) * i;
@@ -719,8 +736,7 @@ static void test_max_segments()
  	}

  	ret = pwritev(test_fd, iov, max_segments, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, max_segments, "writev", __func__);
  }

  /*
@@ -737,10 +753,8 @@ static void test_single_block(unsigned long offset, unsigned long size)
  	memset(in_buf, 0, buf_size);

  	ret = pwrite(test_fd, out_buf + offset, size, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_io_result(ret, size, "write", __func__);

  	ret = pread(test_fd, in_buf + offset, size, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to read buf", __func__);
+	check_io_result(ret, size, "read", __func__);

  	compare(in_buf + offset, out_buf + offset, size);
@@ -776,11 +790,13 @@ static void test_unaligned_single_vectors()
  		}

  		ret = pwritev(test_fd, iov, vecs, 0);
-		if (ret < 0) {
-			if (!should_fail)
-				err(errno, "%s: unexpected failure", __func__);
+		if (ret < 0 && should_fail)
  			continue;
-		}
+		check_iov_result(ret, iov, vecs, "writev", __func__);

  		if (should_fail)
  			errx(EINVAL, "%s: expected failure, but succeeded", __func__);
@@ -793,8 +803,7 @@ static void test_unaligned_single_vectors()
  		}

  		ret = preadv(test_fd, iov, vecs, 0);
-		if (ret < 0)
-			err(errno, "%s: failed to read buf", __func__);
+		check_iov_result(ret, iov, vecs, "readv", __func__);

  		for (j = 0; j < vecs; j++) {
  			offset = i + j;
@@ -833,11 +842,13 @@ static void test_aligned_mixed_vectors()
  		}

  		ret = pwritev(test_fd, iov, vecs, 0);
-		if (ret < 0) {
-			if (!should_fail)
-				err(errno, "%s: unexpected failure", __func__);
+		if (ret < 0 && should_fail)
  			continue;
-		}
+		check_iov_result(ret, iov, vecs, "writev", __func__);

  		if (should_fail)
  			errx(EINVAL, "%s: expected failure, but succeeded", __func__);
@@ -850,8 +855,7 @@ static void test_aligned_mixed_vectors()
  		}

  		ret = preadv(test_fd, iov, vecs, 0);
-		if (ret < 0)
-			err(errno, "%s: failed to read buf", __func__);
+		check_iov_result(ret, iov, vecs, "readv", __func__);

  		for (j = 0; j < vecs; j++) {
  			offset = (i + j) * pagesize;
@@ -875,8 +879,7 @@ static void test_max_bytes()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);
  }

  static void test_max_bytes_plus_one()
@@ -893,8 +896,7 @@ static void test_max_bytes_plus_one()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);
  }

  static void test_max_bytes_minus_one()
@@ -909,8 +911,7 @@ static void test_max_bytes_minus_one()
  	}

  	ret = pwritev(test_fd, iov, vecs, 0);
-	if (ret < 0)
-		err(errno, "%s: failed to write buf", __func__);
+	check_iov_result(ret, iov, vecs, "writev", __func__);
  }

  static void run_tests()

-ck