[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Jorik Cronenberg]

Add the possibility to add a timeout to TST_PROCESS_STATE_WAIT.
Like checkpoints add TST_PROCESS_STATE_WAIT2()
for specifying a timeout. The original TST_PROCESS_STATE_WAIT()
works the same as before. Timeout can be specified in milliseconds.

Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
---
 include/tst_process_state.h | 12 ++++++++----
 lib/tst_process_state.c     | 19 ++++++++++++++-----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/include/tst_process_state.h b/include/tst_process_state.h
index fab0491d9..27a8ffc36 100644
--- a/include/tst_process_state.h
+++ b/include/tst_process_state.h
@@ -47,9 +47,13 @@
  */
 #ifdef TST_TEST_H__
 
+#define TST_PROCESS_STATE_WAIT2(pid, state, msec_timeout) \
+	tst_process_state_wait(__FILE__, __LINE__, NULL, \
+	                       (pid), (state), msec_timeout)
+
 #define TST_PROCESS_STATE_WAIT(pid, state) \
 	tst_process_state_wait(__FILE__, __LINE__, NULL, \
-	                       (pid), (state))
+	                       (pid), (state), 0)
 #else
 /*
  * The same as above but does not use tst_brkm() interface.
@@ -65,8 +69,8 @@ int tst_process_state_wait2(pid_t pid, const char state);
 	                        (pid), (state))
 #endif
 
-void tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void),
-                            pid_t pid, const char state);
+int tst_process_state_wait(const char *file, const int lineno,
+                            void (*cleanup_fn)(void), pid_t pid,
+			    const char state, unsigned int msec_timeout);
 
 #endif /* TST_PROCESS_STATE__ */
diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
index 7a7824959..32b44992c 100644
--- a/lib/tst_process_state.c
+++ b/lib/tst_process_state.c
@@ -28,11 +28,12 @@
 #include "test.h"
 #include "tst_process_state.h"
 
-void tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void),
-                            pid_t pid, const char state)
+int tst_process_state_wait(const char *file, const int lineno,
+                            void (*cleanup_fn)(void), pid_t pid,
+			    const char state, unsigned int msec_timeout)
 {
 	char proc_path[128], cur_state;
+	unsigned int msecs = 0;
 
 	snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
 
@@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file, const int lineno,
 		                "%*i %*s %c", &cur_state);
 
 		if (state == cur_state)
-			return;
+			break;
 
-		usleep(10000);
+		usleep(1000);
+		msecs += 1;
+
+		if (msecs >= msec_timeout) {
+			errno = ETIMEDOUT;
+			return -1;
+		}
 	}
+
+	return 0;
 }
 
 int tst_process_state_wait2(pid_t pid, const char state)
-- 
2.24.1

[LTP] [PATCH 2/2] syscalls/vmsplice: Add NONBLOCK testcase

[Jorik Cronenberg]

Add a testcase for vmsplice() with the flag SPLICE_F_NONBLOCK.
And also test that vmsplice() blocks when writing to a full pipe
without the flag specified.

Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
---
 testcases/kernel/syscalls/vmsplice/.gitignore |  1 +
 .../kernel/syscalls/vmsplice/vmsplice04.c     | 87 +++++++++++++++++++
 2 files changed, 88 insertions(+)
 create mode 100644 testcases/kernel/syscalls/vmsplice/vmsplice04.c

diff --git a/testcases/kernel/syscalls/vmsplice/.gitignore b/testcases/kernel/syscalls/vmsplice/.gitignore
index 03922073c..042c32585 100644
--- a/testcases/kernel/syscalls/vmsplice/.gitignore
+++ b/testcases/kernel/syscalls/vmsplice/.gitignore
@@ -1,3 +1,4 @@
 /vmsplice01
 /vmsplice02
 /vmsplice03
+/vmsplice04
diff --git a/testcases/kernel/syscalls/vmsplice/vmsplice04.c b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
new file mode 100644
index 000000000..c49657d84
--- /dev/null
+++ b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Jorik Cronenberg <jcronenberg@suse.de>
+ *
+ * Test vmsplice() to a full pipe with SPLICE_F_NONBLOCK and without
+ * With SPLICE_F_NONBLOCK vmsplice() should return with errno EAGAIN
+ * Without SPLICE_F_NONBLOCK it should block
+ */
+
+#define _GNU_SOURCE
+
+#include "tst_test.h"
+#include "lapi/vmsplice.h"
+#include <stdlib.h>
+
+
+static int pipes[2];
+static ssize_t pipe_max_size;
+static char *write_buffer;
+
+static void vmsplice_test(void)
+{
+	int status;
+	struct iovec iov;
+	int pid;
+
+	iov.iov_base = write_buffer;
+	iov.iov_len = pipe_max_size;
+
+
+	TEST(vmsplice(pipes[1], &iov, 1, 0));
+	if (TST_RET < 0)
+		tst_brk(TBROK | TTERRNO,
+		    "Initial vmsplice() to fill pipe failed");
+
+	TEST(vmsplice(pipes[1], &iov, 1, SPLICE_F_NONBLOCK));
+	if (TST_RET < 0 && TST_ERR == EAGAIN)
+		tst_res(TPASS | TTERRNO, "vmsplice failed as expected");
+	else if (TST_RET < 0)
+		tst_res(TFAIL | TTERRNO,
+		    "vmsplice failed with unexpected errno");
+	else
+		tst_res(TFAIL, "vmsplice wrote to a full pipe");
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		TEST(vmsplice(pipes[1], &iov, 1, 0));
+		if (TST_RET < 0)
+			tst_res(TFAIL | TTERRNO, "vmsplice() failed");
+		else
+			tst_res(TFAIL, "vmsplice() wrote to a full pipe");
+		exit(0);
+	} else {
+		if (TST_PROCESS_STATE_WAIT2(pid, 'S', 1000) < 0)
+			return;
+		else
+			tst_res(TPASS, "vmsplice() blocked");
+		SAFE_KILL(pid, SIGKILL);
+		SAFE_WAIT(&status);
+	}
+
+}
+static void cleanup(void)
+{
+	if (pipes[1] > 0)
+		SAFE_CLOSE(pipes[1]);
+	if (pipes[0] > 0)
+		SAFE_CLOSE(pipes[0]);
+	if (write_buffer)
+		free(write_buffer);
+}
+static void setup(void)
+{
+	SAFE_PIPE(pipes);
+
+	pipe_max_size = SAFE_FCNTL(pipes[1], F_GETPIPE_SZ);
+	write_buffer = SAFE_MALLOC(pipe_max_size);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = vmsplice_test,
+	.min_kver = "2.6.17",
+	.forks_child = 1,
+};
-- 
2.24.1

[LTP] [PATCH 2/2] syscalls/vmsplice: Add NONBLOCK testcase

[Yang Xu]

Hi
> Add a testcase for vmsplice() with the flag SPLICE_F_NONBLOCK.
> And also test that vmsplice() blocks when writing to a full pipe
> without the flag specified.
> 
> Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
> ---
>   testcases/kernel/syscalls/vmsplice/.gitignore |  1 +
>   .../kernel/syscalls/vmsplice/vmsplice04.c     | 87 +++++++++++++++++++
>   2 files changed, 88 insertions(+)
>   create mode 100644 testcases/kernel/syscalls/vmsplice/vmsplice04.c
add vmsplice04 into runtest/syscalls
> 
> diff --git a/testcases/kernel/syscalls/vmsplice/.gitignore b/testcases/kernel/syscalls/vmsplice/.gitignore
> index 03922073c..042c32585 100644
> --- a/testcases/kernel/syscalls/vmsplice/.gitignore
> +++ b/testcases/kernel/syscalls/vmsplice/.gitignore
> @@ -1,3 +1,4 @@
>   /vmsplice01
>   /vmsplice02
>   /vmsplice03
> +/vmsplice04
> diff --git a/testcases/kernel/syscalls/vmsplice/vmsplice04.c b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
> new file mode 100644
> index 000000000..c49657d84
> --- /dev/null
> +++ b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 SUSE LLC
> + * Author: Jorik Cronenberg <jcronenberg@suse.de>
> + *
> + * Test vmsplice() to a full pipe with SPLICE_F_NONBLOCK and without
> + * With SPLICE_F_NONBLOCK vmsplice() should return with errno EAGAIN
> + * Without SPLICE_F_NONBLOCK it should block
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include "tst_test.h"
> +#include "lapi/vmsplice.h"
> +#include <stdlib.h>
miss "lapi/fcntl.h". it will compile failed because of undefined 
F_GETPIPE_SZ error on centos6 like my pipe12.c(v1)

https://travis-ci.org/xuyang0410/ltp/jobs/629597516?utm_medium=notification&utm_source=github_status

> +
> +
> +static int pipes[2];
> +static ssize_t pipe_max_size;
> +static char *write_buffer;
> +
> +static void vmsplice_test(void)
> +{
> +	int status;
> +	struct iovec iov;
> +	int pid;
> +
> +	iov.iov_base = write_buffer;
> +	iov.iov_len = pipe_max_size;
> +
> +
> +	TEST(vmsplice(pipes[1], &iov, 1, 0));
> +	if (TST_RET < 0)
> +		tst_brk(TBROK | TTERRNO,
> +		    "Initial vmsplice() to fill pipe failed");
> +
> +	TEST(vmsplice(pipes[1], &iov, 1, SPLICE_F_NONBLOCK));
> +	if (TST_RET < 0 && TST_ERR == EAGAIN)
> +		tst_res(TPASS | TTERRNO, "vmsplice failed as expected");
I think we should add more info. such as "vmsplice failed EAGAIN to full 
pipe with SPLICE_F_NONBLOCK mode"
> +	else if (TST_RET < 0)
> +		tst_res(TFAIL | TTERRNO,
> +		    "vmsplice failed with unexpected errno");
here as well
> +	else
> +		tst_res(TFAIL, "vmsplice wrote to a full pipe");
here as well
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		TEST(vmsplice(pipes[1], &iov, 1, 0));
> +		if (TST_RET < 0)
> +			tst_res(TFAIL | TTERRNO, "vmsplice() failed");
> +		else
> +			tst_res(TFAIL, "vmsplice() wrote to a full pipe");
> +		exit(0);
> +	} else {
> +		if (TST_PROCESS_STATE_WAIT2(pid, 'S', 1000) < 0)
> +			return;
> +		else
> +			tst_res(TPASS, "vmsplice() blocked");
here as well (without SPLICE_F_NONBLOCK mode)
> +		SAFE_KILL(pid, SIGKILL);
> +		SAFE_WAIT(&status);
> +	}
> +
> +}
> +static void cleanup(void)
> +{
> +	if (pipes[1] > 0)
> +		SAFE_CLOSE(pipes[1]);
> +	if (pipes[0] > 0)
> +		SAFE_CLOSE(pipes[0]);
> +	if (write_buffer)
> +		free(write_buffer);
> +}
> +static void setup(void)
> +{
> +	SAFE_PIPE(pipes);
> +
> +	pipe_max_size = SAFE_FCNTL(pipes[1], F_GETPIPE_SZ);
> +	write_buffer = SAFE_MALLOC(pipe_max_size);
Can we use guarded buffer for iov in setup?
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#2231-guarded-buffers

other than, this patch looks good to me.
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = vmsplice_test,
> +	.min_kver = "2.6.17",
> +	.forks_child = 1,
> +};
> 

[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Yang Xu]

> Add the possibility to add a timeout to TST_PROCESS_STATE_WAIT.
> Like checkpoints add TST_PROCESS_STATE_WAIT2()
> for specifying a timeout. The original TST_PROCESS_STATE_WAIT()
> works the same as before. Timeout can be specified in milliseconds.
> 
Hi Jorik

We have tst_process_state_wait2 since commit dbf270c5 ("lib: Add 
tst_process_state_wait2()"), this api has same functions as 
tst_process_state_wait but only return error instead of TBROK.

I think using TST_PROCESS_STATE_WAIT2 is confused and we can only expand
tst_process_state_wait make it support sleep specifying in milliseconds.

Best Regards
Yang Xu
> Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
> ---
>   include/tst_process_state.h | 12 ++++++++----
>   lib/tst_process_state.c     | 19 ++++++++++++++-----
>   2 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/include/tst_process_state.h b/include/tst_process_state.h
> index fab0491d9..27a8ffc36 100644
> --- a/include/tst_process_state.h
> +++ b/include/tst_process_state.h
> @@ -47,9 +47,13 @@
>    */
>   #ifdef TST_TEST_H__
>   
> +#define TST_PROCESS_STATE_WAIT2(pid, state, msec_timeout) \
> +	tst_process_state_wait(__FILE__, __LINE__, NULL, \
> +	                       (pid), (state), msec_timeout)
> +
>   #define TST_PROCESS_STATE_WAIT(pid, state) \
>   	tst_process_state_wait(__FILE__, __LINE__, NULL, \
> -	                       (pid), (state))
> +	                       (pid), (state), 0)
>   #else
>   /*
>    * The same as above but does not use tst_brkm() interface.
> @@ -65,8 +69,8 @@ int tst_process_state_wait2(pid_t pid, const char state);
>   	                        (pid), (state))
>   #endif
>   
> -void tst_process_state_wait(const char *file, const int lineno,
> -                            void (*cleanup_fn)(void),
> -                            pid_t pid, const char state);
> +int tst_process_state_wait(const char *file, const int lineno,
> +                            void (*cleanup_fn)(void), pid_t pid,
> +			    const char state, unsigned int msec_timeout);
>   
>   #endif /* TST_PROCESS_STATE__ */
> diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
> index 7a7824959..32b44992c 100644
> --- a/lib/tst_process_state.c
> +++ b/lib/tst_process_state.c
> @@ -28,11 +28,12 @@
>   #include "test.h"
>   #include "tst_process_state.h"
>   
> -void tst_process_state_wait(const char *file, const int lineno,
> -                            void (*cleanup_fn)(void),
> -                            pid_t pid, const char state)
> +int tst_process_state_wait(const char *file, const int lineno,
> +                            void (*cleanup_fn)(void), pid_t pid,
> +			    const char state, unsigned int msec_timeout)
>   {
>   	char proc_path[128], cur_state;
> +	unsigned int msecs = 0;
>   
>   	snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
>   
> @@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file, const int lineno,
>   		                "%*i %*s %c", &cur_state);
>   
>   		if (state == cur_state)
> -			return;
> +			break;
>   
> -		usleep(10000);
> +		usleep(1000);
> +		msecs += 1;
> +
> +		if (msecs >= msec_timeout) {
> +			errno = ETIMEDOUT;
> +			return -1;
> +		}
>   	}
> +
> +	return 0;
>   }
>   
>   int tst_process_state_wait2(pid_t pid, const char state)
> 

[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Jorik Cronenberg]

Hi Yang, thanks for the review!

