* [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
@ 2011-12-15 9:35 Jan Stancek
2011-12-15 10:15 ` Garrett Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2011-12-15 9:35 UTC (permalink / raw)
To: ltp-list
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
This test occasionally hangs on some machines. The hang has been
observed mostly on single CPU ones.
pipeio code is using signal(2), setting by default SA_RESTART
flag, which is also the case for SIGCHLD.
If last child manages to exit while parent is still at open(),
parent gets SIGCHLD and open() is restarted. At this point test
hangs.
Here's strace output from parent point of view:
=== snip ===
brk(0) = 0x11bb000
brk(0x11dd000) = 0x11dd000
getpid() = 18826
stat("tpipe.18826", 0x7fff89e1d410) = -1 ENOENT (No such file or
directory)
mknod("tpipe.18826", S_IFIFO|0777) = 0
rt_sigaction(SIGCHLD, {0x400a54, [CHLD], SA_RESTORER|SA_RESTART,
0x354aa32a20}, {SIG_DFL, [], 0}, 8) = 0
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|
CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f53b9ddd9d0) = 18827
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 3), ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
= 0x7f53b9de5000
open("tpipe.18826", O_RDONLY
) = ? ERESTARTSYS (To be restarted)
--- SIGCHLD (Child exited) @ 0 (0) ---
wait4(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 18827
rt_sigreturn(0xffffffffffffffff) = 2
open("tpipe.18826", O_RDONLY
=== /snip ===
This patch is introducing semaphore, which prevents children from
exiting until parent completes open(). It also adds timed wait,
so parent waits for children to exit before it deletes pipe and
semaphore.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/ipc/pipeio/pipeio.c | 45
+++++++++++++++++++++++++++++++---
1 files changed, 41 insertions(+), 4 deletions(-)
[-- Attachment #2: 0002-pipeio-prevent-race-between-SIGCHLD-and-open.patch --]
[-- Type: text/x-patch, Size: 3174 bytes --]
diff --git a/testcases/kernel/ipc/pipeio/pipeio.c b/testcases/kernel/ipc/pipeio/pipeio.c
index 22ab3a7..dd52316 100644
--- a/testcases/kernel/ipc/pipeio/pipeio.c
+++ b/testcases/kernel/ipc/pipeio/pipeio.c
@@ -158,6 +158,8 @@ char *av[];
struct semid_ds *buf;
unsigned short int *array;
} u;
+ unsigned int uwait_iter = 1000;
+ unsigned int uwait_total = 5000000;
u.val = 0;
format = HEX;
@@ -443,12 +445,16 @@ char *av[];
writebuf[size-1] = 'A'; /* to detect partial read/write problem */
- if ((sem_id = semget(IPC_PRIVATE, 1, IPC_CREAT|S_IRWXU)) == -1) {
+ if ((sem_id = semget(IPC_PRIVATE, 2, IPC_CREAT|S_IRWXU)) == -1) {
tst_brkm(TBROK|TERRNO, NULL, "Couldn't allocate semaphore");
}
if (semctl(sem_id, 0, SETVAL, u) == -1)
- tst_brkm(TBROK|TERRNO, NULL, "Couldn't initialize semaphore value");
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't initialize semaphore 0 value");
+
+ /* semaphore to hold off children from exiting until open() completes */
+ if (semctl(sem_id, 1, SETVAL, u) == -1)
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't initialize semaphore 1 value");
if (background) {
if ((n=fork()) == -1) {
@@ -539,7 +545,7 @@ printf("child after fork pid = %d\n", getpid());
};
if (semop(sem_id, &sem_op, 1) == -1)
- tst_brkm(TBROK|TERRNO, NULL, "Couldn't raise the semaphore");
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't raise the semaphore 0");
pid_word = (int *)&writebuf[0];
count_word = (int *)&writebuf[NBPW];
@@ -586,6 +592,15 @@ printf("child after fork pid = %d\n", getpid());
}
fflush(stderr);
}
+
+ /* child waits until parent completes open() */
+ sem_op = (struct sembuf) {
+ .sem_num = 1,
+ .sem_op = -1,
+ .sem_flg = 0
+ };
+ if (semop(sem_id, &sem_op, 1) == -1)
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't lower the semaphore 1");
}
if (c > 0) { /***** if parent *****/
@@ -602,6 +617,15 @@ printf("child after fork pid = %d\n", getpid());
close(write_fd);
}
+ /* raise semaphore so children can exit */
+ sem_op = (struct sembuf) {
+ .sem_num = 1,
+ .sem_op = num_wrters,
+ .sem_flg = 0
+ };
+ if (semop(sem_id, &sem_op, 1) == -1)
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't raise the semaphore 1");
+
sem_op = (struct sembuf) {
.sem_num = 0,
.sem_op = -num_wrters,
@@ -612,7 +636,7 @@ printf("child after fork pid = %d\n", getpid());
if (errno == EINTR) {
continue;
}
- tst_brkm(TBROK|TERRNO, NULL, "Couldn't wait on semaphore");
+ tst_brkm(TBROK|TERRNO, NULL, "Couldn't wait on semaphore 0");
}
for (i=num_wrters*num_writes; i > 0 || loop; --i) {
@@ -694,6 +718,19 @@ output:
tst_resm(TPASS, "1 PASS %d pipe reads complete, read size = %d, %s %s",
count+1,size,pipe_type,blk_type);
+ /* wait for all children to finish, timeout after uwait_total
+ semtimedop might not be available everywhere */
+ for (i=0; i<uwait_total; i+=uwait_iter) {
+ if (semctl(sem_id, 1, GETVAL) == 0) {
+ break;
+ }
+ usleep(uwait_iter);
+ }
+
+ if (i > uwait_total) {
+ tst_resm(TWARN, "Timed out waiting for child processes to exit");
+ }
+
semctl(sem_id, 0, IPC_RMID);
if (!unpipe)
[-- Attachment #3: Type: text/plain, Size: 339 bytes --]
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.
But none more important than the need to reduce IT complexity
while improving strategic productivity. Learn More!
http://www.accelacomm.com/jaw/sdnl/114/51507609/
[-- Attachment #4: Type: text/plain, Size: 155 bytes --]
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
2011-12-15 9:35 [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open() Jan Stancek
@ 2011-12-15 10:15 ` Garrett Cooper
2011-12-15 10:36 ` Jan Stancek
0 siblings, 1 reply; 6+ messages in thread
From: Garrett Cooper @ 2011-12-15 10:15 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp-list
On Thu, Dec 15, 2011 at 1:35 AM, Jan Stancek <jstancek@redhat.com> wrote:
>
> This test occasionally hangs on some machines. The hang has been
> observed mostly on single CPU ones.
>
> pipeio code is using signal(2), setting by default SA_RESTART
> flag, which is also the case for SIGCHLD.
>
> If last child manages to exit while parent is still at open(),
> parent gets SIGCHLD and open() is restarted. At this point test
> hangs.
>
> Here's strace output from parent point of view:
Committed -- thanks!
-Garrett
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.
But none more important than the need to reduce IT complexity
while improving strategic productivity. Learn More!
http://www.accelacomm.com/jaw/sdnl/114/51507609/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
2011-12-15 10:15 ` Garrett Cooper
@ 2011-12-15 10:36 ` Jan Stancek
2011-12-15 11:10 ` Garrett Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2011-12-15 10:36 UTC (permalink / raw)
To: Garrett Cooper; +Cc: ltp-list
Garrett,
I'm not sure what happened, but I see only first patch from series
committed with description belonging to second one.
Regards,
Jan
----- Original Message -----
> From: "Garrett Cooper" <yanegomi@gmail.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp-list@lists.sourceforge.net
> Sent: Thursday, December 15, 2011 11:15:34 AM
> Subject: Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
>
> On Thu, Dec 15, 2011 at 1:35 AM, Jan Stancek <jstancek@redhat.com>
> wrote:
> >
> > This test occasionally hangs on some machines. The hang has been
> > observed mostly on single CPU ones.
> >
> > pipeio code is using signal(2), setting by default SA_RESTART
> > flag, which is also the case for SIGCHLD.
> >
> > If last child manages to exit while parent is still at open(),
> > parent gets SIGCHLD and open() is restarted. At this point test
> > hangs.
> >
> > Here's strace output from parent point of view:
>
> Committed -- thanks!
> -Garrett
>
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.
But none more important than the need to reduce IT complexity
while improving strategic productivity. Learn More!
http://www.accelacomm.com/jaw/sdnl/114/51507609/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
2011-12-15 10:36 ` Jan Stancek
@ 2011-12-15 11:10 ` Garrett Cooper
2011-12-15 12:05 ` Jan Stancek
0 siblings, 1 reply; 6+ messages in thread
From: Garrett Cooper @ 2011-12-15 11:10 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp-list
On Thu, Dec 15, 2011 at 2:36 AM, Jan Stancek <jstancek@redhat.com> wrote:
> Garrett,
>
> I'm not sure what happened, but I see only first patch from series
> committed with description belonging to second one.
>
> Regards,
> Jan
I got confused by the multiple patches. Is this thread the one I
need to look at to rectify the other issues identified?
Thanks,
-Garrett
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.
But none more important than the need to reduce IT complexity
while improving strategic productivity. Learn More!
http://www.accelacomm.com/jaw/sdnl/114/51507609/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
2011-12-15 11:10 ` Garrett Cooper
@ 2011-12-15 12:05 ` Jan Stancek
2011-12-21 12:51 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2011-12-15 12:05 UTC (permalink / raw)
To: Garrett Cooper; +Cc: ltp-list
----- Original Message -----
> From: "Garrett Cooper" <yanegomi@gmail.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp-list@lists.sourceforge.net
> Sent: Thursday, December 15, 2011 12:10:17 PM
> Subject: Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
>
> On Thu, Dec 15, 2011 at 2:36 AM, Jan Stancek <jstancek@redhat.com>
> wrote:
> > Garrett,
> >
> > I'm not sure what happened, but I see only first patch from series
> > committed with description belonging to second one.
> >
> > Regards,
> > Jan
>
> I got confused by the multiple patches. Is this thread the one I
> need to look at to rectify the other issues identified?
Yes, this is the one.
I posted single patch yesterday:
[LTP] [PATCH] pipeio: prevent race between SIGCHLD and open()
You requested to use TERRNO instead of strerror(errno), so
while I was at it, I made this change for all existing code in pipeio.c:
[LTP] [PATCH v2 1/2] pipeio: convert strerror(errno) to TERRNO
and then put my original patch (with minor changes) on top of that:
[LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
Regards,
Jan
> Thanks,
> -Garrett
>
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.
But none more important than the need to reduce IT complexity
while improving strategic productivity. Learn More!
http://www.accelacomm.com/jaw/sdnl/114/51507609/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
2011-12-15 12:05 ` Jan Stancek
@ 2011-12-21 12:51 ` Cyril Hrubis
0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2011-12-21 12:51 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp-list
Hi!
> I posted single patch yesterday:
> [LTP] [PATCH] pipeio: prevent race between SIGCHLD and open()
>
> You requested to use TERRNO instead of strerror(errno), so
> while I was at it, I made this change for all existing code in pipeio.c:
> [LTP] [PATCH v2 1/2] pipeio: convert strerror(errno) to TERRNO
>
> and then put my original patch (with minor changes) on top of that:
> [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open()
Should be fixed in latest commit, thanks!
--
Cyril Hrubis
chrubis@suse.cz
------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create
new or port existing apps to sell to consumers worldwide. Explore the
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-21 12:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-15 9:35 [LTP] [PATCH v2 2/2] pipeio: prevent race between SIGCHLD and open() Jan Stancek
2011-12-15 10:15 ` Garrett Cooper
2011-12-15 10:36 ` Jan Stancek
2011-12-15 11:10 ` Garrett Cooper
2011-12-15 12:05 ` Jan Stancek
2011-12-21 12:51 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox