[PATCH 1/3] support: Add support_process_state_wait
Florian Weimer
fweimer@redhat.com
Fri Dec 20 14:09:00 GMT 2019
* Adhemerval Zanella:
> It allows parent process to wait for children state using a polling
> strategy over procfs on Linux. The polling is used over ptrace to
> avoid the need to handle signals on the target pid and to handle some
> system specific limitations (such as YAMA).
>
> The polling has some limitations, such as resource consumption due
> the procfs read in a loop and the lack of synchronization after the
> state is obtained.
>
> The interface idea is to simplify some sleep synchronization in waitid
> tests.
The goal is to reduce timeouts, I assume, replacing arbitrary two-second
waits.
> diff --git a/support/process_state.h b/support/process_state.h
> new file mode 100644
> index 0000000000..d616f0e3e6
> --- /dev/null
> +++ b/support/process_state.h
> @@ -0,0 +1,41 @@
> +enum process_state_t
> +{
> + process_state_running = 0x01, /* R (running) */
> + process_state_sleeping = 0x02, /* S (sleeping) */
> + process_state_disk_sleep = 0x04, /* D (disk sleep) */
> + process_state_stopped = 0x08, /* T (stopped) */
> + process_state_tracing_stop = 0x10, /* t (tracing stop) */
> + process_state_dead = 0x20, /* X (dead) */
> + process_state_zombie = 0x40, /* Z (zombie) */
> + process_state_parked = 0x80, /* P (parked) */
> +};
Maybe drop the reversed _t suffix and add a support_ prefix?
> +
> +/* Wait for process PID to reach state STATE. It can be a combination of
> + multiple possible states ('process_state_running | process_state_sleeping')
> + where the function return when any of these state are observed. */
> +void support_process_state_wait (pid_t pid, enum process_state_t state);
This interface does not allow to poll for a process not to have a
specific state. I guess we can adjust it once the need arises, so you
can leave it for now.
> --- /dev/null
> +++ b/support/support_process_state.c
> @@ -0,0 +1,89 @@
> +void
> +support_process_state_wait (pid_t pid, enum process_state_t state)
> +{
> +#ifdef __linux__
> + /* For Linux it does a polling check on /proc/<pid>/status checking on
> + third field. */
> +
> + /* It mimics the kernel states from fs/proc/array.c */
> + static struct process_states_t
> + {
> + enum process_state_t s;
> + char v;
> + } process_states[] = {
> + { process_state_running, 'R' },
> + { process_state_sleeping, 'S' },
> + { process_state_disk_sleep, 'D' },
> + { process_state_stopped, 'T' },
> + { process_state_tracing_stop, 't' },
> + { process_state_dead, 'X' },
> + { process_state_zombie, 'Z' },
> + { process_state_parked, 'P' },
> + };
Should be const.
> + char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
> + + 1];
> + snprintf (proc_path, sizeof (proc_path), "/proc/%i/stat", pid);
> +
> + int fd = xopen (proc_path, O_RDONLY, 0600);
> +
> + char statbuf[3 * sizeof (pid_t) /* pid. */
> + + 2 /* ' (' */
> + + 64 /* task name. */
> + + 2 /* ' )' */
> + + 1 /* <state>, 1 char. */
> + + 1]; /* Final \0. */
> + for (;;)
> + {
> + ssize_t ret = read (fd, statbuf, sizeof (statbuf) - 1);
> + TEST_VERIFY (ret >= 0);
> + statbuf[ret] = '\0';
> +
> + char cur_state;
> + int sret = sscanf (statbuf, "%*i %*s %c", &cur_state);
> + TEST_VERIFY (sret == 1);
Hmm. Does this work if COMM includes a space? I think it's safer to
parse /proc/PID/status.
> + for (size_t i = 0; i < array_length (process_states); ++i)
> + if (state & process_states[i].s && cur_state == process_states[i].v)
> + {
> + close (fd);
> + return;
> + }
> +
> + xlseek (fd, 0, SEEK_SET);
> + nanosleep (&(struct timespec) { 0, 10000000 }, NULL);
> + }
> +
> + close (fd);
Should this have a fallback timeout?
The sleep should probably be usleep or nanosleep, to avoid interfering
with laram.
The test looks reasonable.
Thanks,
Florian
More information about the Libc-alpha
mailing list