[PATCH i-g-t v2 1/3] lib/igt_device_scan: drop device scan cache logic
Lucas De Marchi
lucas.demarchi at intel.com
Thu Jan 23 17:25:35 UTC 2025
On Thu, Jan 23, 2025 at 10:52:06AM +0100, Zbigniew Kempczyński wrote:
>Tests which plays with module unload/load/reload must ensure scanned
>device list cache is up-to-date. Missing direct calling of device scan
>after module operations may lead to use stale data and use invalid
>device name if it was changed in the meantime.
>
>Lucas noticed device scanning overhead is minimal and we may drop
>caching logic and scan (get drm data from udev) unconditionally.
>From igt_runner perspective where single subtest is a separate process
>device scan cost might likely be ignored.
>
>Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
awesome, thanks
Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
Lucas De Marchi
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>---
>v2: Retrieve back current scanning state to prevent unnecessary freeing.
>---
> benchmarks/gem_wsim.c | 2 +-
> lib/drmtest.c | 2 +-
> lib/igt_device_scan.c | 14 ++++----------
> lib/igt_device_scan.h | 2 +-
> lib/igt_multigpu.c | 2 +-
> tests/core_hotunplug.c | 2 +-
> tests/device_reset.c | 2 +-
> tests/intel/i915_suspend.c | 4 ++--
> tools/intel_gpu_top.c | 2 +-
> tools/intel_pm_rpm.c | 2 +-
> tools/lsgpu.c | 2 +-
> 11 files changed, 15 insertions(+), 21 deletions(-)
>
>diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>index 454b6f017b..7a6b100288 100644
>--- a/benchmarks/gem_wsim.c
>+++ b/benchmarks/gem_wsim.c
>@@ -3178,7 +3178,7 @@ int main(int argc, char **argv)
> }
> }
>
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> if (list_devices_arg) {
> struct igt_devices_print_format fmt = {
>diff --git a/lib/drmtest.c b/lib/drmtest.c
>index 2dd4540b85..436b6de780 100644
>--- a/lib/drmtest.c
>+++ b/lib/drmtest.c
>@@ -475,7 +475,7 @@ void drm_load_module(unsigned int chipset)
> }
>
> pthread_mutex_unlock(&mutex);
>- igt_devices_scan(true);
>+ igt_devices_scan();
> }
>
> static int __open_driver(const char *base, int offset, unsigned int chipset, int as_idx)
>diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>index 8e2297087e..44b632008b 100644
>--- a/lib/igt_device_scan.c
>+++ b/lib/igt_device_scan.c
>@@ -1090,18 +1090,12 @@ void igt_devices_free(void)
> * igt_devices_scan
> * @force: enforce scanning devices
> *
>- * Function scans udev in search of gpu devices. For first run it can be
>- * called with @force = false. If something changes during the the test
>- * or test does some module loading (new drm devices occurs during execution)
>- * function must be called again with @force = true to refresh device array.
>+ * Function scans udev in search of gpu devices.
> */
>-void igt_devices_scan(bool force)
>+void igt_devices_scan(void)
> {
>- if (force && igt_devs.devs_scanned)
>- igt_devices_free();
>-
> if (igt_devs.devs_scanned)
>- return;
>+ igt_devices_free();
>
> prepare_scan();
> scan_drm_devices();
>@@ -1983,7 +1977,7 @@ static bool __igt_device_card_match(const char *filter,
> * Scan devices in case the user hasn't yet,
> * but leave a decision on forced rescan on the user side.
> */
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> if (igt_device_filter_apply(filter) == false)
> return false;
>diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
>index 9087337458..fa90134aa3 100644
>--- a/lib/igt_device_scan.h
>+++ b/lib/igt_device_scan.h
>@@ -63,7 +63,7 @@ struct igt_device_card {
> uint16_t pci_vendor, pci_device;
> };
>
>-void igt_devices_scan(bool force);
>+void igt_devices_scan(void);
>
> void igt_devices_print(const struct igt_devices_print_format *fmt);
> void igt_devices_print_vendors(void);
>diff --git a/lib/igt_multigpu.c b/lib/igt_multigpu.c
>index be0c113322..d4b89d706a 100644
>--- a/lib/igt_multigpu.c
>+++ b/lib/igt_multigpu.c
>@@ -37,7 +37,7 @@ static int print_gpus(int count, int gpu_num)
>
> igt_info("PCI devices available in the system:\n");
>
>- igt_devices_scan(true);
>+ igt_devices_scan();
> devices = igt_device_filter_pci();
> igt_devices_print(&fmt);
>
>diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
>index 7f17f4423d..18e9dfc346 100644
>--- a/tests/core_hotunplug.c
>+++ b/tests/core_hotunplug.c
>@@ -488,7 +488,7 @@ static bool healthcheck(struct hotunplug *priv, bool recover)
> sleep(1);
>
> /* device name may have changed, rebuild IGT device list */
>- igt_devices_scan(true);
>+ igt_devices_scan();
>
> node_healthcheck(priv, recover ? FLAG_RECOVER : 0);
> if (!priv->failure)
>diff --git a/tests/device_reset.c b/tests/device_reset.c
>index 40681931c2..6c93a96c21 100644
>--- a/tests/device_reset.c
>+++ b/tests/device_reset.c
>@@ -399,7 +399,7 @@ static void healthcheck(struct device_fds *dev)
> /* give the kernel a breath for re-creating device nodes in devtmpfs */
> sleep(1);
> /* refresh device list */
>- igt_devices_scan(true);
>+ igt_devices_scan();
> igt_debug("reopen the device\n");
> dev->fds.dev = __drm_open_driver(DRIVER_ANY);
> }
>diff --git a/tests/intel/i915_suspend.c b/tests/intel/i915_suspend.c
>index 3398b372b5..fae1031a23 100644
>--- a/tests/intel/i915_suspend.c
>+++ b/tests/intel/i915_suspend.c
>@@ -265,7 +265,7 @@ test_suspend_without_i915(int state)
> int fd;
>
> fd = __drm_open_driver(DRIVER_INTEL);
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> /*
> * When module is unloaded and s2idle is triggered, PCI core leaves the endpoint
>@@ -317,7 +317,7 @@ igt_main
> * instead of reloading the i915 module.
> */
> if (igt_device_filter_count())
>- igt_devices_scan(true);
>+ igt_devices_scan();
> fd = drm_open_driver(DRIVER_INTEL);
> }
>
>diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>index a608b894d3..68d27089b9 100644
>--- a/tools/intel_gpu_top.c
>+++ b/tools/intel_gpu_top.c
>@@ -2701,7 +2701,7 @@ int main(int argc, char **argv)
> break;
> };
>
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> if (list_device) {
> struct igt_devices_print_format fmt = {
>diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c
>index 17cd2bc1d6..08c25ca8a6 100644
>--- a/tools/intel_pm_rpm.c
>+++ b/tools/intel_pm_rpm.c
>@@ -142,7 +142,7 @@ int main(int argc, char *argv[])
> }
>
> env_device = getenv("IGT_DEVICE");
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> if (env_device) {
> if (!igt_device_card_match(env_device, &card)) {
>diff --git a/tools/lsgpu.c b/tools/lsgpu.c
>index 2bf2cf5f05..e482ca6b75 100644
>--- a/tools/lsgpu.c
>+++ b/tools/lsgpu.c
>@@ -362,7 +362,7 @@ int main(int argc, char *argv[])
> printf("Notice: Using filter from .igtrc\n");
> }
>
>- igt_devices_scan(false);
>+ igt_devices_scan();
>
> if (igt_device != NULL) {
> struct igt_device_card card;
>--
>2.34.1
>
More information about the igt-dev
mailing list