Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
To: igt-dev@lists.freedesktop.org
Cc: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>,
	Peter Senna Tschudin <peter.senna@linux.intel.com>,
	Adam Miszczak <adam.miszczak@linux.intel.com>,
	Lukasz Laguna <lukasz.laguna@intel.com>,
	Jakub Kolakowski <jakub1.kolakowski@intel.com>,
	Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>,
	Chris Wilson <chris.p.wilson@intel.com>
Subject: [PATCH v2 i-g-t] lib/igt_core: Refactor libpciaccess init/cleanup wrappers
Date: Wed, 11 Sep 2024 16:13:49 +0200	[thread overview]
Message-ID: <20240911141349.455943-1-marcin.bernatowicz@linux.intel.com> (raw)

Enable reinitialization of the libpciaccess global state, necessary
to correctly handle dynamic add/remove of PCI devices, such as the
creation/removal of Virtual Functions (VFs). Update
igt_pci_system_cleanup() to conditionally call pci_system_cleanup()
based on the initialization state. Introduce igt_pci_system_reinit()
for explicit reinitialization of the libpciaccess global state,
particularly useful after PCI device changes, to be used in
subsequent patches.

v2:
- Change pci_system_initialized type from int to bool for consistency
  with install_handler (Peter)
- Simplify conditional logic in igt_pci_system_init by removing
  redundant assignments

Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
Cc: Lukasz Laguna <lukasz.laguna@intel.com>
Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Cc: Peter Senna Tschudin <peter.senna@linux.intel.com>
---
 lib/igt_core.c | 40 ++++++++++++++++++++++++++++++++++------
 lib/igt_core.h | 36 ++++++++++++++++++++++++------------
 2 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 88b5af732..407f7b551 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -3483,23 +3483,51 @@ void igt_srandom(void)
 }
 
 /* IGT wrappers around libpciaccess init/cleanup functions */
+static bool pci_system_initialized;
+static pthread_mutex_t pci_system_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+void igt_pci_system_cleanup(void)
+{
+	pthread_mutex_lock(&pci_system_mutex);
+	if (pci_system_initialized) {
+		pci_system_cleanup();
+		pci_system_initialized = false;
+	}
+	pthread_mutex_unlock(&pci_system_mutex);
+}
 
 static void pci_system_exit_handler(int sig)
 {
-	pci_system_cleanup();
+	igt_pci_system_cleanup();
 }
 
-static void __pci_system_init(void)
+int igt_pci_system_init(void)
 {
-	if (!igt_warn_on_f(pci_system_init(), "Could not initialize libpciaccess global data\n"))
+	int ret = 0;
+	bool install_handler = false;
+
+	pthread_mutex_lock(&pci_system_mutex);
+	if (!pci_system_initialized) {
+		ret = igt_warn_on_f(pci_system_init(),
+				    "Could not initialize libpciaccess global data\n");
+		if (!ret) {
+			pci_system_initialized = true;
+			install_handler = true;
+		}
+	}
+	pthread_mutex_unlock(&pci_system_mutex);
+
+	if (install_handler)
 		igt_install_exit_handler(pci_system_exit_handler);
+
+	return ret;
 }
 
-int igt_pci_system_init(void)
+int igt_pci_system_reinit(void)
 {
-	static pthread_once_t once_control = PTHREAD_ONCE_INIT;
+	igt_pci_system_cleanup();
 
-	return pthread_once(&once_control, __pci_system_init);
+	return igt_pci_system_init();
 }
 
 /**
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 58864c2bc..90f57402f 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -1530,29 +1530,41 @@ void igt_kmsg(const char *format, ...);
  * igt_pci_system_init:
  * IGT wrapper around pci_system_init()
  *
- * Runs pci_system_init() and installs pci_system_cleanup() as IGT exit handler when
- * called first per thread, subsequent calls are noop.  Tests should use this wrapper
+ * Runs pci_system_init() and installs igt_pci_system_cleanup() as IGT exit handler when
+ * called first per thread, subsequent calls are noop. Tests should use this wrapper
  * instead of pci_system_init() to avoid memory leaking which happens each time a call
  * to pci_system_init() is repeated not preceded by pci_system_cleanup() (may easily
  * happen in consequence of long jumps performed by IGT flow control functions).
  *
- * Return value: equal return value of pthread_once() (return value of pci_system_init()
- *		 can't be passed through pthread_once())
+ * Return:
+ * Return value of pci_system_init() or 0 if pci system is already initialized.
  */
 int igt_pci_system_init(void);
 
+/**
+ * igt_pci_system_reinit:
+ * Reinitialize libpciaccess global data.
+ *
+ * Executes igt_pci_system_cleanup() and igt_pci_system_init() to refresh
+ * the PCI system state, typically needed after PCI devices are added or
+ * removed.
+ *
+ * Note: All previously obtained handles (pci_dev, mmio) become invalid
+ * after this call. Do not use old handles post-reinitialization.
+ *
+ * Return: Outcome of igt_pci_system_init().
+ */
+int igt_pci_system_reinit(void);
+
 /**
  * igt_pci_system_cleanup():
- * IGT replacement for pci_system_cleanup()
+ * IGT wrapper around pci_system_cleanup()
  *
- * For use in IGT library and tests to avoid destroying libpciaccess global data.
- * Direct calls to pci_system_cleanup() should be either dropped or replaced with this
- * wrapper (for code clarity), otherwise subsequent access to libpciaccess global data
- * may be lost unless preceded by direct call to pci_system_init() (not recommended).
+ * Runs pci_system_cleanup() if igt_pci_system_init() was successfully called
+ * before. This allows to refresh the libpciaccess global data when followed
+ * by igt_pci_system_init(), see igt_pci_system_reinit().
  */
-static inline void igt_pci_system_cleanup(void)
-{
-}
+void igt_pci_system_cleanup(void);
 
 void igt_emit_ignore_dmesg_regex(const char *ignore_dmesg_regex);
 
-- 
2.31.1


             reply	other threads:[~2024-09-11 14:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-11 14:13 Marcin Bernatowicz [this message]
2024-09-12  6:42 ` ✗ Fi.CI.BAT: failure for lib/igt_core: Refactor libpciaccess init/cleanup wrappers (rev2) Patchwork
2024-09-12  7:09 ` ✓ CI.xeBAT: success " Patchwork
2024-09-12 11:57 ` ✗ CI.xeFULL: failure " Patchwork

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=20240911141349.455943-1-marcin.bernatowicz@linux.intel.com \
    --to=marcin.bernatowicz@linux.intel.com \
    --cc=adam.miszczak@linux.intel.com \
    --cc=chris.p.wilson@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jakub1.kolakowski@intel.com \
    --cc=janusz.krzysztofik@linux.intel.com \
    --cc=lukasz.laguna@intel.com \
    --cc=peter.senna@linux.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox