[igt-dev] [PATCH i-g-t 1/1] Use the new procps library libproc2
Kamil Konieczny
kamil.konieczny at linux.intel.com
Tue Apr 25 08:35:54 UTC 2023
Hi,
I forgot that there is one place which possibly
need a rework, see below.
On 2023-04-24 at 18:26:42 +0200, Kamil Konieczny wrote:
> From: Craig Small <csmall at dropbear.xyz>
>
> Added support for new libproc2.
>
> [Corrected some errors pointed by checkpatch.pl,
> add linux includes in #ifdef __linux__ section,
> use #elif HAVE_LIBPROC2 for new libproc2 code - Kamil]
> Signed-off-by: Craig Small <csmall at dropbear.xyz>
> ---
> lib/igt_aux.c | 243 ++++++++++++++++++++++++++++++++++++++++--------
> lib/meson.build | 7 +-
> meson.build | 10 +-
> 3 files changed, 218 insertions(+), 42 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 672d7d4b0..db6c4da93 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -52,8 +52,19 @@
> #include <assert.h>
> #include <grp.h>
>
> +#ifdef HAVE_LIBPROCPS
> #include <proc/readproc.h>
> +#elif HAVE_LIBPROC2
> +#include <libproc2/pids.h>
> +#endif
> +
> +#include <dirent.h>
> +#ifdef __linux__
> #include <libudev.h>
> +#include <linux/limits.h>
> +#else
> +#include <limits.h>
> +#endif
>
> #include "drmtest.h"
> #include "i915_drm.h"
> @@ -1217,6 +1228,7 @@ void igt_unlock_mem(void)
> */
> int igt_is_process_running(const char *comm)
> {
> +#if HAVE_LIBPROCPS
> PROCTAB *proc;
> proc_t *proc_info;
> bool found = false;
> @@ -1235,6 +1247,26 @@ int igt_is_process_running(const char *comm)
>
> closeproc(proc);
> return found;
> +#elif HAVE_LIBPROC2
> + enum pids_item Item[] = { PIDS_CMD };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> + char *pid_comm;
> + bool found = false;
> +
> + if (procps_pids_new(&info, Item, 1) < 0)
> + return false;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + pid_comm = PIDS_VAL(0, str, stack, info);
> + if (!strncasecmp(pid_comm, comm, strlen(pid_comm))) {
> + found = true;
> + break;
> + }
> + }
> +
> + procps_pids_unref(&info);
> + return found;
> +#endif
> }
>
> /**
> @@ -1251,6 +1283,7 @@ int igt_is_process_running(const char *comm)
> */
> int igt_terminate_process(int sig, const char *comm)
> {
> +#if HAVE_LIBPROCPS
> PROCTAB *proc;
> proc_t *proc_info;
> int err = 0;
> @@ -1272,6 +1305,28 @@ int igt_terminate_process(int sig, const char *comm)
>
> closeproc(proc);
> return err;
> +#elif HAVE_LIBPROC2
> + enum pids_item Items[] = { PIDS_ID_PID, PIDS_CMD };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> + char *pid_comm;
> + int pid;
> + int err = 0;
> +
> + if (procps_pids_new(&info, Items, 2) < 0)
> + return -errno;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + pid = PIDS_VAL(0, s_int, stack, info);
> + pid_comm = PIDS_VAL(1, str, stack, info);
> + if (!strncasecmp(pid_comm, comm, strlen(pid_comm))) {
> + if (kill(pid, sig) < 0)
> + err = -errno;
> + break;
> + }
> + }
> + procps_pids_unref(&info);
> + return err;
> +#endif
> }
>
> struct pinfo {
> @@ -1341,9 +1396,9 @@ igt_show_stat_header(void)
> }
>
> static void
> -igt_show_stat(proc_t *info, int *state, const char *fn)
> +igt_show_stat(const pid_t tid, const char *cmd, int *state, const char *fn)
> {
> - struct pinfo p = { .pid = info->tid, .comm = info->cmd, .fn = fn };
> + struct pinfo p = { .pid = tid, .comm = cmd, .fn = fn };
>
> if (!*state)
> igt_show_stat_header();
> @@ -1353,7 +1408,7 @@ igt_show_stat(proc_t *info, int *state, const char *fn)
> }
>
> static void
> -__igt_lsof_fds(proc_t *proc_info, int *state, char *proc_path, const char *dir)
> +__igt_lsof_fds(const pid_t tid, const char *cmd, int *state, char *proc_path, const char *dir)
> {
> struct dirent *d;
> struct stat st;
> @@ -1400,7 +1455,7 @@ again:
> dirn = dirname(copy_fd_lnk);
>
> if (!strncmp(dir, dirn, strlen(dir)))
> - igt_show_stat(proc_info, state, fd_lnk);
> + igt_show_stat(tid, cmd, state, fd_lnk);
>
> free(copy_fd_lnk);
> free(fd_lnk);
> @@ -1416,13 +1471,13 @@ again:
> static void
> __igt_lsof(const char *dir)
> {
> - PROCTAB *proc;
> - proc_t *proc_info;
> -
> char path[30];
> char *name_lnk;
> struct stat st;
> int state = 0;
> +#ifdef HAVE_LIBPROCPS
> + PROCTAB *proc;
> + proc_t *proc_info;
>
> proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> igt_assert(proc != NULL);
> @@ -1443,19 +1498,56 @@ __igt_lsof(const char *dir)
> name_lnk[read] = '\0';
>
> if (!strncmp(dir, name_lnk, strlen(dir)))
> - igt_show_stat(proc_info, &state, name_lnk);
> + igt_show_stat(proc_info->tid, proc_info->cmd, &state, name_lnk);
>
> /* check also fd, seems that lsof(8) doesn't look here */
> memset(path, 0, sizeof(path));
> snprintf(path, sizeof(path), "/proc/%d/fd", proc_info->tid);
>
> - __igt_lsof_fds(proc_info, &state, path, dir);
> + __igt_lsof_fds(proc_info->tid, proc_info->cmd, &state, path, dir);
>
> free(name_lnk);
> freeproc(proc_info);
> }
>
> closeproc(proc);
> +#elif HAVE_LIBPROC2
> + enum pids_item Items[] = { PIDS_ID_PID, PIDS_CMD };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> +
> + if (procps_pids_new(&info, Items, 2) < 0)
> + return;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + ssize_t read;
> + int tid = PIDS_VAL(0, s_int, stack, info);
> + char *pid_comm = PIDS_VAL(1, str, stack, info);
> +
> + /* check current working directory */
> + memset(path, 0, sizeof(path));
> + snprintf(path, sizeof(path), "/proc/%d/cwd", tid);
> +
> + if (stat(path, &st) == -1)
> + continue;
> +
> + name_lnk = malloc(st.st_size + 1);
> +
> + igt_assert((read = readlink(path, name_lnk, st.st_size + 1)));
> + name_lnk[read] = '\0';
> +
> + if (!strncmp(dir, name_lnk, strlen(dir)))
> + igt_show_stat(tid, pid_comm, &state, name_lnk);
> +
> + /* check also fd, seems that lsof(8) doesn't look here */
> + memset(path, 0, sizeof(path));
> + snprintf(path, sizeof(path), "/proc/%d/fd", tid);
> +
> + __igt_lsof_fds(tid, pid_comm, &state, path, dir);
> +
> + free(name_lnk);
> + }
> + procps_pids_unref(&info);
> +#endif
> }
>
> /**
> @@ -1490,7 +1582,7 @@ igt_lsof(const char *dpath)
> free(sanitized);
> }
>
> -static void pulseaudio_unload_module(proc_t *proc_info)
> +static void pulseaudio_unload_module(const uid_t euid, const gid_t egid)
> {
> struct igt_helper_process pa_proc = {};
> char xdg_dir[PATH_MAX];
> @@ -1498,14 +1590,14 @@ static void pulseaudio_unload_module(proc_t *proc_info)
> struct passwd *pw;
>
> igt_fork_helper(&pa_proc) {
> - pw = getpwuid(proc_info->euid);
> + pw = getpwuid(euid);
> homedir = pw->pw_dir;
> - snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
> + snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", euid);
>
> igt_info("Request pulseaudio to stop using audio device\n");
>
> - setgid(proc_info->egid);
> - setuid(proc_info->euid);
> + setgid(egid);
> + setuid(euid);
> clearenv();
> setenv("HOME", homedir, 1);
> setenv("XDG_RUNTIME_DIR",xdg_dir, 1);
> @@ -1524,10 +1616,13 @@ static void pipewire_reserve_wait(void)
> char xdg_dir[PATH_MAX];
> const char *homedir;
> struct passwd *pw;
> - proc_t *proc_info;
> - PROCTAB *proc;
> + int tid = 0, euid, egid;
>
> +#ifdef HAVE_LIBPROCPS
> igt_fork_helper(&pw_reserve_proc) {
------- ^
See below for libproc2 code.
> + proc_t *proc_info;
> + PROCTAB *proc;
> +
> igt_info("Preventing pipewire-pulse to use the audio drivers\n");
>
> proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> @@ -1540,19 +1635,43 @@ static void pipewire_reserve_wait(void)
> }
> closeproc(proc);
>
> + tid = proc_info->tid;
> + euid = proc_info->euid;
> + egid = proc_info->egid;
> + freeproc(proc_info);
> +#elif HAVE_LIBPROC2
> + igt_fork(child, 1) {
------- ^
These are not the same constructs, +cc Petri.
Regards,
Kamil
> + enum pids_item Items[] = { PIDS_ID_PID, PIDS_ID_EUID, PIDS_ID_EGID };
> + enum rel_items { EU_PID, EU_EUID, EU_EGID };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> +
> + igt_info("Preventing pipewire-pulse to use the audio drivers\n");
> +
> + if (procps_pids_new(&info, Items, 3) < 0)
> + return;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + tid = PIDS_VAL(EU_PID, s_int, stack, info);
> + if (pipewire_pulse_pid == tid)
> + break;
> + }
> + euid = PIDS_VAL(EU_EUID, s_int, stack, info);
> + egid = PIDS_VAL(EU_EGID, s_int, stack, info);
> + procps_pids_unref(&info);
> +#endif
> +
> /* Sanity check: if it can't find the process, it means it has gone */
> - if (pipewire_pulse_pid != proc_info->tid)
> + if (pipewire_pulse_pid != tid)
> exit(0);
>
> - pw = getpwuid(proc_info->euid);
> + pw = getpwuid(euid);
> homedir = pw->pw_dir;
> - snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
> - setgid(proc_info->egid);
> - setuid(proc_info->euid);
> + snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", euid);
> + setgid(egid);
> + setuid(euid);
> clearenv();
> setenv("HOME", homedir, 1);
> setenv("XDG_RUNTIME_DIR",xdg_dir, 1);
> - freeproc(proc_info);
>
> /*
> * pw-reserve will run in background. It will only exit when
> @@ -1570,9 +1689,7 @@ static void pipewire_reserve_wait(void)
> int pipewire_pulse_start_reserve(void)
> {
> bool is_pw_reserve_running = false;
> - proc_t *proc_info;
> int attempts = 0;
> - PROCTAB *proc;
>
> if (!pipewire_pulse_pid)
> return 0;
> @@ -1584,6 +1701,10 @@ int pipewire_pulse_start_reserve(void)
> * pipewire version 0.3.50 or upper.
> */
> for (attempts = 0; attempts < PIPEWIRE_RESERVE_MAX_TIME; attempts++) {
> +#ifdef HAVE_LIBPROCPS
> + PROCTAB *proc;
> + proc_t *proc_info;
> +
> usleep(1000);
> proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> igt_assert(proc != NULL);
> @@ -1598,6 +1719,24 @@ int pipewire_pulse_start_reserve(void)
> freeproc(proc_info);
> }
> closeproc(proc);
> +#elif HAVE_LIBPROC2
> + enum pids_item Items[] = { PIDS_ID_PID, PIDS_CMD };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> +
> + usleep(1000);
> +
> + if (procps_pids_new(&info, Items, 2) < 0)
> + return 1;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + if (!strcmp(PIDS_VAL(1, str, stack, info), "pw-reserve")) {
> + is_pw_reserve_running = true;
> + pipewire_pw_reserve_pid = PIDS_VAL(0, s_int, stack, info);
> + break;
> + }
> + }
> + procps_pids_unref(&info);
> +#endif
> if (is_pw_reserve_running)
> break;
> }
> @@ -1645,7 +1784,7 @@ void pipewire_pulse_stop_reserve(void)
> * If the check fails, it means that the process can simply be killed.
> */
> static int
> -__igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
> +__igt_lsof_audio_and_kill_proc(const pid_t tid, const char *cmd, const uid_t euid, const gid_t egid, char *proc_path)
> {
> const char *audio_dev = "/dev/snd/";
> char path[PATH_MAX * 2];
> @@ -1670,10 +1809,10 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
> * 2) unload/unbind the the audio driver(s);
> * 3) stop the pw-reserve thread.
> */
> - if (!strcmp(proc_info->cmd, "pipewire-pulse")) {
> + if (!strcmp(cmd, "pipewire-pulse")) {
> igt_info("process %d (%s) is using audio device. Should be requested to stop using them.\n",
> - proc_info->tid, proc_info->cmd);
> - pipewire_pulse_pid = proc_info->tid;
> + tid, cmd);
> + pipewire_pulse_pid = tid;
> return 0;
> }
> /*
> @@ -1685,9 +1824,9 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
> * will respawn them. So, just ignore here, they'll honor pw-reserve,
> * when the time comes.
> */
> - if (!strcmp(proc_info->cmd, "pipewire-media-session"))
> + if (!strcmp(cmd, "pipewire-media-session"))
> return 0;
> - if (!strcmp(proc_info->cmd, "wireplumber"))
> + if (!strcmp(cmd, "wireplumber"))
> return 0;
>
> dp = opendir(proc_path);
> @@ -1723,22 +1862,22 @@ __igt_lsof_audio_and_kill_proc(proc_t *proc_info, char *proc_path)
> * enough to unbind audio modules and won't cause race issues
> * with systemd trying to reload it.
> */
> - if (!strcmp(proc_info->cmd, "pulseaudio")) {
> - pulseaudio_unload_module(proc_info);
> + if (!strcmp(cmd, "pulseaudio")) {
> + pulseaudio_unload_module(euid, egid);
> break;
> }
>
> /* For all other processes, just kill them */
> igt_info("process %d (%s) is using audio device. Should be terminated.\n",
> - proc_info->tid, proc_info->cmd);
> + tid, cmd);
>
> - if (kill(proc_info->tid, SIGTERM) < 0) {
> + if (kill(tid, SIGTERM) < 0) {
> igt_info("Fail to terminate %s (pid: %d) with SIGTERM\n",
> - proc_info->cmd, proc_info->tid);
> - if (kill(proc_info->tid, SIGABRT) < 0) {
> + cmd, tid);
> + if (kill(tid, SIGABRT) < 0) {
> fail++;
> igt_info("Fail to terminate %s (pid: %d) with SIGABRT\n",
> - proc_info->cmd, proc_info->tid);
> + cmd, tid);
> }
> }
>
> @@ -1760,23 +1899,47 @@ int
> igt_lsof_kill_audio_processes(void)
> {
> char path[PATH_MAX];
> + int fail = 0;
> +
> +#ifdef HAVE_LIBPROCPS
> proc_t *proc_info;
> PROCTAB *proc;
> - int fail = 0;
>
> proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> igt_assert(proc != NULL);
> pipewire_pulse_pid = 0;
> -
> while ((proc_info = readproc(proc, NULL))) {
> if (snprintf(path, sizeof(path), "/proc/%d/fd", proc_info->tid) < 1)
> fail++;
> else
> - fail += __igt_lsof_audio_and_kill_proc(proc_info, path);
> + fail += __igt_lsof_audio_and_kill_proc(proc_info->tid, proc_info->cmd, proc_info->euid, proc_info->egid, path);
>
> freeproc(proc_info);
> }
> closeproc(proc);
> +#elif HAVE_LIBPROC2
> + enum pids_item Items[] = { PIDS_ID_PID, PIDS_CMD, PIDS_ID_EUID, PIDS_ID_EGID };
> + enum rel_items { EU_PID, EU_CMD, EU_EUID, EU_EGID };
> + struct pids_info *info = NULL;
> + struct pids_stack *stack;
> + pid_t tid;
> +
> + if (procps_pids_new(&info, Items, 4) < 0)
> + return 1;
> + while ((stack = procps_pids_get(info, PIDS_FETCH_TASKS_ONLY))) {
> + tid = PIDS_VAL(EU_PID, s_int, stack, info);
> +
> + if (snprintf(path, sizeof(path), "/proc/%d/fd", tid) < 1)
> + fail++;
> + else
> + fail += __igt_lsof_audio_and_kill_proc(tid,
> + PIDS_VAL(EU_CMD, str, stack, info),
> + PIDS_VAL(EU_EUID, s_int, stack, info),
> + PIDS_VAL(EU_EGID, s_int, stack, info),
> + path);
> + }
> + procps_pids_unref(&info);
> +#endif
>
> return fail;
> }
> diff --git a/lib/meson.build b/lib/meson.build
> index b21c252b5..48a27a612 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -113,7 +113,6 @@ lib_deps = [
> libdrm,
> libdw,
> libkmod,
> - libprocps,
> libudev,
> math,
> pciaccess,
> @@ -177,6 +176,12 @@ if chamelium.found()
> lib_sources += 'monitor_edids/monitor_edids_helper.c'
> endif
>
> +if libprocps.found()
> + lib_deps += libprocps
> +else
> + lib_deps += libproc2
> +endif
> +
> if get_option('srcdir') != ''
> srcdir = join_paths(get_option('srcdir'), 'tests')
> else
> diff --git a/meson.build b/meson.build
> index 036217844..98acc8daf 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -124,7 +124,15 @@ build_info += 'With libdrm: ' + ','.join(libdrm_info)
>
> pciaccess = dependency('pciaccess', version : '>=0.10')
> libkmod = dependency('libkmod')
> -libprocps = dependency('libprocps', required : true)
> +libprocps = dependency('libprocps', required : false)
> +libproc2 = dependency('libproc2', required : false)
> +if libprocps.found()
> + config.set('HAVE_LIBPROCPS', 1)
> +elif libproc2.found()
> + config.set('HAVE_LIBPROC2', 1)
> +else
> + error('Either libprocps or libproc2 is required')
> +endif
>
> libunwind = dependency('libunwind', required : get_option('libunwind'))
> build_info += 'With libunwind: @0@'.format(libunwind.found())
> --
> 2.37.2
>
More information about the igt-dev
mailing list