>
>> Add the possibility to add a timeout to TST_PROCESS_STATE_WAIT.
>> Like checkpoints add TST_PROCESS_STATE_WAIT2()
>> for specifying a timeout. The original TST_PROCESS_STATE_WAIT()
>> works the same as before. Timeout can be specified in milliseconds.
>>
> Hi Jorik
>
> We have tst_process_state_wait2 since commit dbf270c5 ("lib: Add
> tst_process_state_wait2()"), this api has same functions as
> tst_process_state_wait but only return error instead of TBROK.
>
> I think using TST_PROCESS_STATE_WAIT2 is confused and we can only expand
> tst_process_state_wait make it support sleep specifying in milliseconds.
>
> Best Regards
> Yang Xu

I don't think I quite understand what you mean. I can see that using
TST_PROCESS_STATE_WAIT2 is confusing. But I didn't want to touch the
existing TST_PROCESS_STATE_WAIT to ensure all older tests still run the
same. Are you saying i should go through all tests that use
TST_PROCESS_STATE_WAIT and specify that they use a timeout of 0(which
according to a git grep doesn't seem too many, so it wouldn't be too
much effort) and then change TST_PROCESS_STATE_WAIT to include a timeout
or should I just rename TST_PROCESS_STATE_WAIT2 to something that
seperates it more from tst_process_state_wait2?

regards,
Jorik Cronenberg
>> Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
>> ---
>> ? include/tst_process_state.h | 12 ++++++++----
>> ? lib/tst_process_state.c???? | 19 ++++++++++++++-----
>> ? 2 files changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/tst_process_state.h b/include/tst_process_state.h
>> index fab0491d9..27a8ffc36 100644
>> --- a/include/tst_process_state.h
>> +++ b/include/tst_process_state.h
>> @@ -47,9 +47,13 @@
>> ?? */
>> ? #ifdef TST_TEST_H__
>> ? +#define TST_PROCESS_STATE_WAIT2(pid, state, msec_timeout) \
>> +??? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>> +?????????????????????????? (pid), (state), msec_timeout)
>> +
>> ? #define TST_PROCESS_STATE_WAIT(pid, state) \
>> ????? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>> -?????????????????????????? (pid), (state))
>> +?????????????????????????? (pid), (state), 0)
>> ? #else
>> ? /*
>> ?? * The same as above but does not use tst_brkm() interface.
>> @@ -65,8 +69,8 @@ int tst_process_state_wait2(pid_t pid, const char
>> state);
>> ????????????????????????????? (pid), (state))
>> ? #endif
>> ? -void tst_process_state_wait(const char *file, const int lineno,
>> -??????????????????????????? void (*cleanup_fn)(void),
>> -??????????????????????????? pid_t pid, const char state);
>> +int tst_process_state_wait(const char *file, const int lineno,
>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>> +??????????????? const char state, unsigned int msec_timeout);
>> ? ? #endif /* TST_PROCESS_STATE__ */
>> diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
>> index 7a7824959..32b44992c 100644
>> --- a/lib/tst_process_state.c
>> +++ b/lib/tst_process_state.c
>> @@ -28,11 +28,12 @@
>> ? #include "test.h"
>> ? #include "tst_process_state.h"
>> ? -void tst_process_state_wait(const char *file, const int lineno,
>> -??????????????????????????? void (*cleanup_fn)(void),
>> -??????????????????????????? pid_t pid, const char state)
>> +int tst_process_state_wait(const char *file, const int lineno,
>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>> +??????????????? const char state, unsigned int msec_timeout)
>> ? {
>> ????? char proc_path[128], cur_state;
>> +??? unsigned int msecs = 0;
>> ? ????? snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
>> ? @@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file,
>> const int lineno,
>> ????????????????????????? "%*i %*s %c", &cur_state);
>> ? ????????? if (state == cur_state)
>> -??????????? return;
>> +??????????? break;
>> ? -??????? usleep(10000);
>> +??????? usleep(1000);
>> +??????? msecs += 1;
>> +
>> +??????? if (msecs >= msec_timeout) {
>> +??????????? errno = ETIMEDOUT;
>> +??????????? return -1;
>> +??????? }
>> ????? }
>> +
>> +??? return 0;
>> ? }
>> ? ? int tst_process_state_wait2(pid_t pid, const char state)
>>
>
>

[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Yang Xu]

Hi Jorik
> Hi Yang, thanks for the review!
> 
>>
>>> Add the possibility to add a timeout to TST_PROCESS_STATE_WAIT.
>>> Like checkpoints add TST_PROCESS_STATE_WAIT2()
>>> for specifying a timeout. The original TST_PROCESS_STATE_WAIT()
>>> works the same as before. Timeout can be specified in milliseconds.
>>>
>> Hi Jorik
>>
>> We have tst_process_state_wait2 since commit dbf270c5 ("lib: Add
>> tst_process_state_wait2()"), this api has same functions as
>> tst_process_state_wait but only return error instead of TBROK.
>>
>> I think using TST_PROCESS_STATE_WAIT2 is confused and we can only expand
>> tst_process_state_wait make it support sleep specifying in milliseconds.
>>
>> Best Regards
>> Yang Xu
> 
> I don't think I quite understand what you mean. I can see that using
> TST_PROCESS_STATE_WAIT2 is confusing. But I didn't want to touch the
> existing TST_PROCESS_STATE_WAIT to ensure all older tests still run the
> same. Are you saying i should go through all tests that use
> TST_PROCESS_STATE_WAIT and specify that they use a timeout of 0(which
> according to a git grep doesn't seem too many, so it wouldn't be too
> much effort) and then change TST_PROCESS_STATE_WAIT to include a timeout
Yes.
> or should I just rename TST_PROCESS_STATE_WAIT2 to something that
> seperates it more from tst_process_state_wait2?
Also, I am fine with the second way. Let we listen cyril's advise.
@Cyril What do you think about it?
> 
> regards,
> Jorik Cronenberg
>>> Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
>>> ---
>>>  ? include/tst_process_state.h | 12 ++++++++----
>>>  ? lib/tst_process_state.c???? | 19 ++++++++++++++-----
>>>  ? 2 files changed, 22 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/tst_process_state.h b/include/tst_process_state.h
>>> index fab0491d9..27a8ffc36 100644
>>> --- a/include/tst_process_state.h
>>> +++ b/include/tst_process_state.h
>>> @@ -47,9 +47,13 @@
>>>  ?? */
>>>  ? #ifdef TST_TEST_H__
>>>  ? +#define TST_PROCESS_STATE_WAIT2(pid, state, msec_timeout) \
>>> +??? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>>> +?????????????????????????? (pid), (state), msec_timeout)
>>> +
>>>  ? #define TST_PROCESS_STATE_WAIT(pid, state) \
>>>  ????? tst_process_state_wait(__FILE__, __LINE__, NULL, \
>>> -?????????????????????????? (pid), (state))
>>> +?????????????????????????? (pid), (state), 0)
>>>  ? #else
>>>  ? /*
>>>  ?? * The same as above but does not use tst_brkm() interface.
>>> @@ -65,8 +69,8 @@ int tst_process_state_wait2(pid_t pid, const char
>>> state);
>>>  ????????????????????????????? (pid), (state))
>>>  ? #endif
>>>  ? -void tst_process_state_wait(const char *file, const int lineno,
>>> -??????????????????????????? void (*cleanup_fn)(void),
>>> -??????????????????????????? pid_t pid, const char state);
>>> +int tst_process_state_wait(const char *file, const int lineno,
>>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>>> +??????????????? const char state, unsigned int msec_timeout);
>>>  ? ? #endif /* TST_PROCESS_STATE__ */
>>> diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
>>> index 7a7824959..32b44992c 100644
>>> --- a/lib/tst_process_state.c
>>> +++ b/lib/tst_process_state.c
>>> @@ -28,11 +28,12 @@
>>>  ? #include "test.h"
>>>  ? #include "tst_process_state.h"
>>>  ? -void tst_process_state_wait(const char *file, const int lineno,
>>> -??????????????????????????? void (*cleanup_fn)(void),
>>> -??????????????????????????? pid_t pid, const char state)
>>> +int tst_process_state_wait(const char *file, const int lineno,
>>> +??????????????????????????? void (*cleanup_fn)(void), pid_t pid,
>>> +??????????????? const char state, unsigned int msec_timeout)
>>>  ? {
>>>  ????? char proc_path[128], cur_state;
>>> +??? unsigned int msecs = 0;
>>>  ? ????? snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
>>>  ? @@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file,
>>> const int lineno,
>>>  ????????????????????????? "%*i %*s %c", &cur_state);
>>>  ? ????????? if (state == cur_state)
>>> -??????????? return;
>>> +??????????? break;
>>>  ? -??????? usleep(10000);
>>> +??????? usleep(1000);
>>> +??????? msecs += 1;
>>> +
>>> +??????? if (msecs >= msec_timeout) {
>>> +??????????? errno = ETIMEDOUT;
>>> +??????????? return -1;
>>> +??????? }
>>>  ????? }
>>> +
>>> +??? return 0;
>>>  ? }
>>>  ? ? int tst_process_state_wait2(pid_t pid, const char state)
>>>
>>
>>
> 

[LTP] [PATCH 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Cyril Hrubis]

Hi!
> >> We have tst_process_state_wait2 since commit dbf270c5 ("lib: Add
> >> tst_process_state_wait2()"), this api has same functions as
> >> tst_process_state_wait but only return error instead of TBROK.
> >>
> >> I think using TST_PROCESS_STATE_WAIT2 is confused and we can only expand
> >> tst_process_state_wait make it support sleep specifying in milliseconds.
> >>
> >> Best Regards
> >> Yang Xu
> > 
> > I don't think I quite understand what you mean. I can see that using
> > TST_PROCESS_STATE_WAIT2 is confusing. But I didn't want to touch the
> > existing TST_PROCESS_STATE_WAIT to ensure all older tests still run the
> > same. Are you saying i should go through all tests that use
> > TST_PROCESS_STATE_WAIT and specify that they use a timeout of 0(which
> > according to a git grep doesn't seem too many, so it wouldn't be too
> > much effort) and then change TST_PROCESS_STATE_WAIT to include a timeout
> Yes.
> > or should I just rename TST_PROCESS_STATE_WAIT2 to something that
> > seperates it more from tst_process_state_wait2?
> Also, I am fine with the second way. Let we listen cyril's advise.
> @Cyril What do you think about it?

Well that function is only supposed to be used from the old API while
the macro is defined for the new API so they are never exported at the
same time.

On the other hand we can change these few testcases quite easily, so we
may as well do that.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 1/2] lib: Add timeout to TST_PROCESS_STATE_WAIT

