[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[Xie Ziyao]

We don't need do_child() loop 'TEST_LOOPING(lc)' times to invoke the
recvfrom() function, whitch makes no sense. If we run with "-i large number",
this will slow down the program.

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
 testcases/kernel/syscalls/sendfile/sendfile02.c | 14 ++++++++------
 testcases/kernel/syscalls/sendfile/sendfile04.c | 14 ++++++++------
 testcases/kernel/syscalls/sendfile/sendfile05.c | 14 ++++++++------
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/testcases/kernel/syscalls/sendfile/sendfile02.c b/testcases/kernel/syscalls/sendfile/sendfile02.c
index e5f115146..acd7a5a50 100644
--- a/testcases/kernel/syscalls/sendfile/sendfile02.c
+++ b/testcases/kernel/syscalls/sendfile/sendfile02.c
@@ -156,15 +156,17 @@ void do_sendfile(OFF_T offset, int i)
  */
 void do_child(void)
 {
-	int lc;
-	socklen_t length;
 	char rbuf[4096];
+	ssize_t ret = 0;
+	socklen_t length = sizeof(sin1);

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		length = sizeof(sin1);
-		recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1,
-			 &length);
+	ret = recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1, &length);
+	if (ret < 0) {
+		tst_resm(TFAIL, "child process recvfrom failed: %s\n",
+			 strerror(errno));
+		exit(1);
 	}
+
 	exit(0);
 }

diff --git a/testcases/kernel/syscalls/sendfile/sendfile04.c b/testcases/kernel/syscalls/sendfile/sendfile04.c
index 0f315abb0..322e2b58e 100644
--- a/testcases/kernel/syscalls/sendfile/sendfile04.c
+++ b/testcases/kernel/syscalls/sendfile/sendfile04.c
@@ -153,15 +153,17 @@ void do_sendfile(int prot, int pass_unmapped_buffer)
  */
 void do_child(void)
 {
-	int lc;
-	socklen_t length;
 	char rbuf[4096];
+	ssize_t ret = 0;
+	socklen_t length = sizeof(sin1);

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		length = sizeof(sin1);
-		recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1,
-			 &length);
+	ret = recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1, &length);
+	if (ret < 0) {
+		tst_resm(TFAIL, "child process recvfrom failed: %s\n",
+			 strerror(errno));
+		exit(1);
 	}
+
 	exit(0);
 }

diff --git a/testcases/kernel/syscalls/sendfile/sendfile05.c b/testcases/kernel/syscalls/sendfile/sendfile05.c
index 0f268ceb3..a8e76d91d 100644
--- a/testcases/kernel/syscalls/sendfile/sendfile05.c
+++ b/testcases/kernel/syscalls/sendfile/sendfile05.c
@@ -118,15 +118,17 @@ void do_sendfile(void)
  */
 void do_child(void)
 {
-	int lc;
-	socklen_t length;
 	char rbuf[4096];
+	ssize_t ret = 0;
+	socklen_t length = sizeof(sin1);

-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		length = sizeof(sin1);
-		recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1,
-			 &length);
+	ret = recvfrom(sockfd, rbuf, 4096, 0, (struct sockaddr *)&sin1, &length);
+	if (ret < 0) {
+		tst_resm(TFAIL, "child process recvfrom failed: %s\n",
+			 strerror(errno));
+		exit(1);
 	}
+
 	exit(0);
 }

--
2.17.1

[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[Cyril Hrubis]

Hi!
Looking at the code the sendfile04 and sendfile05 does not need 99% of
the code in the tests, as a matter of fact it even breaks the tests with
sufficiently large -i parameter.

Can we please instead remove all the server and socket code from
sendfile04 and sendfile05? We can just easily pass in_fd and out_fd
pointing to a regular files to the sendfile() syscall instead.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[xieziyao]

Of course, I'm interested in modifying the testcases that way, and I'll submit another version to fix the problem.

Thanks for your review, Cyril.

Best Regards,
Ziyao

-----Original Message-----
From: Cyril Hrubis [mailto:chrubis@suse.cz] 
Sent: Wednesday, April 21, 2021 8:35 PM
To: xieziyao <xieziyao@huawei.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

Hi!
Looking at the code the sendfile04 and sendfile05 does not need 99% of the code in the tests, as a matter of fact it even breaks the tests with sufficiently large -i parameter.

Can we please instead remove all the server and socket code from
sendfile04 and sendfile05? We can just easily pass in_fd and out_fd pointing to a regular files to the sendfile() syscall instead.

--
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[xieziyao]

Hi, Cyril,

I just made changes to sendfile04 and sendfile05 on your suggestions:
1. Convert sendfile04 to the new API;
2. Remove the socket code of server/client and use SAFE_SOCKETPAIR() instead, which can simplify the code logic.

And I think the other testcases of the sendfile testsuite have the same problem. Can we please modify other testcases in the same way?

Please see: https://patchwork.ozlabs.org/project/ltp/list/?series=240489

Best Regards,
Ziyao

-----Original Message-----
From: xieziyao 
Sent: Thursday, April 22, 2021 3:13 PM
To: 'Cyril Hrubis' <chrubis@suse.cz>
Cc: ltp@lists.linux.it
Subject: RE: [LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

Of course, I'm interested in modifying the testcases that way, and I'll submit another version to fix the problem.

Thanks for your review, Cyril.

Best Regards,
Ziyao

-----Original Message-----
From: Cyril Hrubis [mailto:chrubis@suse.cz] 
Sent: Wednesday, April 21, 2021 8:35 PM
To: xieziyao <xieziyao@huawei.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

Hi!
Looking at the code the sendfile04 and sendfile05 does not need 99% of the code in the tests, as a matter of fact it even breaks the tests with sufficiently large -i parameter.

Can we please instead remove all the server and socket code from
sendfile04 and sendfile05? We can just easily pass in_fd and out_fd pointing to a regular files to the sendfile() syscall instead.

--
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/sendfile: Delete unnecessary loop logic in do_child()

[Cyril Hrubis]

Hi!
> And I think the other testcases of the sendfile testsuite have the
> same problem. Can we please modify other testcases in the same way?

Yes please, at least sendfile03, sendfile07 should be cleand up the same
way as the sendfile04 and sendfile05 were.

The rest of the tests seems to actually transfer data, but the code is
really messy so they should be cleaned up and converted to new library
as well.

-- 
Cyril Hrubis
chrubis@suse.cz