aio_suspend may be interrupted by completion signal.

aio_suspend may be interrupted by IO request's completion signal, so we should use aio_error instead signal hander to check whether this request has complete.

Signed-off-by Bian Naimeng <[email protected]>

---
 .../conformance/interfaces/aio_suspend/1-1.c       |   49 +++++++------------
 1 files changed, 18 insertions(+), 31 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +11 -8

index 7d33e62..63d986e 100644
@@ -44,18 +44,23 @@
 #define BUF_SIZE	1024*1024
 #define WAIT_FOR_AIOCB	6
 
-int received_selected	= 0;
 int received_all	= 0;
 
-void
-sigrt1_handler(int signum, siginfo_t *info, void *context)
+int selected_request_status(struct aiocb *paiocb, int expect)
 {
-	if (info->si_value.sival_int == WAIT_FOR_AIOCB)
-		received_selected = 1;
+	int err = aio_error(paiocb);
+
+	if (err != expect) {
+		printf (TNAME " Selected request expect %d[%s], not %d[%s]\n",
+			expect, strerror(expect), err, strerror(err));
+		return 0;
+	}
+
+	return 1;
 }
 
 void
-sigrt2_handler(int signum, siginfo_t *info, void *context)
+sigrt1_handler(int signum, siginfo_t *info, void *context)
 {
 	received_all = 1;
 }

@@ -123,37 +128,25 @@ main ()
 		aiocbs[i]->aio_buf = &bufs[i*BUF_SIZE];
 		aiocbs[i]->aio_nbytes = BUF_SIZE;
 		aiocbs[i]->aio_lio_opcode = LIO_READ;
-
-		/* Use SIRTMIN+1 for individual completions */

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +1 -4

-	/* Setup handler for individual operation completion */
+	/* Setup handler for list completion */
 	action.sa_sigaction = sigrt1_handler;
 	sigemptyset(&action.sa_mask);
 	action.sa_flags = SA_SIGINFO|SA_RESTART;
 	sigaction(SIGRTMIN+1, &action, NULL);
 
-	/* Setup handler for list completion */
-	action.sa_sigaction = sigrt2_handler;
-	sigemptyset(&action.sa_mask);

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +0 -0

 	if (ret) {
 		printf(TNAME " Error at lio_listio() %d: %s\n", errno, strerror(errno));
 		for (i=0; i<NUM_AIOCBS; i++)

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +2 -5

@@ -178,11 +169,8 @@ main ()
 
 	/* Suspend on selected request */
 	ret = aio_suspend((const struct aiocb **)plist, 2, NULL);
-
-	/* Check selected request has completed */
-	if (!received_selected) {
-		printf (TNAME " Error : AIOCB %d should have completed after suspend\n",
-			WAIT_FOR_AIOCB);
+	if (ret) {
+		printf (TNAME " Error at aio_suspend() %d: %s\n", errno, strerror (errno));
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

@@ -191,9 +179,8 @@ main ()
 		exit (PTS_FAIL);

aio_suspend may be interrupted by IO request's completion signal, so we should use aio_error instead signal hander to check whether this request has complete.

Signed-off-by Bian Naimeng <[email protected]>

---
 .../conformance/interfaces/aio_suspend/1-1.c       |   51 +++++++-------------
 1 files changed, 17 insertions(+), 34 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +3 -22

index 7d33e62..8c85ca5 100644
@@ -44,19 +44,11 @@
 #define BUF_SIZE	1024*1024
 #define WAIT_FOR_AIOCB	6
 
-int received_selected	= 0;
 int received_all	= 0;
 
 void
 sigrt1_handler(int signum, siginfo_t *info, void *context)
 {
-	if (info->si_value.sival_int == WAIT_FOR_AIOCB)
-		received_selected = 1;
-}
-
-void
-sigrt2_handler(int signum, siginfo_t *info, void *context)
-{
 	received_all = 1;
 }
 

@@ -123,30 +115,19 @@ main ()
 		aiocbs[i]->aio_buf = &bufs[i*BUF_SIZE];
 		aiocbs[i]->aio_nbytes = BUF_SIZE;
 		aiocbs[i]->aio_lio_opcode = LIO_READ;
-
-		/* Use SIRTMIN+1 for individual completions */
-		aiocbs[i]->aio_sigevent.sigev_notify = SIGEV_SIGNAL;
-		aiocbs[i]->aio_sigevent.sigev_signo = SIGRTMIN+1;
-		aiocbs[i]->aio_sigevent.sigev_value.sival_int = i;
 	}
 
-	/* Use SIGRTMIN+2 for list completion */
+	/* Use SIGRTMIN+1 for list completion */
 	event.sigev_notify = SIGEV_SIGNAL;
-	event.sigev_signo = SIGRTMIN+2;
+	event.sigev_signo = SIGRTMIN+1;
 	event.sigev_value.sival_ptr = NULL;
 
-	/* Setup handler for individual operation completion */
+	/* Setup handler for list completion */
 	action.sa_sigaction = sigrt1_handler;
 	sigemptyset(&action.sa_mask);
 	action.sa_flags = SA_SIGINFO|SA_RESTART;
 	sigaction(SIGRTMIN+1, &action, NULL);
 
-	/* Setup handler for list completion */
-	action.sa_sigaction = sigrt2_handler;
-	sigemptyset(&action.sa_mask);
-	action.sa_flags = SA_SIGINFO|SA_RESTART;
-	sigaction(SIGRTMIN+2, &action, NULL);
-
 	/* Setup suspend list */
 	plist[0] = NULL;
 	plist[1] = aiocbs[WAIT_FOR_AIOCB];

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +4 -3

 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

@@ -165,9 +147,10 @@ main ()
 	}
 
 	/* Check selected request has not completed yet */
-	if (received_selected) {
-		printf (TNAME " Error : AIOCB %d already completed before suspend\n",
-			WAIT_FOR_AIOCB);
+	err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
+	if (err != EINPROGRESS) {
+		printf (TNAME " Error : AIOCB %d should not have completed "
+			"before suspend\n", WAIT_FOR_AIOCB);
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

		exit(PTS_FAIL);
}

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +3 -5

-
-	/* Check selected request has completed */
-	if (!received_selected) {
-		printf (TNAME " Error : AIOCB %d should have completed after suspend\n",
-			WAIT_FOR_AIOCB);
+	if (ret) {
+		printf (TNAME " Error at aio_suspend() %d: %s\n",
+			errno, strerror (errno));
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

Quoted 5 lines

Quoted 4 lines

      if (ret) {
-             printf(TNAME " Error at lio_listio() %d: %s\n", errno, strerror(errno));
+             printf (TNAME " Error at lio_listio() %d: %s\n",
+                     errno, strerror(errno));

Quoted 3 lines

According to lkml coding style (that we are trying to use here), space should be only between C keywords and (). Not betweeen library calls like printf and so.

This is true for LTP code, but this is third party code. The documentation claims to conform to the LKML coding style though (even though I know it isn't true).

Quoted 26 lines

Quoted 17 lines

              for (i=0; i<NUM_AIOCBS; i++)
                      free (aiocbs[i]);
              free (bufs);

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +4 -3

@@ -165,9 +147,10 @@ main ()
       }
 
       /* Check selected request has not completed yet */
-     if (received_selected) {
-             printf (TNAME " Error : AIOCB %d already completed before suspend\n",
-                     WAIT_FOR_AIOCB);
+     err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
+     if (err != EINPROGRESS) {
+             printf (TNAME " Error : AIOCB %d should not have completed "
+                     "before suspend\n", WAIT_FOR_AIOCB);
               for (i=0; i<NUM_AIOCBS; i++)
                       free (aiocbs[i]);
               free (bufs);

....

Quoted 2 lines

              exit(PTS_FAIL);
}

Another problem lies here. The test relies that aio operation in not finished when we got here. And as some linux kernels fallbacks aio as synchronous IO, when we got there, it's allready finished. So I think we should change this PTS_FAIL to PTS_UNRESOLVED.

Quoted 1 line

Garrett what you think?

If an operating system doesn't fully implement a set of requirements, it shouldn't claim to do so. If it does then that's a bug in the implementation.

Quoted 6 lines

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c main () · line 163

Quoted 4 lines

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +0 -0

@@ -178,11 +161,9 @@ main ()
 
       /* Suspend on selected request */
       ret = aio_suspend((const struct aiocb **)plist, 2, NULL);

Please remove useless cast to (const struct aiocb**)

Depends on whether or not this is correct. According to GCC it matters:

../../../conformance/interfaces/aio_suspend/1-1.c:180: warning:
passing argument 1 of 'aio_suspend' from incompatible pointer type

This should probably have the restrict c99 keyword attached to it though.

Quoted 29 lines

Quoted 12 lines

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +3 -5

-
-     /* Check selected request has completed */
-     if (!received_selected) {
-             printf (TNAME " Error : AIOCB %d should have completed after suspend\n",
-                     WAIT_FOR_AIOCB);
+     if (ret) {
+             printf (TNAME " Error at aio_suspend() %d: %s\n",
+                     errno, strerror (errno));
               for (i=0; i<NUM_AIOCBS; i++)
                       free (aiocbs[i]);
               free (bufs);

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c main () · line 173

Quoted 2 lines

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +0 -0

@@ -191,9 +172,11 @@ main ()
               exit (PTS_FAIL);
       }

Here again with the printf.

Quoted 11 lines

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/1-1.c +5 -3

-
-     if (ret) {
-             printf (TNAME " Error at aio_suspend() %d: %s\n", errno, strerror (errno));
+     /* Check selected request has completed */
+     err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
+     if (err) {
+             printf (TNAME " Error : AIOCB %d should have completed after "
+                     "suspend\n", WAIT_FOR_AIOCB);
               for (i=0; i<NUM_AIOCBS; i++)
                       free (aiocbs[i]);
               free (bufs);

Quoted 1 line

And here.

I've cleaned up the style in this test to be more logical. Unfortunately the bull.net contributed tests weren't cleaned up on import to posix_testsuite, just like some of the syscall testcases in LTP proper.

aio_suspend may be interrupted by IO request's completion signal, so we should use aio_error instead signal hander to check whether this request has complete.

Signed-off-by Bian Naimeng <[email protected]>

---
 .../conformance/interfaces/aio_suspend/4-1.c       |   65 +++++++-------------
 1 files changed, 23 insertions(+), 42 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +13 -9

index 260c26a..6699ab4 100644
@@ -41,18 +41,24 @@
 #define BUF_SIZE	1024*1024
 #define WAIT_FOR_AIOCB	6
 
-int received_selected	= 0;
 int received_all	= 0;
 
-void
-sigrt1_handler(int signum, siginfo_t *info, void *context)
+int
+selected_request_status(struct aiocb *paiocb, int expect)
 {
-	if (info->si_value.sival_int == WAIT_FOR_AIOCB)
-		received_selected = 1;
+	int err = aio_error(paiocb);
+
+	if (err != expect) {
+		printf (TNAME " Selected request expect %d[%s], not %d[%s]\n",
+			expect, strerror(expect), err, strerror(err));
+		return 0;
+	}
+
+	return 1;
 }
 
 void
-sigrt2_handler(int signum, siginfo_t *info, void *context)
+sigrt1_handler(int signum, siginfo_t *info, void *context)
 {
 	received_all = 1;
 }

@@ -68,7 +74,7 @@ main ()
 	char *bufs;
 	struct sigaction action;
 	struct sigevent event;
-	struct timespec ts = {0, 10000000}; /* 10 ms */
+	struct timespec ts = {0, 1000}; /* 1 us */
 	int errors = 0;
 	int ret;
 	int err;

@@ -121,39 +127,28 @@ main ()
 		aiocbs[i]->aio_buf = &bufs[i*BUF_SIZE];
 		aiocbs[i]->aio_nbytes = BUF_SIZE;
 		aiocbs[i]->aio_lio_opcode = LIO_READ;
-
-		/* Use SIRTMIN+1 for individual completions */

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +1 -4

-	/* Setup handler for individual operation completion */
+	/* Setup handler for list completion */
 	action.sa_sigaction = sigrt1_handler;
 	sigemptyset(&action.sa_mask);
 	action.sa_flags = SA_SIGINFO|SA_RESTART;
 	sigaction(SIGRTMIN+1, &action, NULL);
 
-	/* Setup handler for list completion */
-	action.sa_sigaction = sigrt2_handler;
-	sigemptyset(&action.sa_mask);

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +2 -1

 	if (ret) {
-		printf(TNAME " Error at lio_listio() %d: %s\n", errno, strerror(errno));
+		printf (TNAME " Error at lio_listio() %d: %s\n",
+			errno, strerror(errno));
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +3 -14

-	/* Check selected request has not completed */
-	if (received_selected) {
-		printf (TNAME " Error : AIOCB %d should not have completed after timed out suspend\n",
-			WAIT_FOR_AIOCB);
-		for (i=0; i<NUM_AIOCBS; i++)
-			free (aiocbs[i]);
-		free (bufs);
-		free (aiocbs);
-		close (fd);
-		exit (PTS_FAIL);
-	}
-
 	/* timed out aio_suspend should return -1 and set errno to EAGAIN */
-	if (ret != -1) {
-		printf (TNAME " aio_suspend() should return -1\n");
+	if (ret != -1 || errno != EAGAIN) {
+		printf (TNAME " aio_suspend() should return -1 and set errno "
+			"to EAGAIN: %d, %s\n", ret, strerror(errno));
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

@@ -200,9 +182,8 @@ main ()
 		exit (PTS_FAIL);

aio_suspend may be interrupted by IO request's completion signal, so we should use aio_error instead signal hander to check whether this request has complete.

Signed-off-by Bian Naimeng <[email protected]>

---
 .../conformance/interfaces/aio_suspend/4-1.c       |   62 ++++++--------------
 1 files changed, 18 insertions(+), 44 deletions(-)

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +4 -23

index 260c26a..bb90234 100644
@@ -41,19 +41,11 @@
 #define BUF_SIZE	1024*1024
 #define WAIT_FOR_AIOCB	6
 
-int received_selected	= 0;
 int received_all	= 0;
 
 void
 sigrt1_handler(int signum, siginfo_t *info, void *context)
 {
-	if (info->si_value.sival_int == WAIT_FOR_AIOCB)
-		received_selected = 1;
-}
-
-void
-sigrt2_handler(int signum, siginfo_t *info, void *context)
-{
 	received_all = 1;
 }
 

@@ -68,7 +60,7 @@ main ()
 	char *bufs;
 	struct sigaction action;
 	struct sigevent event;
-	struct timespec ts = {0, 10000000}; /* 10 ms */
+	struct timespec ts = {0, 1000}; /* 1 us */
 	int errors = 0;
 	int ret;
 	int err;

@@ -121,30 +113,19 @@ main ()
 		aiocbs[i]->aio_buf = &bufs[i*BUF_SIZE];
 		aiocbs[i]->aio_nbytes = BUF_SIZE;
 		aiocbs[i]->aio_lio_opcode = LIO_READ;
-
-		/* Use SIRTMIN+1 for individual completions */
-		aiocbs[i]->aio_sigevent.sigev_notify = SIGEV_SIGNAL;
-		aiocbs[i]->aio_sigevent.sigev_signo = SIGRTMIN+1;
-		aiocbs[i]->aio_sigevent.sigev_value.sival_int = i;
 	}
 
-	/* Use SIGRTMIN+2 for list completion */
+	/* Use SIGRTMIN+1 for list completion */
 	event.sigev_notify = SIGEV_SIGNAL;
-	event.sigev_signo = SIGRTMIN+2;
+	event.sigev_signo = SIGRTMIN+1;
 	event.sigev_value.sival_ptr = NULL;
 
-	/* Setup handler for individual operation completion */
+	/* Setup handler for list completion */
 	action.sa_sigaction = sigrt1_handler;
 	sigemptyset(&action.sa_mask);
 	action.sa_flags = SA_SIGINFO|SA_RESTART;
 	sigaction(SIGRTMIN+1, &action, NULL);
 
-	/* Setup handler for list completion */
-	action.sa_sigaction = sigrt2_handler;
-	sigemptyset(&action.sa_mask);
-	action.sa_flags = SA_SIGINFO|SA_RESTART;
-	sigaction(SIGRTMIN+2, &action, NULL);
-
 	/* Setup suspend list */
 	plist[0] = NULL;
 	plist[1] = aiocbs[WAIT_FOR_AIOCB];

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +4 -3

@@ -163,9 +145,10 @@ main ()
 	}
 
 	/* Check selected request has not completed yet */
-	if (received_selected) {
-		printf (TNAME " Error : AIOCB %d already completed before suspend\n",
-			WAIT_FOR_AIOCB);
+	err = aio_error(aiocbs[WAIT_FOR_AIOCB]);
+	if (err !=  EINPROGRESS) {
+		printf (TNAME " Error : AIOCB %d should not have completed "
+			"before suspend\n", WAIT_FOR_AIOCB);
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

		exit(PTS_FAIL);
	}

testcases/open_posix_testsuite/conformance/interfaces/aio_suspend/4-1.c +3 -14

-	/* Check selected request has not completed */
-	if (received_selected) {
-		printf (TNAME " Error : AIOCB %d should not have completed after timed out suspend\n",
-			WAIT_FOR_AIOCB);
-		for (i=0; i<NUM_AIOCBS; i++)
-			free (aiocbs[i]);
-		free (bufs);
-		free (aiocbs);
-		close (fd);
-		exit (PTS_FAIL);
-	}
-
 	/* timed out aio_suspend should return -1 and set errno to EAGAIN */
-	if (ret != -1) {
-		printf (TNAME " aio_suspend() should return -1\n");
+	if (ret != -1 || errno != EAGAIN) {
+		printf (TNAME " aio_suspend() should return -1 and set errno "
+			"to EAGAIN: %d, %s\n", ret, strerror(errno));
 		for (i=0; i<NUM_AIOCBS; i++)
 			free (aiocbs[i]);
 		free (bufs);

aio_suspend may be interrupted by IO request's completion signal, so we should use aio_error instead signal hander to check whether this request has complete.