diotest: use user specified bufsize correctly

PATCH New Eryu Guan 2 participants 7 messages Mar 2, 2017 – Mar 15, 2017

Share & tools

Eryu Guan

author

2017-03-02 13:33 UTC #0 lore raw

PATCH 1/3 diotest: use user specified bufsize correctly

In diotestN, option b is used to specify the buffer size used in the tests, but some tests are overriding this user specified size with the default value (BUFSIZE).

So always use 'bufsize' once it's initialized by user specified value.

Signed-off-by Eryu Guan <[email protected]>

---
 testcases/kernel/io/direct_io/diotest2.c |  9 +++++----
 testcases/kernel/io/direct_io/diotest3.c |  1 -
 testcases/kernel/io/direct_io/diotest4.c | 10 +++++-----
 testcases/kernel/io/direct_io/diotest5.c |  2 +-
 testcases/kernel/io/direct_io/diotest6.c |  6 +++---
 5 files changed, 14 insertions(+), 14 deletions(-)

testcases/kernel/io/direct_io/diotest2.c +5 -4

index 15ae932..c32dbeb 100644
@@ -69,6 +69,10 @@ int TST_TOTAL = 3;		/* Total number of test conditions */
 #define WRITE_DIRECT 2
 #define RDWR_DIRECT 3
 
+int fd1 = -1;
+char filename[LEN];
+int bufsize = BUFSIZE;
+
 /*
  * runtest: write the data to the file. Read the data from the file and compare.
  *	For each iteration, write data starting at offse+iter*bufsize

@@ -78,7 +82,7 @@ int runtest(int fd_r, int fd_w, int iter, off64_t offset, int action)
 {
 	char *buf1;
 	char *buf2;
-	int i, bufsize = BUFSIZE;
+	int i;
 
 	/* Allocate for buffers */
 	if ((buf1 = valloc(bufsize)) == 0) {

@@ -129,15 +133,12 @@ void prg_usage()
 	exit(1);
 }
 
-int fd1 = -1;
-char filename[LEN];
 static void setup(void);
 static void cleanup(void);
 
 int main(int argc, char *argv[])
 {
 	int iter = 100;		/* Iterations. Default 100 */
-	int bufsize = BUFSIZE;	/* Buffer size. Default 4k */
 	off64_t offset = 0;	/* Offset. Default 0 */
 	int i, action, fd_r, fd_w;
 	int fail_count = 0, total = 0, failed = 0;

testcases/kernel/io/direct_io/diotest3.c +0 -1

index 5e204e7..efbc79d 100644
@@ -96,7 +96,6 @@ int runtest(int fd_r, int fd_w, int childnum, int action)
 	char *buf1;
 	char *buf2;
 	off_t seekoff;
-	int bufsize = BUFSIZE;
 	int i;
 
 	/* Allocate for buffers */

testcases/kernel/io/direct_io/diotest4.c +5 -5

index e9a7ac4..d8a3a42 100644
@@ -224,19 +224,19 @@ int main(int argc, char *argv[])
 		tst_brkm(TBROK, cleanup, "open failed for %s: %s",
 			 filename, strerror(errno));
 	}