[Jorik Cronenberg]

Add a timeout to TST_PROCESS_STATE_WAIT. Timeout can be
specified in milliseconds. After timeout it returns -1
and sets errno to ETIMEDOUT. Specifying a timeout of 0
makes it for the most part function like the old
TST_PROCESS_STATE_WAIT.

Update all existing testcases that used TST_PROCESS_STATE_WAIT
and the new test library.

Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
---
v2:
removed TST_PROCESS_STATE_WAIT2
change all existing testcases that use TST_PROCESS_STATE_WAIT and
new testlib
change doc to reflect changes

also while going through tests I noticed that my previous
implementation was broken and did timeout even with timeout
set to 0, that should now also be fixed.

 doc/test-writing-guidelines.txt               |  7 +++++--
 include/tst_process_state.h                   | 12 ++++++------
 lib/tst_process_state.c                       | 19 ++++++++++++++-----
 testcases/kernel/mem/mtest01/mtest01.c        |  2 +-
 testcases/kernel/syscalls/clone/clone08.c     |  2 +-
 .../syscalls/futex/futex_cmp_requeue01.c      |  2 +-
 .../kernel/syscalls/ipc/msgsnd/msgsnd05.c     |  2 +-
 .../kernel/syscalls/ipc/msgsnd/msgsnd06.c     |  2 +-
 testcases/kernel/syscalls/pause/pause01.c     |  2 +-
 testcases/kernel/syscalls/wait4/wait401.c     |  2 +-
 10 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index f0aa69ad4..bfc3b5554 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -820,11 +820,14 @@ For the details of the interface, look into the 'include/tst_checkpoint.h'.
  * Z - zombie process
  * T - process is traced
  */
-TST_PROCESS_STATE_WAIT(pid, state)
+TST_PROCESS_STATE_WAIT(pid, state, msec_timeout)
 -------------------------------------------------------------------------------
 
 The 'TST_PROCESS_STATE_WAIT()' waits until process 'pid' is in requested
-'state'. The call polls +/proc/pid/stat+ to get this information.
+'state' or timeout is reached. The call polls +/proc/pid/stat+ to get this
+information. A timeout of 0 will wait infinitely.
+
+On timeout -1 is returned and errno set to ETIMEDOUT.
 
 It's mostly used with state 'S' which means that process is sleeping in kernel
 for example in 'pause()' or any other blocking syscall.
diff --git a/include/tst_process_state.h b/include/tst_process_state.h
index fab0491d9..a070e0921 100644
--- a/include/tst_process_state.h
+++ b/include/tst_process_state.h
@@ -47,9 +47,9 @@
  */
 #ifdef TST_TEST_H__
 
-#define TST_PROCESS_STATE_WAIT(pid, state) \
+#define TST_PROCESS_STATE_WAIT(pid, state, msec_timeout) \
 	tst_process_state_wait(__FILE__, __LINE__, NULL, \
-	                       (pid), (state))
+	                       (pid), (state), (msec_timeout))
 #else
 /*
  * The same as above but does not use tst_brkm() interface.
@@ -62,11 +62,11 @@ int tst_process_state_wait2(pid_t pid, const char state);
 
 # define TST_PROCESS_STATE_WAIT(cleanup_fn, pid, state) \
 	 tst_process_state_wait(__FILE__, __LINE__, (cleanup_fn), \
-	                        (pid), (state))
+	                        (pid), (state), 0)
 #endif
 
-void tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void),
-                            pid_t pid, const char state);
+int tst_process_state_wait(const char *file, const int lineno,
+                            void (*cleanup_fn)(void), pid_t pid,
+			    const char state, unsigned int msec_timeout);
 
 #endif /* TST_PROCESS_STATE__ */
diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
index 7a7824959..323684b55 100644
--- a/lib/tst_process_state.c
+++ b/lib/tst_process_state.c
@@ -28,11 +28,12 @@
 #include "test.h"
 #include "tst_process_state.h"
 
-void tst_process_state_wait(const char *file, const int lineno,
-                            void (*cleanup_fn)(void),
-                            pid_t pid, const char state)
+int tst_process_state_wait(const char *file, const int lineno,
+                            void (*cleanup_fn)(void), pid_t pid,
+			    const char state, unsigned int msec_timeout)
 {
 	char proc_path[128], cur_state;
+	unsigned int msecs = 0;
 
 	snprintf(proc_path, sizeof(proc_path), "/proc/%i/stat", pid);
 
@@ -41,10 +42,18 @@ void tst_process_state_wait(const char *file, const int lineno,
 		                "%*i %*s %c", &cur_state);
 
 		if (state == cur_state)
-			return;
+			break;
 
-		usleep(10000);
+		usleep(1000);
+		msecs += 1;
+
+		if (msecs >= msec_timeout && msec_timeout) {
+			errno = ETIMEDOUT;
+			return -1;
+		}
 	}
