All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
@ 2021-12-13 16:04 Andrea Cervesato via ltp
  2021-12-14 15:04 ` Petr Vorel
  2021-12-15 13:11 ` Cyril Hrubis
  0 siblings, 2 replies; 7+ messages in thread
From: Andrea Cervesato via ltp @ 2021-12-13 16:04 UTC (permalink / raw)
  To: ltp

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/io/ltp-aiodio/dio_append.c | 174 +++++++-------------
 1 file changed, 62 insertions(+), 112 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c
index b1b4dc039..3b1ddfc40 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_append.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_append.c
@@ -1,143 +1,93 @@
-
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2004 Daniel McNeil <daniel@osdl.org>
- *               2004 Open Source Development Lab
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
- * Module: .c
+ *				 2004 Open Source Development Lab
+ *				 2004  Marty Ridgeway <mridge@us.ibm.com>
+ * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Change History:
- *
- * 2/2004  Marty Ridgeway (mridge@us.ibm.com) Changes to adapt to LTP
+/*\
+ * [Description]
  *
- */
-/*
  * dio_append - append zeroed data to a file using O_DIRECT while
  *	a 2nd process is doing buffered reads and check if the buffer
  *	reads always see zero.
  */
 #define _GNU_SOURCE
 
-#include <stdlib.h>
-#include <sys/types.h>
-#include <signal.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <memory.h>
-#include <limits.h>
-
-#include "test.h"
-#define NUM_CHILDREN 8
+#include "tst_test.h"
+#include "common.h"
 
-#include "common_checkzero.h"
+static int *run_child;
 
-int read_eof(char *filename)
-{
-	int fd;
-	int i;
-	int r;
-	char buf[4096];
+static char *str_numchildren;
+static char *str_writesize;
+static char *str_appends;
 
-	while ((fd = open(filename, O_RDONLY)) < 0) {
-		sleep(1);	/* wait for file to be created */
-	}
-
-	for (i = 0; i < 1000000; i++) {
-		off_t offset;
-		char *bufoff;
-
-		offset = lseek(fd, SEEK_END, 0);
-		r = read(fd, buf, 4096);
-		if (r > 0) {
-			if ((bufoff = check_zero(buf, r))) {
-				fprintf(stderr, "non-zero read at offset %p\n",
-					offset + bufoff);
-				exit(1);
-			}
-		}
-	}
-	return 0;
-}
+static int numchildren;
+static long long writesize;
+static int appends;
 
