[LTP] [PATCH v2 0/7] Cleanup sched/process.c

[LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Petr Vorel]

Hi,

changes v1->v2:
* split 1 commit into 2, use freopen() as suggested by Li
* 4 more commits for further cleanup

Petr Vorel (7):
  sched/process.c: Replace errfp with stderr
  sched/process.c: Open debugfp with freopen()
  ci/alpine: Enable process.c
  sched/process.c: Log into cwd, add *.log suffix
  sched/.gitignore: Ignore logs
  sched/process.c: Remove non-Linux code
  sched/process.c: Remove useless TRUE FALSE definitions

 ci/alpine.sh                                  |   1 -
 .../kernel/sched/process_stress/.gitignore    |   1 +
 .../kernel/sched/process_stress/Makefile      |   2 -
 .../kernel/sched/process_stress/process.c     | 118 ++++--------------
 4 files changed, 23 insertions(+), 99 deletions(-)

-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/7] sched/process.c: Replace errfp with stderr

[Petr Vorel]

From: Petr Vorel <petr.vorel@gmail.com>

As comment suggest it's indeed not needed :).

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 .../kernel/sched/process_stress/process.c     | 22 +++++++++----------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/sched/process_stress/process.c b/testcases/kernel/sched/process_stress/process.c
index a5ff80987..2dd501f2e 100644
--- a/testcases/kernel/sched/process_stress/process.c
+++ b/testcases/kernel/sched/process_stress/process.c
@@ -142,10 +142,8 @@ timer_t timer;			/* timer structure */
 Pinfo *shmaddr;			/* Start address  of shared memory */
 
 #ifndef _LINUX
-FILE *errfp = stderr;		/* error file pointer, probably not necessary */
 FILE *debugfp = stderr;		/* debug file pointer, used if AUSDEBUG set */
 #else
-#define errfp stderr
 #define debugfp stderr
 #endif
 
@@ -201,12 +199,12 @@ void print_shm(void)
 		return;
 
 	for (pinfo = shmaddr, i = 0; i < nodesum; i++, pinfo++) {
-		fprintf(errfp,
+		fprintf(stderr,
 			"slot: %-4d pid: %-6d ppid: %-6d msg: %-2d err: %-2d lst:",
 			i, pinfo->pid, pinfo->ppid, pinfo->msg, pinfo->err);
 		for (j = 0, listp = pinfo->list; j < BVAL; j++, listp++)
-			fprintf(errfp, " %d", *listp);
-		fprintf(errfp, "\n");
+			fprintf(stderr, " %d", *listp);
+		fprintf(stderr, "\n");
 	}
 }
 
@@ -276,13 +274,13 @@ void rm_msgqueue(void)
 
 	/* remove message queue id. */
 	if (msgctl(msgid, IPC_RMID, NULL) && errno != EINVAL) {
-		fprintf(errfp, "msgctl failed msgid: errno %d\n", errno);
+		fprintf(stderr, "msgctl failed msgid: errno %d\n", errno);
 		perror("msgctl failed");
 	}
 
 	/* remove message queue id. */
 	if (msgctl(msgerr, IPC_RMID, NULL) && errno != EINVAL) {
-		fprintf(errfp, "msgctl failed msgerr: errno %d\n", errno);
+		fprintf(stderr, "msgctl failed msgerr: errno %d\n", errno);
 		perror("msgctl failed");
 	}
 }
@@ -297,7 +295,7 @@ void rm_shmseg(void)
 
 	/* remove shared memory id (and shared memory segment). */
 	if (shmctl(shmid, IPC_RMID, NULL) && errno != EINVAL) {
-		fprintf(errfp, "shmctl failed: errno %d\n", errno);
+		fprintf(stderr, "shmctl failed: errno %d\n", errno);
 		perror("shmctl failed");
 	}
 }