-	if ((buf0 = valloc(BUFSIZE)) == NULL) {
+	if ((buf0 = valloc(bufsize)) == NULL) {
 		tst_brkm(TBROK, cleanup, "valloc() buf0 failed: %s",
 			 strerror(errno));
 	}
 	for (i = 1; i < fblocks; i++) {
-		fillbuf(buf0, BUFSIZE, (char)i);
-		if (write(fd, buf0, BUFSIZE) < 0) {
+		fillbuf(buf0, bufsize, (char)i);
+		if (write(fd, buf0, bufsize) < 0) {
 			tst_brkm(TBROK, cleanup, "write failed for %s: %s",
 				 filename, strerror(errno));
 		}
 	}
 	close(fd);
-	if ((buf2 = valloc(BUFSIZE)) == NULL) {
+	if ((buf2 = valloc(bufsize)) == NULL) {
 		tst_brkm(TBROK, cleanup, "valloc() buf2 failed: %s",
 			 strerror(errno));
 	}

@@ -284,7 +284,7 @@ int main(int argc, char *argv[])
 	total++;
 
 	/* Test-4: Read beyond the file size */
-	offset = BUFSIZE * (fblocks + 10);
+	offset = bufsize * (fblocks + 10);
 	count = bufsize;
 	if (lseek(fd, offset, SEEK_SET) < 0) {
 		tst_resm(TFAIL, "lseek failed: %s", strerror(errno));

testcases/kernel/io/direct_io/diotest5.c +1 -1

index 2c3774a..dc4ecd1 100644
@@ -84,7 +84,7 @@ static int fd1 = -1;
 */
 int runtest(int fd_r, int fd_w, int iter, off64_t offset, int action)
 {
-	int i, bufsize = BUFSIZE;
+	int i;
 	struct iovec *iov1, *iov2, *iovp;
 
 	/* Allocate for buffers and data pointers */

testcases/kernel/io/direct_io/diotest6.c +3 -3

index 42e1d34..5594e04 100644
@@ -85,11 +85,11 @@ static void prg_usage(void)
 int runtest(int fd_r, int fd_w, int childnum, int action)
 {
 	off64_t seekoff;
-	int i, bufsize = BUFSIZE;
+	int i;
 	char *buf1, *buf2;
 
-	buf1 = valloc(BUFSIZE);
-	buf2 = valloc(BUFSIZE);
+	buf1 = valloc(bufsize);
+	buf2 = valloc(bufsize);
 
 	if (!buf1 || !buf2) {
 		tst_resm(TBROK | TERRNO, "valloc() failed");

Signature · 1 line

2.9.3

Eryu Guan

author

2017-03-02 13:33 UTC #1 lore raw

PATCH 2/3 diotest6: test readv/writev not read/write

According to the comments in diotest6, it's supposed to test readv/writev syscalls not read/write, which are tested by diotest3.

Update the case to test readv/writev.

Signed-off-by Eryu Guan <[email protected]>

---
 testcases/kernel/io/direct_io/diotest6.c | 77 +++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 25 deletions(-)

testcases/kernel/io/direct_io/diotest6.c +52 ~~-25~~

index 5594e04..92dd232 100644
@@ -85,60 +85,87 @@ static void prg_usage(void)
 int runtest(int fd_r, int fd_w, int childnum, int action)
 {
 	off64_t seekoff;
-	int i;
-	char *buf1, *buf2;
+	int i, ret = -1;
+	struct iovec *iov_r, *iov_w, *iovp;
 
-	buf1 = valloc(bufsize);
-	buf2 = valloc(bufsize);
-
-	if (!buf1 || !buf2) {
-		tst_resm(TBROK | TERRNO, "valloc() failed");
-		free(buf1);
-		free(buf2);
+	/* allocate read/write io vectors */
+	iov_r = calloc(nvector, sizeof(*iov_r));
+	if (!iov_r) {
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_r array");
+		return -1;
+	}
+	iov_w = calloc(nvector, sizeof(*iov_w));
+	if (!iov_w) {
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_w array");
+		free(iov_r);
 		return -1;
 	}
 
-	/* Allocate for buffers and data pointers */
-	seekoff = offset + bufsize * childnum;
+	/* allocate buffers and setup read/write io vectors */
+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_r[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}
+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_w[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}
 
 	/* seek, write, read and verify */
+	seekoff = offset + bufsize * childnum * nvector;
 	for (i = 0; i < iter; i++) {
-		fillbuf(buf1, bufsize, childnum+i);
+		vfillbuf(iov_w, nvector, childnum+i);
 
 		if (lseek(fd_w, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before write failed: %s",
 				 strerror(errno));
-			return (-1);
+			goto free_out;
 		}
-		if (write(fd_w, buf1, bufsize) < bufsize) {
-			tst_resm(TFAIL, "write failed: %s", strerror(errno));
-			return (-1);
+		if (writev(fd_w, iov_w, nvector) < 0) {
+			tst_resm(TFAIL, "writev failed: %s", strerror(errno));
+			goto free_out;
 		}
 		if (action == READ_DIRECT) {
 			/* Make sure data is on to disk before read */
 			if (fsync(fd_w) < 0) {
 				tst_resm(TFAIL, "fsync failed: %s",
 					 strerror(errno));
-				return (-1);
+				goto free_out;
 			}
 		}
 		if (lseek(fd_r, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before read failed: %s",
 				 strerror(errno));
-			return (-1);
+			goto free_out;
 		}
-		int ret;
-		if ((ret = read(fd_r, buf2, bufsize)) < bufsize) {
-			tst_resm(TFAIL, "read failed: %s", strerror(errno));
-			return (-1);
+		if (readv(fd_r, iov_r, nvector) < 0) {
+			tst_resm(TFAIL, "readv failed: %s", strerror(errno));
+			goto free_out;
 		}
-		if (bufcmp(buf1, buf2, bufsize) != 0) {
+		if (vbufcmp(iov_w, iov_r, nvector) != 0) {
 			tst_resm(TFAIL, "comparsion failed. Child=%d offset=%d",
 				 childnum, (int)seekoff);
-			return (-1);
+			goto free_out;
 		}
 	}
-	return 0;
+	ret = 0;
+
+free_out:
+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	free(iov_r);
+	free(iov_w);
+	return ret;
 }
 
 /*

Signature · 1 line

2.9.3

Cyril Hrubis

2017-03-15 10:31 UTC #2 lore raw

PATCH 2/3 diotest6: test readv/writev not read/write

Hi!

Quoted 6 lines

testcases/kernel/io/direct_io/diotest6.c +2 -4

-	if (!buf1 || !buf2) {
-		tst_resm(TBROK | TERRNO, "valloc() failed");
-		free(buf1);
-		free(buf2);
+	/* allocate read/write io vectors */
+	iov_r = calloc(nvector, sizeof(*iov_r));

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 102

Quoted 10 lines

testcases/kernel/io/direct_io/diotest6.c +8 -0

+	if (!iov_r) {
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_r array");
+		return -1;
+	}
+	iov_w = calloc(nvector, sizeof(*iov_w));
+	if (!iov_w) {
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_w array");
+		free(iov_r);
 		return -1;
 	}

Doing free(NULL) is no-op. So we can simplify this code by allocating both arrays at once and then checking if !iow_r || !iow_w just like we did with the normal buffers.

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 112

Quoted 11 lines

testcases/kernel/io/direct_io/diotest6.c +9 -2

-	/* Allocate for buffers and data pointers */
-	seekoff = offset + bufsize * childnum;
+	/* allocate buffers and setup read/write io vectors */
+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_r[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}

Uh, why do we increment both iovp and i here?

When we can simply do:

for (i = 0; i < nvector; i++) {
	iovp[i].iov_base = valloc(bufsize);
	...
}

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 120

Quoted 8 lines

testcases/kernel/io/direct_io/diotest6.c +8 -0

+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_w[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}

Here as well.

Quoted 9 lines

testcases/kernel/io/direct_io/diotest6.c +2 -1

 	/* seek, write, read and verify */
+	seekoff = offset + bufsize * childnum * nvector;
 	for (i = 0; i < iter; i++) {
-		fillbuf(buf1, bufsize, childnum+i);
+		vfillbuf(iov_w, nvector, childnum+i);
 
 		if (lseek(fd_w, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before write failed: %s",
 				 strerror(errno));

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 135

Quoted 10 lines

testcases/kernel/io/direct_io/diotest6.c +4 -4

-			return (-1);
+			goto free_out;
 		}
-		if (write(fd_w, buf1, bufsize) < bufsize) {
-			tst_resm(TFAIL, "write failed: %s", strerror(errno));
-			return (-1);
+		if (writev(fd_w, iov_w, nvector) < 0) {
+			tst_resm(TFAIL, "writev failed: %s", strerror(errno));
+			goto free_out;
 		}

We have to check here that the whole vector was written as well.

Quoted 13 lines

testcases/kernel/io/direct_io/diotest6.c +1 -2

 		if (action == READ_DIRECT) {
 			/* Make sure data is on to disk before read */
 			if (fsync(fd_w) < 0) {
 				tst_resm(TFAIL, "fsync failed: %s",
 					 strerror(errno));
-				return (-1);
+				goto free_out;
 			}
 		}
 		if (lseek(fd_r, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before read failed: %s",
 				 strerror(errno));
-			return (-1);

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 152

Quoted 10 lines

testcases/kernel/io/direct_io/diotest6.c +4 -4

+			goto free_out;
 		}
-		int ret;
-		if ((ret = read(fd_r, buf2, bufsize)) < bufsize) {
-			tst_resm(TFAIL, "read failed: %s", strerror(errno));
-			return (-1);
+		if (readv(fd_r, iov_r, nvector) < 0) {
+			tst_resm(TFAIL, "readv failed: %s", strerror(errno));
+			goto free_out;
 		}

Here as well.

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 161

Quoted 12 lines

testcases/kernel/io/direct_io/diotest6.c +5 -3

-		if (bufcmp(buf1, buf2, bufsize) != 0) {
+		if (vbufcmp(iov_w, iov_r, nvector) != 0) {
 			tst_resm(TFAIL, "comparsion failed. Child=%d offset=%d",
 				 childnum, (int)seekoff);
-			return (-1);
+			goto free_out;
 		}
 	}
-	return 0;
+	ret = 0;
+
+free_out:

I would call this label err: or just out: but that is minor.

Quoted 16 lines

testcases/kernel/io/direct_io/diotest6.c +7 -0

+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	free(iov_r);
+	free(iov_w);
+	return ret;
 }
 
 /*

Signature · 5 lines

2.9.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Signature · 2 lines

Cyril Hrubis
[email protected]

Eryu Guan

author

2017-03-15 10:48 UTC #3 lore raw

PATCH 2/3 diotest6: test readv/writev not read/write

Quoted 13 lines · Cyril Hrubis

Hi!

Quoted 12 lines

testcases/kernel/io/direct_io/diotest6.c +8 -4

-	if (!buf1 || !buf2) {
-		tst_resm(TBROK | TERRNO, "valloc() failed");
-		free(buf1);
-		free(buf2);
+	/* allocate read/write io vectors */
+	iov_r = calloc(nvector, sizeof(*iov_r));
+	if (!iov_r) {
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_r array");
+		return -1;
+	}
+	iov_w = calloc(nvector, sizeof(*iov_w));
+	if (!iov_w) {

Quoted 8 lines

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 102
Quoted 4 lines
testcases/kernel/io/direct_io/diotest6.c +2 -0
+		tst_resm(TBROK | TERRNO, "calloc failed for iov_w array");
+		free(iov_r);
 		return -1;
 	}
Doing free(NULL) is no-op. So we can simplify this code by allocating both arrays at once and then checking if !iow_r || !iow_w just like we did with the normal buffers.

OK.

Quoted 15 lines

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 112

Quoted 12 lines

testcases/kernel/io/direct_io/diotest6.c +9 -2

-	/* Allocate for buffers and data pointers */
-	seekoff = offset + bufsize * childnum;
+	/* allocate buffers and setup read/write io vectors */
+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_r[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}

Uh, why do we increment both iovp and i here?

Quoted 6 lines

When we can simply do:

for (i = 0; i < nvector; i++) {
	iovp[i].iov_base = valloc(bufsize);
	...
}

You're right, no need to increase both iovp and i. I "copied" this pattern from diotest5.c, and "i" isn't even used in similar loop there.

Quoted 18 lines

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 120

Quoted 9 lines

testcases/kernel/io/direct_io/diotest6.c +8 -0

+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++) {
+		iovp->iov_base = valloc(bufsize);
+		if (!iovp->iov_base) {
+			tst_resm(TBROK | TERRNO, "valloc error iov_w[%d]", i);
+			goto free_out;
+		}
+		iovp->iov_len = bufsize;
+	}

Here as well.

Quoted 6 lines

testcases/kernel/io/direct_io/diotest6.c +2 -1

 	/* seek, write, read and verify */
+	seekoff = offset + bufsize * childnum * nvector;
 	for (i = 0; i < iter; i++) {
-		fillbuf(buf1, bufsize, childnum+i);
+		vfillbuf(iov_w, nvector, childnum+i);

Quoted 15 lines

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 135

Quoted 13 lines

testcases/kernel/io/direct_io/diotest6.c +4 -4

 		if (lseek(fd_w, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before write failed: %s",
 				 strerror(errno));
-			return (-1);
+			goto free_out;
 		}
-		if (write(fd_w, buf1, bufsize) < bufsize) {
-			tst_resm(TFAIL, "write failed: %s", strerror(errno));
-			return (-1);
+		if (writev(fd_w, iov_w, nvector) < 0) {
+			tst_resm(TFAIL, "writev failed: %s", strerror(errno));
+			goto free_out;
 		}

We have to check here that the whole vector was written as well.

Sure, I noticed that there're other diotests break out if write[v] didn't write bufsize data.

Quoted 27 lines

Quoted 14 lines

testcases/kernel/io/direct_io/diotest6.c +1 -2

 		if (action == READ_DIRECT) {
 			/* Make sure data is on to disk before read */
 			if (fsync(fd_w) < 0) {
 				tst_resm(TFAIL, "fsync failed: %s",
 					 strerror(errno));
-				return (-1);
+				goto free_out;
 			}
 		}
 		if (lseek(fd_r, seekoff, SEEK_SET) < 0) {
 			tst_resm(TFAIL, "lseek before read failed: %s",
 				 strerror(errno));
-			return (-1);

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 152

Quoted 10 lines

testcases/kernel/io/direct_io/diotest6.c +4 -4

+			goto free_out;
 		}
-		int ret;
-		if ((ret = read(fd_r, buf2, bufsize)) < bufsize) {
-			tst_resm(TFAIL, "read failed: %s", strerror(errno));
-			return (-1);
+		if (readv(fd_r, iov_r, nvector) < 0) {
+			tst_resm(TFAIL, "readv failed: %s", strerror(errno));
+			goto free_out;
 		}

Here as well.

Quoted 14 lines

testcases/kernel/io/direct_io/diotest6.c static void prg_usage(void) · line 161
Quoted 12 lines
testcases/kernel/io/direct_io/diotest6.c +5 -3
-		if (bufcmp(buf1, buf2, bufsize) != 0) {
+		if (vbufcmp(iov_w, iov_r, nvector) != 0) {
 			tst_resm(TFAIL, "comparsion failed. Child=%d offset=%d",
 				 childnum, (int)seekoff);
-			return (-1);
+			goto free_out;
 		}
 	}
-	return 0;
+	ret = 0;
+
+free_out:
I would call this label err: or just out: but that is minor.

OK, I'll rename it to "err".

Thanks for the review!

Eryu

Quoted 21 lines

Quoted 17 lines

testcases/kernel/io/direct_io/diotest6.c +7 -0

+	for (i = 0, iovp = iov_r; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	for (i = 0, iovp = iov_w; i < nvector; iovp++, i++)
+		free(iovp->iov_base);
+	free(iov_r);
+	free(iov_w);
+	return ret;
 }
 
 /*

Signature · 5 lines

2.9.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Signature · 2 lines

Cyril Hrubis
[email protected]

Eryu Guan

author

2017-03-02 13:33 UTC #4 lore raw

PATCH 3/3 runtest/dio: enable dio29/30

In reply to Eryu Guan

Not sure why dio29 and dio30 are commented out, I found a bug in 4.10 kernel by running them, see this patch

https://patchwork.kernel.org/patch/9598823/

So let's enable them by default.

Signed-off-by Eryu Guan <[email protected]>

---
 runtest/dio | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

runtest/dio +2 -2

index 97820e9..a26e001 100644
@@ -40,8 +40,8 @@ dio27 diotest6 -b 8192 -o 1024000 -i 1000 -v 100
 dio28 diotest6 -b 8192 -o 1024000 -i 1000 -v 200
 
 ### Run the tests with more children
-#dio29 diotest3 -b 65536 -n 100 -i 1000 -o 1024000
-#dio30 diotest6 -b 65536 -n 100 -i 1000 -o 1024000
+dio29 diotest3 -b 65536 -n 100 -i 1000 -o 1024000
+dio30 diotest6 -b 65536 -n 100 -i 1000 -o 1024000
 #
 # RAW DEVICE TEST SECTION
 #   DEV1 and DEV2 should be exported prior to execution or

Signature · 1 line

2.9.3

Cyril Hrubis

2017-03-15 10:34 UTC #5 lore raw

PATCH 3/3 runtest/dio: enable dio29/30

Hi!
Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]

Cyril Hrubis

2017-03-15 10:13 UTC #6 lore raw

PATCH 1/3 diotest: use user specified bufsize correctly

In reply to Eryu Guan

Hi!
Pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]