-void dio_append(char *filename)
+static void setup(void)
 {
-	int fd;
-	void *bufptr = NULL;
-	int i;
-	int w;
+	numchildren = 16;
+	writesize = 64 * 1024;
+	appends = 1000;
 
-	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
+	if (tst_parse_int(str_numchildren, &numchildren, 1, INT_MAX))
+		tst_brk(TBROK, "Invalid number of children '%s'", str_numchildren);
 
-	if (fd < 0) {
-		perror("cannot create file");
-		return;
-	}
+	if (tst_parse_filesize(str_writesize, &writesize, 1, LLONG_MAX))
+		tst_brk(TBROK, "Invalid write file size '%s'", str_writesize);
 
-	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
-	if (TEST_RETURN) {
-		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
-		close(fd);
-		return;
-	}
+	if (tst_parse_int(str_appends, &appends, 1, INT_MAX))
+		tst_brk(TBROK, "Invalid number of appends '%s'", str_appends);
 
-	memset(bufptr, 0, 64 * 1024);
-	for (i = 0; i < 1000; i++) {
-		if ((w = write(fd, bufptr, 64 * 1024)) != 64 * 1024) {
-			fprintf(stderr, "write %d returned %d\n", i, w);
-		}
-	}
+	run_child = SAFE_MMAP(NULL, sizeof(int), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 }
 
-int main(void)
+static void cleanup(void)
 {
-	char filename[PATH_MAX];
-	int pid[NUM_CHILDREN];
-	int num_children = 1;
+	SAFE_MUNMAP(run_child, sizeof(int));
+}
+
+static void run(void)
+{
+	char *filename = "dio_append";
+	int status;
 	int i;
 
-	snprintf(filename, sizeof(filename), "%s/aiodio/file",
-		 getenv("TMP") ? getenv("TMP") : "/tmp");
-
-	printf("Begin dio_append test...\n");
-
-	for (i = 0; i < num_children; i++) {
-		if ((pid[i] = fork()) == 0) {
-			/* child */
-			return read_eof(filename);
-		} else if (pid[i] < 0) {
-			/* error */
-			perror("fork error");
-			break;
-		} else {
-			/* Parent */
-			continue;
+	*run_child = 1;
+
+	for (i = 0; i < numchildren; i++) {
+		if (!SAFE_FORK()) {
+			io_read_eof(filename, run_child);
+			return;
 		}
 	}
 
-	/*
-	 * Parent appends to end of file using direct i/o
-	 */
+	tst_res(TINFO, "Parent append to file");
 
-	dio_append(filename);
+	io_append(filename, 0, O_DIRECT | O_WRONLY | O_CREAT, writesize, appends);
 
-	for (i = 0; i < num_children; i++) {
-		kill(pid[i], SIGTERM);
-	}
-	return 0;
+	if (SAFE_WAITPID(-1, &status, WNOHANG))
+		tst_res(TFAIL, "Non zero bytes read");
+	else
+		tst_res(TPASS, "All bytes read were zeroed");
+
+	*run_child = 0;
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_tmpdir = 1,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{"n:", &str_numchildren, "Number of threads (default 1000)"},
+		{"w:", &str_writesize, "Write size for each append (default 64K)"},
+		{"c:", &str_appends, "Number of appends (default 1000)"},
+		{}
+	},
+};
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-13 16:04 [LTP] [PATCH v4] Refactoring dio_append.c using LTP API Andrea Cervesato via ltp
@ 2021-12-14 15:04 ` Petr Vorel
  2021-12-14 15:13   ` Cyril Hrubis
  2021-12-15 13:11 ` Cyril Hrubis
  1 sibling, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2021-12-14 15:04 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi Andrea,

Info for myself: this patch requires "Add io_read_eof in common.h utilities" [1]

I wonder what I'm missing:
./dio_append
tst_test.c:1409: TINFO: Timeout per run is 0h 05m 00s
dio_append.c:69: TINFO: Parent append to file
common.h:45: TBROK: open(dio_append,16449,0666) failed: EINVAL (22)
common.h:98: TINFO: child 10949 reading file
common.h:98: TINFO: child 10953 reading file
common.h:98: TINFO: child 10940 reading file
common.h:98: TINFO: child 10941 reading file
common.h:98: TINFO: child 10944 reading file
common.h:98: TINFO: child 10946 reading file
common.h:98: TINFO: child 10951 reading file
common.h:98: TINFO: child 10942 reading file
common.h:98: TINFO: child 10948 reading file
common.h:98: TINFO: child 10939 reading file
common.h:98: TINFO: child 10943 reading file
common.h:98: TINFO: child 10950 reading file
common.h:98: TINFO: child 10947 reading file
common.h:98: TINFO: child 10945 reading file
common.h:98: TINFO: child 10952 reading file
common.h:98: TINFO: child 10954 reading file

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0

> +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c
...
> +static void run(void)
> +{
> +	char *filename = "dio_append";
nit: having the same name as the binary filename is a bit confusing.
(sure will work even run as ./dio_append due use of .needs_tmpdir).

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20211213121417.21825-1-andrea.cervesato@suse.com/

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-14 15:04 ` Petr Vorel
@ 2021-12-14 15:13   ` Cyril Hrubis
  2021-12-14 16:39     ` Petr Vorel
  0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-12-14 15:13 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> I wonder what I'm missing:
> ./dio_append
> tst_test.c:1409: TINFO: Timeout per run is 0h 05m 00s
> dio_append.c:69: TINFO: Parent append to file
> common.h:45: TBROK: open(dio_append,16449,0666) failed: EINVAL (22)

That's an O_DIRECT open() EINVAL probably means that O_DIRECT is not
supported. Is your /tmp/ on tmpfs?

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-14 15:13   ` Cyril Hrubis
@ 2021-12-14 16:39     ` Petr Vorel
  2021-12-15 11:35       ` Cyril Hrubis
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2021-12-14 16:39 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

> Hi!
> > I wonder what I'm missing:
> > ./dio_append
> > tst_test.c:1409: TINFO: Timeout per run is 0h 05m 00s
> > dio_append.c:69: TINFO: Parent append to file
> > common.h:45: TBROK: open(dio_append,16449,0666) failed: EINVAL (22)

> That's an O_DIRECT open() EINVAL probably means that O_DIRECT is not
> supported. Is your /tmp/ on tmpfs?
Yes. As we cannot use SAFE_OPEN() in io_read_eof() [1], there should be check
for errno I guess.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20211213121417.21825-1-andrea.cervesato@suse.com/


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-14 16:39     ` Petr Vorel
@ 2021-12-15 11:35       ` Cyril Hrubis
  2021-12-15 11:50         ` Petr Vorel
  0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-12-15 11:35 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> > > I wonder what I'm missing:
> > > ./dio_append
> > > tst_test.c:1409: TINFO: Timeout per run is 0h 05m 00s
> > > dio_append.c:69: TINFO: Parent append to file
> > > common.h:45: TBROK: open(dio_append,16449,0666) failed: EINVAL (22)
> 
> > That's an O_DIRECT open() EINVAL probably means that O_DIRECT is not
> > supported. Is your /tmp/ on tmpfs?
> Yes. As we cannot use SAFE_OPEN() in io_read_eof() [1], there should be check
> for errno I guess.

Maybe it would be a bit cleaner to add a check that would attempt to
open a dummy file in the test setup and exit the O_DIRECT tests before
they attempt to fork children...

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-15 11:35       ` Cyril Hrubis
@ 2021-12-15 11:50         ` Petr Vorel
  0 siblings, 0 replies; 7+ messages in thread
From: Petr Vorel @ 2021-12-15 11:50 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi,

...
> > > That's an O_DIRECT open() EINVAL probably means that O_DIRECT is not
> > > supported. Is your /tmp/ on tmpfs?
> > Yes. As we cannot use SAFE_OPEN() in io_read_eof() [1], there should be check
> > for errno I guess.

> Maybe it would be a bit cleaner to add a check that would attempt to
> open a dummy file in the test setup and exit the O_DIRECT tests before
> they attempt to fork children...
+1

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [LTP] [PATCH v4] Refactoring dio_append.c using LTP API
  2021-12-13 16:04 [LTP] [PATCH v4] Refactoring dio_append.c using LTP API Andrea Cervesato via ltp
  2021-12-14 15:04 ` Petr Vorel
@ 2021-12-15 13:11 ` Cyril Hrubis
  1 sibling, 0 replies; 7+ messages in thread
From: Cyril Hrubis @ 2021-12-15 13:11 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
Pushed with two minor changes, thanks.

> +/*\
> + * [Description]
>   *
> - */
> -/*
>   * dio_append - append zeroed data to a file using O_DIRECT while
>   *	a 2nd process is doing buffered reads and check if the buffer
>   *	reads always see zero.
>   */

I did rewrite this description to be a bit better.

> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_tmpdir = 1,
> +	.forks_child = 1,
> +	.options = (struct tst_option[]) {
> +		{"n:", &str_numchildren, "Number of threads (default 1000)"},
> +		{"w:", &str_writesize, "Write size for each append (default 64K)"},
> +		{"c:", &str_appends, "Number of appends (default 1000)"},

And fixed these descriptions. The description for n: didn't match it at
all. For the rest I've added the switch to the string description as
well, as currently the test library is dumb and just prints the string
as it is.


It would be better to fix the library to print the swich automatically
and then fix all the tests not to include the switches in the string
description. And ideally we would pass a default value in this structure
as well...

But for now the message should include the switches otherwise it's
incomprehensible to the user.

> +		{}
> +	},
> +};

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-15 13:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 16:04 [LTP] [PATCH v4] Refactoring dio_append.c using LTP API Andrea Cervesato via ltp
2021-12-14 15:04 ` Petr Vorel
2021-12-14 15:13   ` Cyril Hrubis
2021-12-14 16:39     ` Petr Vorel
2021-12-15 11:35       ` Cyril Hrubis
2021-12-15 11:50         ` Petr Vorel
2021-12-15 13:11 ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.