@@ -313,13 +311,13 @@ void rm_semseg(void)
 	/* remove sem_lock semaphore id */
 	semarg.val = 0;		/* to fix problem with 4th arg of semctl in 64 bits MARIOG */
 	if (semctl(sem_lock, 0, IPC_RMID, semarg.val) && errno != EINVAL) {
-		fprintf(errfp, "semctl failed: errno %d\n", errno);
+		fprintf(stderr, "semctl failed: errno %d\n", errno);
 		perror("semctl failed");
 	}
 	/* remove sem_count semaphore id. */
 	semarg.val = 0;		/* to fix problem with 4th arg of semctl in 64 bits MARIOG */
 	if (semctl(sem_count, 0, IPC_RMID, semarg.val) && errno != EINVAL) {
-		fprintf(errfp, "semctl failed: errno %d\n", errno);
+		fprintf(stderr, "semctl failed: errno %d\n", errno);
 		perror("semctl failed");
 	}
 }
@@ -1094,7 +1092,7 @@ void messenger(void)
 				case SIGALRM:
 					/* a process is hung, we will terminate */
 					killpg(procgrp, sig);
-					fprintf(errfp,
+					fprintf(stderr,
 						"ALERT! ALERT! WE HAVE TIMED OUT\n");
 					fprintf(stderr,
 						" SEVERE : SIGALRM: A process timed out, we failed\n");
@@ -1109,7 +1107,7 @@ void messenger(void)
 				default:
 					/* somebody sent a signal, we will terminate */
 					killpg(procgrp, sig);
-					fprintf(errfp,
+					fprintf(stderr,
 						"We received signal %d\n", sig);
 					fprintf(stderr,
 						" SEVERE : signal %d received, A proc was killed\n",
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/7] sched/process.c: Open debugfp with freopen()

[Petr Vorel]

From: Petr Vorel <petr.vorel@gmail.com>

i.e. use the recommended way.

This fixes compilation on MUSL which does not like assignment to stderr:

    process.c:551:14: error: assignment of read-only variable 'stderr'
      551 |      debugfp = fopen(foo, "a+");
          |              ^

Also drop debugfp definition. It'd be more obvious to use FILE pointer
for logging, but some debug functions are intended to use to log into
both stderr and into the log files (or at least logging is before stderr
is redirected into the log files), thus use stderr as file descriptor.

Although not sure why part of the code is logged into stdout and other
part into file, let's keep it.

Suggested-by: Li Wang <liwang@redhat.com>
Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 testcases/kernel/sched/process_stress/process.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/sched/process_stress/process.c b/testcases/kernel/sched/process_stress/process.c
index 2dd501f2e..777cdecd4 100644
--- a/testcases/kernel/sched/process_stress/process.c
+++ b/testcases/kernel/sched/process_stress/process.c
@@ -141,12 +141,6 @@ timer_t timer;			/* timer structure */
 
 Pinfo *shmaddr;			/* Start address  of shared memory */
 
-#ifndef _LINUX
-FILE *debugfp = stderr;		/* debug file pointer, used if AUSDEBUG set */
-#else
-#define debugfp stderr
-#endif
-
 struct envstruct *edat = envdata;	/* pointer to environment data */
 
 /* external function declarations */
@@ -260,7 +254,7 @@ void debugout(char *fmt, ...)
 
 	if (AUSDEBUG) {
 		va_start(args, fmt);
-		vfprintf(debugfp, fmt, args);
+		vfprintf(stderr, fmt, args);
 		va_end(args);
 	}
 }
@@ -546,7 +540,12 @@ int spawn(int val)
 			if (!pid) {	/* CHILD */
 				if (AUSDEBUG) {
 					sprintf(foo, "%sslot%d", SLOTDIR, tval);
-					debugfp = fopen(foo, "a+");
+
+					if ((freopen(foo, "w", stderr)) == NULL) {
+						fprintf(stderr, "freopen(%s, w, stderr) failed: %s (%d)\n",
+								foo, strerror(errno), errno);
+						exit(1);
+					}
 				}
 				pinfo = put_proc_info(tval);
 
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 3/7] ci/alpine: Enable process.c

[Petr Vorel]

From: Petr Vorel <petr.vorel@gmail.com>

Fixed in 2 previous commits.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 ci/alpine.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ci/alpine.sh b/ci/alpine.sh
index deb9cfdcf..d93a57616 100755
--- a/ci/alpine.sh
+++ b/ci/alpine.sh
@@ -33,7 +33,6 @@ cat /etc/os-release
 echo "WARNING: remove unsupported tests (until they're fixed)"
 cd $(dirname $0)/..
 rm -rfv \
-	testcases/kernel/sched/process_stress/process.c \
 	testcases/kernel/syscalls/confstr/confstr01.c \
 	testcases/kernel/syscalls/fmtmsg/fmtmsg01.c \
 	testcases/kernel/syscalls/getcontext/getcontext01.c \
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 4/7] sched/process.c: Log into cwd, add *.log suffix