+
+	return 0;
 }
 
 int tst_process_state_wait2(pid_t pid, const char state)
diff --git a/testcases/kernel/mem/mtest01/mtest01.c b/testcases/kernel/mem/mtest01/mtest01.c
index 446d26897..f08d3943f 100644
--- a/testcases/kernel/mem/mtest01/mtest01.c
+++ b/testcases/kernel/mem/mtest01/mtest01.c
@@ -227,7 +227,7 @@ static void mem_test(void)
 			alloc_maxbytes / 1024, write_msg);
 
 	for (i = 0; i < pid_cntr; i++) {
-		TST_PROCESS_STATE_WAIT(pid_list[i], 'T');
+		TST_PROCESS_STATE_WAIT(pid_list[i], 'T', 0);
 		kill(pid_list[i], SIGCONT);
 	}
 }
diff --git a/testcases/kernel/syscalls/clone/clone08.c b/testcases/kernel/syscalls/clone/clone08.c
index aace30806..8e115b042 100644
--- a/testcases/kernel/syscalls/clone/clone08.c
+++ b/testcases/kernel/syscalls/clone/clone08.c
@@ -159,7 +159,7 @@ static void test_clone_stopped(int t)
 
 	child = clone_child(&test_cases[t]);
 
-	TST_PROCESS_STATE_WAIT(child, 'T');
+	TST_PROCESS_STATE_WAIT(child, 'T', 0);
 
 	stopped_flag = 0;
 
diff --git a/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c b/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
index a2e899b8d..f5e88a0d5 100644
--- a/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
+++ b/testcases/kernel/syscalls/futex/futex_cmp_requeue01.c
@@ -87,7 +87,7 @@ static void verify_futex_cmp_requeue(unsigned int n)
 	}
 
 	for (i = 0; i < tc->num_waiters; i++)
-		TST_PROCESS_STATE_WAIT(pid[i], 'S');
+		TST_PROCESS_STATE_WAIT(pid[i], 'S', 0);
 
 	tst_res(TINFO, "Test %d: waiters: %d, wakes: %d, requeues: %d",
 		n, tc->num_waiters, tc->set_wakes, tc->set_requeues);
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
index e169831e0..ace32cdaa 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd05.c
@@ -80,7 +80,7 @@ static void do_test(unsigned int n)
 		_exit(0);
 	}
 
-	TST_PROCESS_STATE_WAIT(pid, 'S');
+	TST_PROCESS_STATE_WAIT(pid, 'S', 0);
 	SAFE_KILL(pid, SIGHUP);
 	tst_reap_children();
 }
diff --git a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
index e7855e844..9f462b672 100644
--- a/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
+++ b/testcases/kernel/syscalls/ipc/msgsnd/msgsnd06.c
@@ -57,7 +57,7 @@ static void do_test(void)
 		_exit(0);
 	}
 
-	TST_PROCESS_STATE_WAIT(pid, 'S');
+	TST_PROCESS_STATE_WAIT(pid, 'S', 0);
 
 	SAFE_MSGCTL(queue_id, IPC_RMID, NULL);
 
diff --git a/testcases/kernel/syscalls/pause/pause01.c b/testcases/kernel/syscalls/pause/pause01.c
index d44e7972a..c88248da0 100644
--- a/testcases/kernel/syscalls/pause/pause01.c
+++ b/testcases/kernel/syscalls/pause/pause01.c
@@ -38,7 +38,7 @@ static void do_test(void)
 		do_child();
 
 	TST_CHECKPOINT_WAIT(0);
