All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
To: "Bai, Zongyao" <zongyao.bai@intel.com>
Cc: Jia Yao <jia.yao@intel.com>, <igt-dev@lists.freedesktop.org>
Subject: Re: [PATCH] lib: replace libprocps/libproc2 with /proc-based process enumeration
Date: Thu, 11 Jun 2026 14:22:55 -0700	[thread overview]
Message-ID: <87bjdgixg0.wl-ashutosh.dixit@intel.com> (raw)
In-Reply-To: <18325ddb-1d87-4bec-bcd0-64686805d231@intel.com>

On Tue, 09 Jun 2026 10:51:13 -0700, Bai, Zongyao wrote:
>
> Hi Jia,
>
> LGTM
>
>  Reviewed-by: Zongyao Bai <zongyao.bai@intel.com>

Hmm, not sure what's with the '_' (or space) before Reviewed-by, because of
which R-b doesn't show if we retrieve/apply the patch.

Anyway, I've fixed this up and merged it.

Thanks for the patch and the review.

-Ashutosh

>
>
> On 6/3/2026 11:28 AM, Jia Yao wrote:
> > The previous code used compile-time #ifdef HAVE_LIBPROCPS / HAVE_LIBPROC2
> > to select between two incompatible APIs, requiring separate builds for
> > Ubuntu 22.04 (libprocps) and Ubuntu 24.04 (libproc2).
> > Replace the platform-specific code with internal static helpers in
> > igt_aux.c that read /proc/<pid>/status directly, requiring no external
> > library and working identically across distro versions.
> > The libprocps/libproc2 meson dependencies have been removed entirely.
> >
> > Cc: Zongyao Bai <zongyao.bai@intel.com>
> > Signed-off-by: Jia Yao <jia.yao@intel.com>
> > ---
> >   lib/igt_aux.c   | 203 +++++++++++++++++++++++++++++++-----------------
> >   lib/meson.build |   6 --
> >   meson.build     |  13 +---
> >   3 files changed, 131 insertions(+), 91 deletions(-)
> >
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 87c0593a4..0d250ae2f 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -53,12 +53,6 @@
> >   #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>
> > @@ -1285,93 +1279,156 @@ void igt_unlock_mem(void)
> >	locked_mem = NULL;
> >   }
> >   +/*
> > + * Internal /proc-based process iterator.  Reads /proc/<pid>/status directly
> > + * so there is no dependency on libprocps or libproc2.
> > + *
> > + * /proc/<pid>/status fields used:
> > + *   Name:  <comm>  - command name (kernel escapes spaces, so %s is safe)
> > + *   Pid:   <pid>
> > + *   Uid:   ruid euid suid fsuid
> > + *   Gid:   rgid egid sgid fsgid
> > + */
> > +struct procps_iter {
> > +	DIR   *procdir;
> > +	char   comm[64];
> > +	pid_t  tid;
> > +	uid_t  euid;
> > +	gid_t  egid;
> > +};
> > +
> > +static struct procps_iter *procps_open(void)
> > +{
> > +	struct procps_iter *iter = calloc(1, sizeof(*iter));
> > +
> > +	if (!iter)
> > +		return NULL;
> > +
> > +	iter->procdir = opendir("/proc");
> > +	if (!iter->procdir) {
> > +		free(iter);
> > +		return NULL;
> > +	}
> > +
> > +	return iter;
> > +}
> > +
> > +static bool procps_next(struct procps_iter *iter,
> > +			pid_t *tid, uid_t *euid, gid_t *egid,
> > +			const char **comm)
> > +{
> > +	struct dirent *ent;
> > +
> > +	if (!iter || !iter->procdir)
> > +		return false;
> > +
> > +	while ((ent = readdir(iter->procdir))) {
> > +		char path[64];
> > +		FILE *f;
> > +		pid_t _tid = 0;
> > +		uid_t _euid = 0;
> > +		gid_t _egid = 0;
> > +		char _comm[64] = "";
> > +		int fields = 0;   /* bit 0=Name, 1=Pid, 2=Uid, 3=Gid */
> > +		char line[256];
> > +
> > +		if (ent->d_name[0] < '1' || ent->d_name[0] > '9')
> > +			continue;
> > +
> > +		snprintf(path, sizeof(path), "/proc/%.20s/status", ent->d_name);
> > +
> > +		f = fopen(path, "r");
> > +		if (!f)
> > +			continue;
> > +
> > +		while (fields != 0xf && fgets(line, sizeof(line), f)) {
> > +			if (!(fields & 0x1) && strncmp(line, "Name:", 5) == 0) {
> > +				if (sscanf(line + 5, " %63s", _comm) == 1)
> > +					fields |= 0x1;
> > +			} else if (!(fields & 0x2) && strncmp(line, "Pid:", 4) == 0) {
> > +				if (sscanf(line + 4, " %d", &_tid) == 1)
> > +					fields |= 0x2;
> > +			} else if (!(fields & 0x4) && strncmp(line, "Uid:", 4) == 0) {
> > +				/* skip ruid, read euid */
> > +				unsigned int _e;
> > +
> > +				if (sscanf(line + 4, " %*u %u", &_e) == 1) {
> > +					_euid = (uid_t)_e;
> > +					fields |= 0x4;
> > +				}
> > +			} else if (!(fields & 0x8) && strncmp(line, "Gid:", 4) == 0) {
> > +				/* skip rgid, read egid */
> > +				unsigned int _e;
> > +
> > +				if (sscanf(line + 4, " %*u %u", &_e) == 1) {
> > +					_egid = (gid_t)_e;
> > +					fields |= 0x8;
> > +				}
> > +			}
> > +		}
> > +
> > +		fclose(f);
> > +
> > +		if (_tid <= 0)
> > +			continue;
> > +
> > +		iter->tid  = _tid;
> > +		iter->euid = _euid;
> > +		iter->egid = _egid;
> > +		snprintf(iter->comm, sizeof(iter->comm), "%s", _comm);
> > +
> > +		*tid  = iter->tid;
> > +		*euid = iter->euid;
> > +		*egid = iter->egid;
> > +		*comm = iter->comm;
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static void procps_close(struct procps_iter *iter)
> > +{
> > +	if (!iter)
> > +		return;
> > +
> > +	if (iter->procdir)
> > +		closedir(iter->procdir);
> > +
> > +	free(iter);
> > +}
> > +
> >   struct igt_process {
> > -#ifdef HAVE_LIBPROCPS
> > -	PROCTAB * proc;
> > -	proc_t *proc_info;
> > -#elif HAVE_LIBPROC2
> > -	struct pids_info *info;
> > -	struct pids_stack *stack;
> > -#endif
> > +	struct procps_iter *iter;
> >	pid_t tid;
> > -	pid_t euid;
> > -	pid_t egid;
> > -	char *comm;
> > +	uid_t euid;
> > +	gid_t egid;
> > +	const char *comm;
> >   };
> >     static void open_process(struct igt_process *prcs)
> >   {
> > -#ifdef HAVE_LIBPROCPS
> > -	prcs->proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> > -	igt_assert_f(prcs->proc != NULL, "procps open failed\n");
> > -	prcs->proc_info = NULL;
> > -#elif HAVE_LIBPROC2
> > -	enum pids_item Items[] = { PIDS_ID_PID, PIDS_ID_EUID, PIDS_ID_EGID, PIDS_CMD };
> > -	int err;
> > -
> > -	prcs->info = NULL;
> > -	err = procps_pids_new(&prcs->info, Items, 4);
> > -	igt_assert_f(err >= 0, "procps-ng open failed\n");
> > -	prcs->stack = NULL;
> > -#endif
> > +	prcs->iter = procps_open();
> > +	igt_assert_f(prcs->iter, "procps open failed (no /proc filesystem?)\n");
> >	prcs->tid = 0;
> >	prcs->comm = NULL;
> >   }
> >     static void close_process(struct igt_process *prcs)
> >   {
> > -#ifdef HAVE_LIBPROCPS
> > -	if (prcs->proc_info)
> > -		freeproc(prcs->proc_info);
> > -
> > -	closeproc(prcs->proc);
> > -	prcs->proc_info = NULL;
> > -	prcs->proc = NULL;
> > -#elif HAVE_LIBPROC2
> > -	procps_pids_unref(&prcs->info);
> > -	prcs->info = NULL;
> > -#endif
> > +	procps_close(prcs->iter);
> > +	prcs->iter = NULL;
> >	prcs->tid = 0;
> >	prcs->comm = NULL;
> >   }
> >     static bool get_process_ids(struct igt_process *prcs)
> >   {
> > -#ifdef HAVE_LIBPROCPS
> > -	if (prcs->proc_info)
> > -		freeproc(prcs->proc_info);
> > -
> > -	prcs->tid = 0;
> > -	prcs->comm = NULL;
> > -	prcs->proc_info = readproc(prcs->proc, NULL);
> > -
> > -	if (prcs->proc_info) {
> > -		prcs->tid = prcs->proc_info->tid;
> > -		prcs->euid = prcs->proc_info->euid;
> > -		prcs->egid = prcs->proc_info->egid;
> > -		prcs->comm = prcs->proc_info->cmd;
> > -	}
> > -#elif HAVE_LIBPROC2
> > -	enum rel_items { EU_PID, EU_EUID, EU_EGID, EU_CMD }; // order at open
> > -
> >	prcs->tid = 0;
> >	prcs->comm = NULL;
> > -	prcs->stack = procps_pids_get(prcs->info, PIDS_FETCH_TASKS_ONLY);
> > -	if (prcs->stack) {
> > -#if defined(HAVE_LIBPROC2_POST_4_0_5_API)
> > -		prcs->tid = PIDS_VAL(EU_PID, s_int, prcs->stack);
> > -		prcs->euid = PIDS_VAL(EU_EUID, s_int, prcs->stack);
> > -		prcs->egid = PIDS_VAL(EU_EGID, s_int, prcs->stack);
> > -		prcs->comm = PIDS_VAL(EU_CMD, str, prcs->stack);
> > -#else
> > -		prcs->tid = PIDS_VAL(EU_PID, s_int, prcs->stack, prcs->info);
> > -		prcs->euid = PIDS_VAL(EU_EUID, s_int, prcs->stack, prcs->info);
> > -		prcs->egid = PIDS_VAL(EU_EGID, s_int, prcs->stack, prcs->info);
> > -		prcs->comm = PIDS_VAL(EU_CMD, str, prcs->stack, prcs->info);
> > -#endif
> > -	}
> > -#endif
> > -	return prcs->tid != 0;
> > +	return procps_next(prcs->iter,
> > +			   &prcs->tid, &prcs->euid, &prcs->egid,
> > +			   &prcs->comm);
> >   }
> >     /**
> > diff --git a/lib/meson.build b/lib/meson.build
> > index a8d6566ae..f25ecd8b2 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -257,12 +257,6 @@ if build_xe_eudebug
> >	lib_sources += 'xe/xe_eudebug.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 e750f7a51..4def24cbc 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -133,18 +133,7 @@ build_info += 'With libdrm: ' + ','.join(libdrm_info)
> >     pciaccess = dependency('pciaccess', version : '>=0.10')
> >   libkmod = dependency('libkmod')
> > -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)
> > -  if libproc2.version().version_compare('>= 4.0.5')
> > -    config.set('HAVE_LIBPROC2_POST_4_0_5_API', 1)
> > -  endif
> > -else
> > -  error('Either libprocps or libproc2 is required')
> > -endif
> > +# igt_procps.c reads /proc directly; libprocps and libproc2 are not needed.
> >     libunwind = dependency('libunwind', required :
> > get_option('libunwind'))
> >   if libunwind.found()

      reply	other threads:[~2026-06-11 21:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03 18:28 [PATCH] lib: replace libprocps/libproc2 with /proc-based process enumeration Jia Yao
2026-06-04  1:23 ` ✓ i915.CI.BAT: success for " Patchwork
2026-06-04  1:30 ` ✓ Xe.CI.BAT: " Patchwork
2026-06-04 17:29 ` ✓ Xe.CI.FULL: " Patchwork
2026-06-05  2:11 ` ✓ i915.CI.Full: " Patchwork
2026-06-09 17:51 ` [PATCH] " Bai, Zongyao
2026-06-11 21:22   ` Dixit, Ashutosh [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bjdgixg0.wl-ashutosh.dixit@intel.com \
    --to=ashutosh.dixit@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jia.yao@intel.com \
    --cc=zongyao.bai@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.