[igt-dev] [PATCH i-g-t v3 1/6] tests/core_hotunplug: properly finish processes using audio devices
Mauro Carvalho Chehab
mauro.chehab at linux.intel.com
Thu May 12 06:49:50 UTC 2022
On Wed, 11 May 2022 10:03:26 -0700
Lucas De Marchi <lucas.demarchi at intel.com> wrote:
> On Fri, May 06, 2022 at 01:48:24PM +0200, Mauro Carvalho Chehab wrote:
> >From: Mauro Carvalho Chehab <mchehab at kernel.org>
> >
> >Before unloading or unbinding an audio driver, all processes
> >that are using it must be terminated. The current logic seeks
> >only for alsactl, but ignore other processes, including
> >pulseaudio.
> >
> >Make the logic more general, extending it to any processes that
> >could have an open device under /dev/snd.
> >
> >It should be noticed that some distros like Fedora and openSUSE
> >are now migrating from pulseaudio into pipewire-pulse. Right
> >now, there's no standard distribution-agnostic way to request
> >pipewire-pulse to stop using audio devices, but there's a new
> >patch upstream that will make things easier:
> >
> > https://gitlab.freedesktop.org/pipewire/pipewire/-/commit/6ad6300ec657c88322a8cd6f3548261d3dc05359
> >
> >Which should be available for pipewire-pulse versions 0.3.50 and
> >upper.
>
> this is for running tests... I don't think it's really expected to have
> pulseaudio or pipewire running. We should basically be doing
> systemctl disable/mask as a configuration step.
That pretty much depends on where this runs: pipewire-pulse is new.
Systems like Fedora and the upcoming RHEL 9 have it loaded by default. Distros
with older stacks usually came with pulseaudio, also enabled by default.
See, a special-purpose machine that was carefully prepared for CI
testing can be configured to have pulseaudio/pipewire-pulse not installed
or disabled, but if those are running, they should stop using the
audio device before removing the device driver.
> And if we want to handle that, why don't we simply systemctl stop them
> instead of having specific commands for each of them to unbind?
Because it doesn't work. On Fedora 35:
$ sudo systemctl stop pipewire-pulse
Failed to stop pipewire-pulse.service: Unit pipewire-pulse.service not loaded.
$ ps ax|grep pulse
2337 ? S<sl 120:47 /usr/bin/pipewire-pulse
$ find /etc/systemd/|grep pulse
/etc/systemd/user/sockets.target.wants/pipewire-pulse.socket
On Ubuntu 20.04:
$ sudo systemctl stop pulseaudio
Failed to stop pulseaudio.service: Unit pulseaudio.service not loaded.
$ ps ax|grep pulse
1469 ? S<sl 0:00 /usr/bin/pulseaudio --daemonize=no --log-target=journal
2061 ? Ssl 0:00 /usr/bin/pulseaudio --daemonize=no --log-target=journal
$ find /etc/systemd/|grep pulse
/etc/systemd/user/default.target.wants/pulseaudio.service
/etc/systemd/user/sockets.target.wants/pulseaudio.socket
Basically, systemctl uses this "socket" kind of service, which is
triggered by audio device detection.
The only way to stop pulseaudio to use an audio device is to call its
API and send a request for it to release the audio device. The same
is true for pipewire-pulse.
> yeah, the fallback to simply kill process with open handles to sound
> device is nice
This is not a fallback... it is to kill other processes like alsactl,
which is started as a daemon on several distributions.
>
> Lucas De Marchi
>
> >
> >Signed-off-by: Mauro Carvalho Chehab <mchehab at kernel.org>
> >---
> > lib/igt_aux.c | 157 +++++++++++++++++++++++++++++++++++++++++
> > lib/igt_aux.h | 1 +
> > lib/igt_kmod.c | 53 ++++++++------
> > lib/igt_kmod.h | 2 +
> > tests/core_hotunplug.c | 11 +--
> > 5 files changed, 196 insertions(+), 28 deletions(-)
> >
> >diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> >index 03cc38c93e5c..f56b5a06f100 100644
> >--- a/lib/igt_aux.c
> >+++ b/lib/igt_aux.c
> >@@ -31,6 +31,7 @@
> > #endif
> > #include <stdio.h>
> > #include <fcntl.h>
> >+#include <pwd.h>
> > #include <sys/stat.h>
> > #include <sys/ioctl.h>
> > #include <string.h>
> >@@ -1410,6 +1411,162 @@ igt_lsof(const char *dpath)
> > free(sanitized);
> > }
> >
> >+static void pulseaudio_unload_module(proc_t *proc_info)
> >+{
> >+ char xdg_dir[PATH_MAX];
> >+ const char *homedir;
> >+ struct passwd *pw;
> >+
> >+ igt_fork(child, 1) {
> >+ pw = getpwuid(proc_info->euid);
> >+ homedir = pw->pw_dir;
> >+ snprintf(xdg_dir, sizeof(xdg_dir), "/run/user/%d", proc_info->euid);
> >+
> >+ igt_info("Ask pulseaudio to stop using audio device\n");
> >+
> >+ setgid(proc_info->egid);
> >+ setuid(proc_info->euid);
> >+ clearenv();
> >+ setenv("HOME", homedir, 1);
> >+ setenv("XDG_RUNTIME_DIR",xdg_dir, 1);
> >+
> >+ system("for i in $(pacmd list-sources|grep module:|cut -d : -f 2); do pactl unload-module $i; done");
> >+ }
> >+ igt_waitchildren();
> >+}
> >+
> >+/**
> >+ * __igt_lsof_audio_and_kill_proc() - check if a given process is using an
> >+ * audio device. If so, stop or prevent them to use such devices.
> >+ *
> >+ * @proc_info: process struct, as returned by readproc()
> >+ * @proc_path: path of the process under procfs
> >+ *
> >+ * No processes can be using an audio device by the time it gets removed.
> >+ * This function checks if a process is using an audio device from /dev/snd.
> >+ * If so, it will check:
> >+ * - if the process is pulseaudio, it can't be killed, as systemd will
> >+ * respawn it. So, instead, send a request for it to stop bind the
> >+ * audio devices.
> >+ * 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)
> >+{
> >+ const char *audio_dev = "/dev/snd/";
> >+ char path[PATH_MAX * 2];
> >+ struct dirent *d;
> >+ struct stat st;
> >+ char *fd_lnk;
> >+ int fail = 0;
> >+ ssize_t read;
> >+
> >+ DIR *dp = opendir(proc_path);
> >+ igt_assert(dp);
> >+
> >+ while ((d = readdir(dp))) {
> >+ if (*d->d_name == '.')
> >+ continue;
> >+
> >+ memset(path, 0, sizeof(path));
> >+ snprintf(path, sizeof(path), "%s/%s", proc_path, d->d_name);
> >+
> >+ if (lstat(path, &st) == -1)
> >+ continue;
> >+
> >+ fd_lnk = malloc(st.st_size + 1);
> >+
> >+ igt_assert((read = readlink(path, fd_lnk, st.st_size + 1)));
> >+ fd_lnk[read] = '\0';
> >+
> >+ if (strncmp(audio_dev, fd_lnk, strlen(audio_dev))) {
> >+ free(fd_lnk);
> >+ continue;
> >+ }
> >+
> >+ free(fd_lnk);
> >+
> >+ /*
> >+ * In order to avoid racing against pa/systemd, ensure that
> >+ * pulseaudio will close all audio files. This should be
> >+ * 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);
> >+ break;
> >+ }
> >+
> >+ /*
> >+ * FIXME: terminating pipewire-pulse is not that easy, as
> >+ * pipewire there's no standard way up to pipewire version
> >+ * 0.3.49. Just trying to kill pipewire will start a race
> >+ * between IGT and systemd. If IGT wins, the audio driver
> >+ * will be unloaded before systemd tries to reload it, but
> >+ * if systemd wins, the audio device will be re-opened and
> >+ * used before IGT has a chance to remove the audio driver.
> >+ * Pipewire version 0.3.50 should bring a standard way:
> >+ *
> >+ * 1) start a thread running:
> >+ * pw-reserve -n Audio0 -r
> >+ * 2) unload/unbind the the audio driver(s);
> >+ * 3) stop the pw-reserve thread.
> >+ *
> >+ * We should add support for it once distros start shipping it.
> >+ */
> >+
> >+ /* 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);
> >+
> >+ if (kill(proc_info->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) {
> >+ fail++;
> >+ igt_info("Fail to terminate %s (pid: %d) with SIGABRT\n",
> >+ proc_info->cmd, proc_info->tid);
> >+ }
> >+ }
> >+
> >+ break;
> >+ }
> >+
> >+ closedir(dp);
> >+ return fail;
> >+}
> >+
> >+/*
> >+ * This function identifies each process running on the machine that is
> >+ * opening an audio device and tries to stop it.
> >+ *
> >+ * Special care should be taken with pipewire and pipewire-pulse, as those
> >+ * daemons are respanned if they got killed.
> >+ */
> >+int
> >+igt_lsof_kill_audio_processes(void)
> >+{
> >+ char path[PATH_MAX];
> >+ proc_t *proc_info;
> >+ PROCTAB *proc;
> >+ int fail = 0;
> >+
> >+ proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> >+ igt_assert(proc != NULL);
> >+
> >+ 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);
> >+
> >+ freeproc(proc_info);
> >+ }
> >+ closeproc(proc);
> >+
> >+ return fail;
> >+}
> >+
> > static struct igt_siglatency {
> > timer_t timer;
> > struct timespec target;
> >diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> >index 9f2588aeca90..bb96d1afb777 100644
> >--- a/lib/igt_aux.h
> >+++ b/lib/igt_aux.h
> >@@ -306,6 +306,7 @@ bool igt_allow_unlimited_files(void);
> > int igt_is_process_running(const char *comm);
> > int igt_terminate_process(int sig, const char *comm);
> > void igt_lsof(const char *dpath);
> >+int igt_lsof_kill_audio_processes(void);
> >
> > #define igt_hweight(x) \
> > __builtin_choose_expr(sizeof(x) == 8, \
> >diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> >index f252535d5a3a..133d19048a9b 100644
> >--- a/lib/igt_kmod.c
> >+++ b/lib/igt_kmod.c
> >@@ -389,7 +389,7 @@ igt_i915_driver_load(const char *opts)
> > return 0;
> > }
> >
> >-int __igt_i915_driver_unload(const char **who)
> >+int igt_audio_driver_unload(const char **who)
> > {
> > int ret;
> > const char *sound[] = {
> >@@ -398,6 +398,27 @@ int __igt_i915_driver_unload(const char **who)
> > NULL,
> > };
> >
> >+ for (const char **m = sound; *m; m++) {
> >+ if (igt_kmod_is_loaded(*m)) {
> >+ if (igt_lsof_kill_audio_processes())
> >+ return EACCES;
> >+
> >+ kick_snd_hda_intel();
> >+ ret = igt_kmod_unload(*m, 0);
> >+ if (ret) {
> >+ if (who)
> >+ *who = *m;
> >+ return ret;
> >+ }
> >+ }
> >+ }
> >+ return 0;
> >+}
> >+
> >+int __igt_i915_driver_unload(const char **who)
> >+{
> >+ int ret;
> >+
> > const char *aux[] = {
> > /* gen5: ips uses symbol_get() so only a soft module dependency */
> > "intel_ips",
> >@@ -411,27 +432,19 @@ int __igt_i915_driver_unload(const char **who)
> > /* unbind vt */
> > bind_fbcon(false);
> >
> >- for (const char **m = sound; *m; m++) {
> >- if (igt_kmod_is_loaded(*m)) {
> >- igt_terminate_process(SIGTERM, "alsactl");
> >- kick_snd_hda_intel();
> >- ret = igt_kmod_unload(*m, 0);
> >- if (ret) {
> >- if (who)
> >- *who = *m;
> >- return ret;
> >- }
> >- }
> >- }
> >+ ret = igt_audio_driver_unload(who);
> >+ if (ret)
> >+ return ret;
> >
> > for (const char **m = aux; *m; m++) {
> >- if (igt_kmod_is_loaded(*m)) {
> >- ret = igt_kmod_unload(*m, 0);
> >- if (ret) {
> >- if (who)
> >- *who = *m;
> >- return ret;
> >- }
> >+ if (!igt_kmod_is_loaded(*m))
> >+ continue;
> >+
> >+ ret = igt_kmod_unload(*m, 0);
> >+ if (ret) {
> >+ if (who)
> >+ *who = *m;
> >+ return ret;
> > }
> > }
> >
> >diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
> >index 0898122b3efe..15f0be31e8e4 100644
> >--- a/lib/igt_kmod.h
> >+++ b/lib/igt_kmod.h
> >@@ -36,6 +36,8 @@ bool igt_kmod_has_param(const char *mod_name, const char *param);
> > int igt_kmod_load(const char *mod_name, const char *opts);
> > int igt_kmod_unload(const char *mod_name, unsigned int flags);
> >
> >+int igt_audio_driver_unload(const char **whom);
> >+
> > int igt_i915_driver_load(const char *opts);
> > int igt_i915_driver_unload(void);
> > int __igt_i915_driver_unload(const char **whom);
> >diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> >index 02eae19e1e16..18e37ec8feba 100644
> >--- a/tests/core_hotunplug.c
> >+++ b/tests/core_hotunplug.c
> >@@ -152,19 +152,14 @@ static void driver_unbind(struct hotunplug *priv, const char *prefix,
> > * safest and easiest way out.
> > */
> > if (priv->snd_unload) {
> >- igt_terminate_process(SIGTERM, "alsactl");
> >-
> >- /* unbind snd_hda_intel */
> >- kick_snd_hda_intel();
> >-
> >- if (igt_kmod_unload("snd_hda_intel", 0)) {
> >+ if (igt_audio_driver_unload(NULL)) {
> > priv->snd_unload = false;
> >- igt_warn("Could not unload snd_hda_intel\n");
> >+ igt_warn("Could not unload audio driver\n");
> > igt_kmod_list_loaded();
> > igt_lsof("/dev/snd");
> > igt_skip("Audio is in use, skipping\n");
> > } else {
> >- igt_info("Preventively unloaded snd_hda_intel\n");
> >+ igt_info("Preventively unloaded audio driver\n");
> > }
> > }
> >
> >--
> >2.35.1
> >
More information about the igt-dev
mailing list