-	TST_PROCESS_STATE_WAIT(pid, 'S');
+	TST_PROCESS_STATE_WAIT(pid, 'S', 0);
 	kill(pid, SIGINT);
 
 	/*
diff --git a/testcases/kernel/syscalls/wait4/wait401.c b/testcases/kernel/syscalls/wait4/wait401.c
index fa0de693a..ed42d8659 100644
--- a/testcases/kernel/syscalls/wait4/wait401.c
+++ b/testcases/kernel/syscalls/wait4/wait401.c
@@ -25,7 +25,7 @@ static void run(void)
 
 	pid = SAFE_FORK();
 	if (!pid) {
-		TST_PROCESS_STATE_WAIT(getppid(), 'S');
+		TST_PROCESS_STATE_WAIT(getppid(), 'S', 0);
 		exit(0);
 	}
 
-- 
2.25.0

[LTP] [PATCH 2/2] syscalls/vmsplice: Add NONBLOCK testcase

[Jorik Cronenberg]

Add a testcase for vmsplice() with the flag SPLICE_F_NONBLOCK.
And also test that vmsplice() blocks when writing to a full pipe
without the flag specified.

---
v2:
Made changes pointed out by Yang Xu:

Add the test to runtest/syscalls
Add "lapi/fcntl.h"
Add additional info to the test's output messages
Use guarded buffer for write_buffer
---

Signed-off-by: Jorik Cronenberg <jcronenberg@suse.de>
---
 runtest/syscalls                              |  1 +
 testcases/kernel/syscalls/vmsplice/.gitignore |  1 +
 .../kernel/syscalls/vmsplice/vmsplice04.c     | 89 +++++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 testcases/kernel/syscalls/vmsplice/vmsplice04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index f58fefe17..ec94c554d 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1550,6 +1550,7 @@ vhangup02 vhangup02
 vmsplice01 vmsplice01
 vmsplice02 vmsplice02
 vmsplice03 vmsplice03
+vmsplice04 vmsplice04
 
 wait01 wait01
 wait02 wait02
diff --git a/testcases/kernel/syscalls/vmsplice/.gitignore b/testcases/kernel/syscalls/vmsplice/.gitignore
index 03922073c..042c32585 100644
--- a/testcases/kernel/syscalls/vmsplice/.gitignore
+++ b/testcases/kernel/syscalls/vmsplice/.gitignore
@@ -1,3 +1,4 @@
 /vmsplice01
 /vmsplice02
 /vmsplice03
+/vmsplice04
diff --git a/testcases/kernel/syscalls/vmsplice/vmsplice04.c b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
new file mode 100644
index 000000000..0952d7caf
--- /dev/null
+++ b/testcases/kernel/syscalls/vmsplice/vmsplice04.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Jorik Cronenberg <jcronenberg@suse.de>
+ *
+ * Test vmsplice() to a full pipe with SPLICE_F_NONBLOCK and without
+ * With SPLICE_F_NONBLOCK vmsplice() should return with errno EAGAIN
+ * Without SPLICE_F_NONBLOCK it should block
+ */
+
+#define _GNU_SOURCE
+
+#include "tst_test.h"
+#include "lapi/vmsplice.h"
+#include "lapi/fcntl.h"
+#include <stdlib.h>
+
+
+static int pipes[2];
+static ssize_t pipe_max_size;
+static char *write_buffer;
+static struct iovec iov;
+
+static void vmsplice_test(void)
+{
+	int status;
+	int pid;
+
+	TEST(vmsplice(pipes[1], &iov, 1, 0));
+	if (TST_RET < 0)
+		tst_brk(TBROK | TTERRNO,
+		    "Initial vmsplice() to fill pipe failed");
+
+	TEST(vmsplice(pipes[1], &iov, 1, SPLICE_F_NONBLOCK));
+	if (TST_RET < 0 && TST_ERR == EAGAIN)
+		tst_res(TPASS | TTERRNO,
+		    "vmsplice(... , SPLICE_F_NONBLOCK) failed as expected");
+	else if (TST_RET < 0)
+		tst_res(TFAIL | TTERRNO,
+		    "vmsplice(... , SPLICE_F_NONBLOCK) "
+		    "failed with unexpected errno");
+	else
+		tst_res(TFAIL,
+		    "vmsplice(... , SPLICE_F_NONBLOCK) wrote to a full pipe");
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		TEST(vmsplice(pipes[1], &iov, 1, 0));
+		if (TST_RET < 0)
+			tst_res(TFAIL | TTERRNO, "vmsplice(... , 0) failed");
+		else
+			tst_res(TFAIL,
+			    "vmsplice(... , 0) wrote to a full pipe");
+		exit(0);
+	} else {
+		if (TST_PROCESS_STATE_WAIT(pid, 'S', 1000) < 0)
+			return;
+		else
+			tst_res(TPASS, "vmsplice(... , 0) blocked");
+		SAFE_KILL(pid, SIGKILL);
+		SAFE_WAIT(&status);
+	}
+
+}
+static void cleanup(void)
+{
+	if (pipes[1] > 0)
+		SAFE_CLOSE(pipes[1]);
+	if (pipes[0] > 0)
+		SAFE_CLOSE(pipes[0]);
+}
+static void setup(void)
+{
+	SAFE_PIPE(pipes);
+
+	pipe_max_size = SAFE_FCNTL(pipes[1], F_GETPIPE_SZ);
+	write_buffer = tst_alloc(pipe_max_size);
+
+	iov.iov_base = write_buffer;
+	iov.iov_len = pipe_max_size;
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = vmsplice_test,
+	.min_kver = "2.6.17",
+	.forks_child = 1,
+};
-- 
2.25.0

[LTP] [PATCH 2/2] syscalls/vmsplice: Add NONBLOCK testcase

[Cyril Hrubis]

Hi!
Pushed with minor changes, thanks.

* Moved the initial vmsplice() to fill the pipe into the test setup
  otherwise the test breaks on -i parameter

* Added curly braces around multiline if/else blocks
  since this is prefered by LKML see:
  https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces

* I've put the parent code in the second test out of the else block,
  the child does not return, there is no need to execute the rest of the
  function in an else block

* Put single empty lines between functions, it's much more readable that
  way

Also btw, if you put signed-off-by line after the --- it gets removed,
since everything after --- that is not the actual patch gets ignored, so
the signed-off-by should have been before the block that described the
changes in the commit.

-- 
Cyril Hrubis
chrubis@suse.cz