[Petr Vorel]

process.c hanged if run with debugging (-D) and log directory (slot/)
wasn't created before.

As nothing depends on this path, log into cwd, for clarity add .log
suffix.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/sched/process_stress/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/testcases/kernel/sched/process_stress/process.c b/testcases/kernel/sched/process_stress/process.c
index 777cdecd4..1fd520eee 100644
--- a/testcases/kernel/sched/process_stress/process.c
+++ b/testcases/kernel/sched/process_stress/process.c
@@ -52,7 +52,6 @@
 #define TNDX 3
 #define MAXBVAL 70
 #define MAXDVAL 11
-#define SLOTDIR "./slot/"
 
 #ifdef _LINUX
 			/* LINUX #defnes */
@@ -539,7 +538,7 @@ int spawn(int val)
 			pid = fork();
 			if (!pid) {	/* CHILD */
 				if (AUSDEBUG) {
-					sprintf(foo, "%sslot%d", SLOTDIR, tval);
+					sprintf(foo, "slot%d.log", tval);
 
 					if ((freopen(foo, "w", stderr)) == NULL) {
 						fprintf(stderr, "freopen(%s, w, stderr) failed: %s (%d)\n",
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 5/7] sched/.gitignore: Ignore logs

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/sched/process_stress/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/kernel/sched/process_stress/.gitignore b/testcases/kernel/sched/process_stress/.gitignore
index b2d4946e6..d05d23b94 100644
--- a/testcases/kernel/sched/process_stress/.gitignore
+++ b/testcases/kernel/sched/process_stress/.gitignore
@@ -1 +1,2 @@
 /process
+/*.log
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 6/7] sched/process.c: Remove non-Linux code

[Petr Vorel]

as we don't care about other platforms.
Also remove -D_LINUX definition from Makefile.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/sched/process_stress/Makefile      |  2 -
 .../kernel/sched/process_stress/process.c     | 67 +------------------
 2 files changed, 2 insertions(+), 67 deletions(-)

diff --git a/testcases/kernel/sched/process_stress/Makefile b/testcases/kernel/sched/process_stress/Makefile
index 011017c65..48c825bd9 100644
--- a/testcases/kernel/sched/process_stress/Makefile
+++ b/testcases/kernel/sched/process_stress/Makefile
@@ -24,6 +24,4 @@ top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/env_pre.mk
 
-CPPFLAGS		+= -D_LINUX
-
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/sched/process_stress/process.c b/testcases/kernel/sched/process_stress/process.c
index 1fd520eee..b66908e7a 100644
--- a/testcases/kernel/sched/process_stress/process.c
+++ b/testcases/kernel/sched/process_stress/process.c
@@ -27,16 +27,9 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
-
-#ifndef _LINUX
-			/* LINUX INCLUDES */
-#include <sys/mode.h>
-#include <sys/timers.h>
-#else
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/ipc.h>
-#endif
 #include <sys/msg.h>
 #include <sys/resource.h>
 #include <sys/select.h>
@@ -53,17 +46,14 @@
 #define MAXBVAL 70
 #define MAXDVAL 11
 
-#ifdef _LINUX
-			/* LINUX #defnes */
 #ifndef TRUE
 #define TRUE 1
 #endif
 #ifndef FALSE
 #define FALSE 0
 #endif
-#endif
 
-#if defined _LINUX && defined DEBUG
+#ifdef DEBUG
 #define prtln()	printf("At line number: %d\n", __LINE__); \
 		fflush(NULL)
 #define dprt(fmt, args...) printf(fmt, ## args)
@@ -78,9 +68,7 @@
 #define    DVAL  (*edat[DNDX].eval.vint)	/* depth of process tree */
 #define    TVAL  (*edat[TNDX].eval.vint)	/* timer value */
 
-#ifdef _LINUX
 typedef long mtyp_t;
-#endif
 
 /* structure of information stored about each process in shared memory */
 typedef struct proc_info {
@@ -346,9 +334,6 @@ void nextofkin(int sig, int code, struct sigcontext *scp)
 		severe("msgsnd failed: %d msgid %d mtyp %d mtext %d\n",
 		       errno, msgerr, 3, mtext);
 	}
-#ifndef _LINUX
-	reltimerid(timer);
-#endif
 	exit(1);
 }
 
@@ -799,25 +784,14 @@ void set_signals(void *sighandler())
 
 	action.sa_handler = (void *)sighandler;
 
-#ifdef _LINUX
 	sigfillset(&action.sa_mask);
-#else
-	SIGINITSET(action.sa_mask);
-#endif
 	action.sa_flags = 0;
 
 	/* Set the signal handler up */
-#ifdef _LINUX
 	sigaddset(&action.sa_mask, SIGTERM);
-#else
-	SIGADDSET(action.sa_mask, SIGTERM);
-#endif
+
 	for (i = 0; siginfo[i].signum != -1; i++) {
-#ifdef _LINUX
 		sigaddset(&action.sa_mask, siginfo[i].signum);
-#else
-		SIGADDSET(action.sa_mask, siginfo[i].signum);
-#endif
 		rc = sigaction(siginfo[i].signum, &action, NULL);
 		if (rc == -1) {
 			sprintf(tmpstr, "sigaction: %s\n", siginfo[i].signame);
@@ -833,36 +807,6 @@ void set_signals(void *sighandler())
 /*
 * Get and set a timer for current process.
 */
-#ifndef _LINUX
-void set_timer(void)
-{
-	struct itimerstruc_t itimer, old_itimer;
-
-	if ((timer = gettimerid(TIMERID_REAL, DELIVERY_SIGNALS)) == -1) {
-		perror("gettimerid");
-		fprintf(stderr, " SEVERE : Could not get timer id, errno=%d.",
-			errno);
-		exit(1);
-	}
-
-	/*
-	 * Start the timer.
-	 */
-	itimer.it_interval.tv_nsec = 0;
-	itimer.it_interval.tv_sec = 0;
-	itimer.it_value.tv_nsec = 0;
-	itimer.it_value.tv_sec = (time_t) (TVAL * 60.0);
-	if (incinterval(timer, &itimer, &old_itimer) == -1) {
-		perror("incinterval");
-		fprintf(stderr,
-			" SEVERE : Could not set timer interval, errno=%d.",
-			errno);
-		(void)reltimerid(timer);
-		exit(1);
-	}
-}
-#else
-
 void set_timer(void)
 {
 	struct itimerval itimer;
@@ -881,7 +825,6 @@ void set_timer(void)
 		exit(1);
 	}
 }
-#endif
 
 /*
  * parse_args
@@ -1157,11 +1100,7 @@ void doit(void)
 #endif
 		/* set the process group so we can terminate all children */
 		set_signals((void *)nextofkin);	/* set up signal handlers and initialize pgrp */
-#ifndef _LINUX
-		procgrp = setpgrp(0, 0);
-#else
 		procgrp = setpgrp();
-#endif
 		if (AUSDEBUG) {
 			fprintf(stderr, "process group: %d\n", procgrp);
 			fflush(stderr);
@@ -1239,7 +1178,6 @@ int main(int argc, char *argv[])
 	parse_args(argc, argv);	/* Get all command line arguments */
 	dprt("value of BVAL = %d, value of DVAL = %d\n", BVAL, DVAL);
 	nodesum = sumit(BVAL, DVAL);
-#ifdef _LINUX
 	if (nodesum > 250) {
 		printf("total number of process to be created "
 		       "nodesum (%d) is greater\n than the allowed "
@@ -1247,7 +1185,6 @@ int main(int argc, char *argv[])
 		printf("reseting the value of nodesum to SEMMSL\n");
 		nodesum = 250;
 	}
-#endif
 
 	dprt("value of nodesum is initiallized to: %d\n", nodesum);
 
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 7/7] sched/process.c: Remove useless TRUE FALSE definitions

[Petr Vorel]

FALSE is not used at all and replace TRUE with 1 (there is no point
trying to use true from <stdbool.h> just for while).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/sched/process_stress/process.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/sched/process_stress/process.c b/testcases/kernel/sched/process_stress/process.c
index b66908e7a..9075305e1 100644
--- a/testcases/kernel/sched/process_stress/process.c
+++ b/testcases/kernel/sched/process_stress/process.c
@@ -46,13 +46,6 @@
 #define MAXBVAL 70
 #define MAXDVAL 11
 
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
 #ifdef DEBUG
 #define prtln()	printf("At line number: %d\n", __LINE__); \
 		fflush(NULL)
@@ -200,7 +193,7 @@ int send_message(int id, mtyp_t type, char *text)
 
 	strcpy(sndbuf.mtext, text);
 	sndbuf.mtyp = type;
-	while (TRUE) {
+	while (1) {
 		rc = msgsnd(id, &sndbuf, sizeof(struct messagebuf), IPC_NOWAIT);
 		if (rc == -1 && errno == EAGAIN) {
 			debugout("msgqueue %d of mtyp %d not ready to send\n",
@@ -992,7 +985,7 @@ void messenger(void)
 	 *  Infinite loop used to receive error messages from children and
 	 *  to terminate process tree.
 	 */
-	while (TRUE) {
+	while (1) {
 		rc = msgrcv(msgerr, &rcvbuf, sizeof(struct messagebuf), 0, 0);
 		if (rc == -1) {
 			switch (errno) {
-- 
2.33.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 249 bytes --]

Hi Petr,

The patchset clean-up looks good.
Reviewed-by: Li Wang <liwang@redhat.com>

But I just wondering, is there a run test file to perform this test case?
I greped the runtest/* but find no file currently includes this:(.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 856 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Petr Vorel]

Hi Li,

> Hi Petr,

> The patchset clean-up looks good.
> Reviewed-by: Li Wang <liwang@redhat.com>

Thanks!

> But I just wondering, is there a run test file to perform this test case?
> I greped the runtest/* but find no file currently includes this:(.

IMHO it's in sched:
$ git grep -w process runtest/
runtest/sched:hackbench01 hackbench 50 process 1000

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 814 bytes --]

> > But I just wondering, is there a run test file to perform this test case?
> > I greped the runtest/* but find no file currently includes this:(.
>
> IMHO it's in sched:
> $ git grep -w process runtest/
> runtest/sched:hackbench01 hackbench 50 process 1000
>

Hmm, here "process" is only a parameter to hackbench01, it
has no direct relevance to the process_stress/process.c.

After checking the history a while, I think that process_stress/process.c
is an independent test and obsoleted for a quite long time, maybe
we can add online back to runtest/sched file or just let is there until
someone converting to the new API.

commit 576f1ee560b2370818b49366bad581952af3dd70
Author: robbiew <robbiew>
Date:   Fri Feb 1 16:39:39 2002 +0000

    Removed a test that was not correctly running.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1789 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Petr Vorel]

> > > But I just wondering, is there a run test file to perform this test case?
> > > I greped the runtest/* but find no file currently includes this:(.

> > IMHO it's in sched:
> > $ git grep -w process runtest/
> > runtest/sched:hackbench01 hackbench 50 process 1000


> Hmm, here "process" is only a parameter to hackbench01, it
> has no direct relevance to the process_stress/process.c.

> After checking the history a while, I think that process_stress/process.c
> is an independent test and obsoleted for a quite long time, maybe
> we can add online back to runtest/sched file or just let is there until
> someone converting to the new API.

> commit 576f1ee560b2370818b49366bad581952af3dd70
> Author: robbiew <robbiew>
> Date:   Fri Feb 1 16:39:39 2002 +0000

>     Removed a test that was not correctly running.

Ah, good catch! I didn't check it's not a binary which would be required.
I'll recheck it once again and send a patch to remove it then.

Kind regards,
Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/7] Cleanup sched/process.c

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 247 bytes --]

> Ah, good catch! I didn't check it's not a binary which would be required.
> I'll recheck it once again and send a patch to remove it then.
>

Agree, it has obsoleted 20 years, I guess it probably not much sense to
keep it.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 657 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp