Hi ! This patch fixes a concurrency issue in semctl01. This test was using usleep to synchronize tasks. On a heavily loaded system, this synchronization is not enough to ensure execution correctness.
This patch is a bit more intrusive than my previous ones.
Patch content :
* Define 2 new functions in kernel/syscalls/lib/libtestsuite.c, used
to synchronize a father and a son using pipes.
- create_sync_pipes: create a pair of pipes used for the
synchronization.
- wait_son_startup: function used in the father to wait for
its son to start ts execution.
- notify_startup: function used in the son to notify it has
started its execution.
* Add a kernel/syscalls/lib/libtestsuite.h file to cleanly export
newly defines functions.
* Fix the semctl01test. The idea used to synchronize :
- For each task created, the father waits for the son to start
its execution using the newly define functions.
- After the last son has been created, the father do a sleep(1)
to give time to the sons to execute the semop function.
The final sleep does not guaranty the sons will have time to do the
semop. On a REALLY heavily loaded system, this will still fail...
The only solution I see to be sure the son is really blocked on the
semop before the father continue its execution it to use the wchan
info from /proc/<pid>/wchan file...
Any other idea ?
Or, do we simply consider the will not run this test on a very
loaded machine ?
Regards.
R.
--
Renaud Lottiaux
Kerlabs
Bâtiment Germanium
80, avenue des buttes de Coësmes
35700 Rennes - France
Phone : (+33|0)6 80 89 19 34
Fax : (+33|0)2 99 84 71 71
Email : [EMAIL PROTECTED]
Web : http://www.kerlabs.com/
Index: cvs/testcases/kernel/syscalls/ipc/semctl/semctl01.c
===================================================================
--- cvs.orig/testcases/kernel/syscalls/ipc/semctl/semctl01.c 2008-03-11 14:49:31.000000000 +0100
+++ cvs/testcases/kernel/syscalls/ipc/semctl/semctl01.c 2008-03-11 16:21:53.000000000 +0100
@@ -53,11 +53,17 @@
* HISTORY
* 03/2001 - Written by Wayne Boyer
*
+ * 11/03/2008 Renaud Lottiaux ([EMAIL PROTECTED])
+ * - Fix concurrency issue. Due to the use of usleep function to
+ * synchronize processes, synchronization issues can occur on a loaded
+ * system. Fix this by using pipes to synchronize processes.
+ *
* RESTRICTIONS
* none
*/
#include "ipcsem.h"
+#include "libtestsuite.h"
char *TCID = "semctl01";
int TST_TOTAL = 10;
@@ -326,6 +332,7 @@
cnt_setup(int opval)
{
int pid, i;
+ int sync_pipes[2];
sops.sem_num = SEM4;
sops.sem_flg = 0;
@@ -344,12 +351,19 @@
sops.sem_op = opval; /* set the correct operation */
for (i=0; i<NCHILD; i++) {
+ if (create_sync_pipes(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "cannot create sync pipes");
+ }
+
/* fork five children to wait */
if ((pid = FORK_OR_VFORK()) == -1) {
tst_brkm(TBROK, cleanup, "fork failed in cnt_setup");
}
if (pid == 0) { /* child */
+ if (notify_startup(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "notify_startup failed");
+ }
#ifdef UCLINUX
if (self_exec(argv0, "ndd", 2, sem_id_1,
sops.sem_op) < 0) {
@@ -360,13 +374,19 @@
child_cnt();
#endif
} else { /* parent */
- /* take a quick nap so that commands execute orderly */
- usleep(50000);
+ if (wait_son_startup(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "wait_son_startup failed");
+ }
/* save the pid so we can kill it later */
pid_arr[i] = pid;
}
}
+ /* After last son has been created, give it a chance to execute the
+ * semop command before we continue. Without this sleep, on SMP machine
+ * the father semctl could be executed before the son semop.
+ */
+ sleep(1);
}
void
@@ -409,6 +429,11 @@
pid_setup()
{
int pid;
+ int sync_pipes[2];
+
+ if (create_sync_pipes(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "cannot create sync pipes");
+ }
/*
* Fork a child to do a semop that will pass.
@@ -418,6 +443,9 @@
}
if (pid == 0) { /* child */
+ if (notify_startup(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "notify_startup failed");
+ }
#ifdef UCLINUX
if (self_exec(argv0, "nd", 1, sem_id_1) < 0) {
tst_brkm(TBROK, cleanup, "self_exec failed "
@@ -427,9 +455,10 @@
child_pid();
#endif
} else { /* parent */
- /* take a quick nap so that commands execute orderly */
- usleep(50000);
-
+ if (wait_son_startup(sync_pipes) == -1) {
+ tst_brkm(TBROK, cleanup, "wait_son_startup failed");
+ }
+ sleep(1);
pid_arr[SEM2] = pid;
}
}
Index: cvs/testcases/kernel/syscalls/ipc/semctl/Makefile
===================================================================
--- cvs.orig/testcases/kernel/syscalls/ipc/semctl/Makefile 2008-03-11 15:50:46.000000000 +0100
+++ cvs/testcases/kernel/syscalls/ipc/semctl/Makefile 2008-03-11 16:21:22.000000000 +0100
@@ -16,8 +16,8 @@
# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
#
-CFLAGS += -I../lib -I../../../../../include -Wall
-LDLIBS += -L../../../../../lib -lltp -L.. -lipc
+CFLAGS += -I../lib -I../../lib -I../../../../../include -Wall
+LDLIBS += -L../../../../../lib -lltp -L.. -lipc -L../.. -ltestsuite
SRCS = $(wildcard *.c)
TARGETS = $(patsubst %.c,%,$(SRCS))
Index: cvs/testcases/kernel/syscalls/lib/libtestsuite.c
===================================================================
--- cvs.orig/testcases/kernel/syscalls/lib/libtestsuite.c 2008-03-11 15:58:29.000000000 +0100
+++ cvs/testcases/kernel/syscalls/lib/libtestsuite.c 2008-03-11 16:17:08.000000000 +0100
@@ -28,6 +28,10 @@
*
* my_getpwnam(), do_file_setup()
*
+ * HISTORY
+ * 11/03/2008 Renaud Lottiaux ([EMAIL PROTECTED])
+ * - Add the following functions to synchronise father and sons processes
+ * create_sync_pipes(), wait_son_startup(), notify_startup()
*/
#include <unistd.h>
#include <stdio.h>
@@ -71,3 +75,36 @@
"%s", fname, errno, strerror(errno));
}
}
+
+int create_sync_pipes(int fd[])
+{
+ return pipe (fd);
+}
+
+int wait_son_startup (int fd[])
+{
+ char buf;
+ int r;
+
+ r = read (fd[0], &buf, 1);
+ close (fd[0]);
+ close (fd[1]);
+
+ if ((r != 1) || (buf != 'A'))
+ return -1;
+ return 0;
+}
+
+int notify_startup (int fd[])
+{
+ char buf = 'A';
+ int r;
+
+ r = write (fd[1], &buf, 1);
+ close (fd[0]);
+ close (fd[1]);
+
+ if (r != 1)
+ return -1;
+ return 0;
+}
Index: cvs/testcases/kernel/syscalls/lib/libtestsuite.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ cvs/testcases/kernel/syscalls/lib/libtestsuite.h 2008-03-11 16:15:33.000000000 +0100
@@ -0,0 +1,12 @@
+/* The following functions are used to synchronize father and sons processes.
+ *
+ * create_sync_pipes: create pipes used for the synchronization. Must be done
+ * by father process before a fork.
+ *
+ * wait_son_startup: wait a son process to reach the "notify_startup" function.
+ *
+ * notify_startup: notify the father process a son has started its execution.
+ */
+int create_sync_pipes(int fd[]);
+int wait_son_startup (int fd[]);
+int notify_startup (int fd[]);
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
