Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] lib/igt_device_scan: limit fetched device attributes from udev
@ 2025-01-30 10:27 Zbigniew Kempczyński
  2025-01-30 12:00 ` Kamil Konieczny
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Zbigniew Kempczyński @ 2025-01-30 10:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Zbigniew Kempczyński, Lucas De Marchi, Kamil Konieczny

Tests don't need all attributes so default device scanning is now
limited to small list directly used in device filters. To be backward
compatible tools like lsgpu still may scan whole attribute list what
keeps this behavior intact.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
v2: refactor code a bit (Kamil)
---
 lib/igt_device_scan.c | 50 ++++++++++++++++++++++++++++++++++---------
 lib/igt_device_scan.h |  1 +
 tools/lsgpu.c         |  2 +-
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index c36b0efa90..711bedc5c8 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include "drmtest.h"
 #include "igt_core.h"
 #include "igt_device_scan.h"
 #include "igt_list.h"
@@ -206,6 +207,8 @@ enum dev_type {
 #define STR_INTEGRATED "integrated"
 #define STR_DISCRETE "discrete"
 
+static const char * const attrs[] = { "driver", "sriov_numvfs", "physfn" };
+
 static inline bool strequal(const char *a, const char *b)
 {
 	if (a == NULL || b == NULL)
@@ -548,7 +551,7 @@ static void get_props(struct udev_device *dev, struct igt_device *idev)
  * Function skips sysattrs from blacklist ht (acquiring some values can take
  * seconds).
  */
-static void get_attrs(struct udev_device *dev, struct igt_device *idev)
+static void get_attrs_all(struct udev_device *dev, struct igt_device *idev)
 {
 	struct udev_list_entry *entry;
 
@@ -569,6 +572,19 @@ static void get_attrs(struct udev_device *dev, struct igt_device *idev)
 	}
 }
 
+static void get_attrs_limited(struct udev_device *dev, struct igt_device *idev)
+{
+	const char *value;
+
+	for (int i = 0; i < ARRAY_SIZE(attrs); i++) {
+		value = udev_device_get_sysattr_value(dev, attrs[i]);
+		if (!value)
+			continue;
+		igt_device_add_attr(idev, attrs[i], value);
+		DBG("attr: %s, val: %s\n", attrs[i], value);
+	}
+}
+
 #define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
 #define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
 #define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
@@ -639,7 +655,8 @@ static char* strdup_nullsafe(const char* str)
  * Fills structure with most usable udev device variables, properties
  * and sysattrs.
  */
-static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
+static struct igt_device *igt_device_new_from_udev(struct udev_device *dev,
+						   bool limit_attrs)
 {
 	struct igt_device *idev = igt_device_new();
 
@@ -654,7 +671,8 @@ static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
 		idev->drm_render = strdup(idev->devnode);
 
 	get_props(dev, idev);
-	get_attrs(dev, idev);
+	limit_attrs ? get_attrs_limited(dev, idev) :
+		      get_attrs_all(dev, idev);
 
 	if (is_pci_subsystem(idev)) {
 		uint16_t vendor, device;
@@ -868,7 +886,8 @@ static struct igt_device *igt_device_from_syspath(const char *syspath)
  */
 static void update_or_add_parent(struct udev *udev,
 				 struct udev_device *dev,
-				 struct igt_device *idev)
+				 struct igt_device *idev,
+				 bool limit_attrs)
 {
 	struct udev_device *parent_dev;
 	struct igt_device *parent_idev;
@@ -899,7 +918,7 @@ static void update_or_add_parent(struct udev *udev,
 		 * return fresh udev device.
 		 */
 		parent_dev = udev_device_new_from_syspath(udev, syspath);
-		parent_idev = igt_device_new_from_udev(parent_dev);
+		parent_idev = igt_device_new_from_udev(parent_dev, limit_attrs);
 		udev_device_unref(parent_dev);
 
 		if (parent_idev)
@@ -1000,7 +1019,7 @@ static void index_pci_devices(void)
  * Function sorts all found devices to keep same order of bus devices
  * for providing predictable search.
  */
-static void scan_drm_devices(void)
+static void scan_drm_devices(bool limit_attrs)
 {
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
@@ -1035,9 +1054,9 @@ static void scan_drm_devices(void)
 
 		path = udev_list_entry_get_name(dev_list_entry);
 		udev_dev = udev_device_new_from_syspath(udev, path);
-		idev = igt_device_new_from_udev(udev_dev);
+		idev = igt_device_new_from_udev(udev_dev, limit_attrs);
 		igt_list_add_tail(&idev->link, &igt_devs.all);
-		update_or_add_parent(udev, udev_dev, idev);
+		update_or_add_parent(udev, udev_dev, idev, limit_attrs);
 
 		udev_device_unref(udev_dev);
 	}
@@ -1092,17 +1111,28 @@ void igt_devices_free(void)
  *
  * Function scans udev in search of gpu devices.
  */
-void igt_devices_scan(void)
+
+static void __igt_devices_scan(bool limit_attrs)
 {
 	if (igt_devs.devs_scanned)
 		igt_devices_free();
 
 	prepare_scan();
-	scan_drm_devices();
+	scan_drm_devices(limit_attrs);
 
 	igt_devs.devs_scanned = true;
 }
 
+void igt_devices_scan(void)
+{
+	__igt_devices_scan(true);
+}
+
+void igt_devices_scan_all_attrs(void)
+{
+	__igt_devices_scan(false);
+}
+
 static inline void _pr_simple(const char *k, const char *v)
 {
 	printf("    %-16s: %s\n", k, v);
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index fa90134aa3..92741fe3c7 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -64,6 +64,7 @@ struct igt_device_card {
 };
 
 void igt_devices_scan(void);
+void igt_devices_scan_all_attrs(void);
 
 void igt_devices_print(const struct igt_devices_print_format *fmt);
 void igt_devices_print_vendors(void);
diff --git a/tools/lsgpu.c b/tools/lsgpu.c
index e482ca6b75..e683900833 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();
+	igt_devices_scan_all_attrs();
 
 	if (igt_device != NULL) {
 		struct igt_device_card card;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH i-g-t] lib/igt_device_scan: limit fetched device attributes from udev
@ 2025-01-28  8:30 Zbigniew Kempczyński
  2025-01-29 14:48 ` Kamil Konieczny
  0 siblings, 1 reply; 12+ messages in thread
From: Zbigniew Kempczyński @ 2025-01-28  8:30 UTC (permalink / raw)
  To: igt-dev; +Cc: Zbigniew Kempczyński, Lucas De Marchi, Kamil Konieczny

Tests don't need all attributes so default device scanning is now
limited to small list directly used in device filters. To be backward
compatible tools like lsgpu still may scan whole attribute list what
keeps this behavior intact.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/igt_device_scan.c | 69 ++++++++++++++++++++++++++++++-------------
 lib/igt_device_scan.h |  1 +
 tools/lsgpu.c         |  2 +-
 3 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index c36b0efa90..15f6bf5728 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include "drmtest.h"
 #include "igt_core.h"
 #include "igt_device_scan.h"
 #include "igt_list.h"
@@ -206,6 +207,8 @@ enum dev_type {
 #define STR_INTEGRATED "integrated"
 #define STR_DISCRETE "discrete"
 
+static const char * const attrs[] = { "driver", "sriov_numvfs", "physfn" };
+
 static inline bool strequal(const char *a, const char *b)
 {
 	if (a == NULL || b == NULL)
@@ -548,24 +551,35 @@ static void get_props(struct udev_device *dev, struct igt_device *idev)
  * Function skips sysattrs from blacklist ht (acquiring some values can take
  * seconds).
  */
-static void get_attrs(struct udev_device *dev, struct igt_device *idev)
+static void get_attrs(struct udev_device *dev, struct igt_device *idev,
+		      bool limit_attrs)
 {
 	struct udev_list_entry *entry;
+	const char *value;
 
-	entry = udev_device_get_sysattr_list_entry(dev);
-	while (entry) {
-		const char *key = udev_list_entry_get_name(entry);
-		const char *value;
+	if (limit_attrs) {
+		for (int i = 0; i < ARRAY_SIZE(attrs); i++) {
+			value = udev_device_get_sysattr_value(dev, attrs[i]);
+			if (!value)
+				continue;
+			igt_device_add_attr(idev, attrs[i], value);
+			DBG("attr: %s, val: %s\n", attrs[i], value);
+		}
+	} else {
+		entry = udev_device_get_sysattr_list_entry(dev);
+		while (entry) {
+			const char *key = udev_list_entry_get_name(entry);
 
-		if (is_on_blacklist(key)) {
+			if (is_on_blacklist(key)) {
+				entry = udev_list_entry_get_next(entry);
+				continue;
+			}
+
+			value = udev_device_get_sysattr_value(dev, key);
+			igt_device_add_attr(idev, key, value);
 			entry = udev_list_entry_get_next(entry);
-			continue;
+			DBG("attr: %s, val: %s\n", key, value);
 		}
-
-		value = udev_device_get_sysattr_value(dev, key);
-		igt_device_add_attr(idev, key, value);
-		entry = udev_list_entry_get_next(entry);
-		DBG("attr: %s, val: %s\n", key, value);
 	}
 }
 
@@ -639,7 +653,8 @@ static char* strdup_nullsafe(const char* str)
  * Fills structure with most usable udev device variables, properties
  * and sysattrs.
  */
-static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
+static struct igt_device *igt_device_new_from_udev(struct udev_device *dev,
+						   bool limit_attrs)
 {
 	struct igt_device *idev = igt_device_new();
 
@@ -654,7 +669,7 @@ static struct igt_device *igt_device_new_from_udev(struct udev_device *dev)
 		idev->drm_render = strdup(idev->devnode);
 
 	get_props(dev, idev);
-	get_attrs(dev, idev);
+	get_attrs(dev, idev, limit_attrs);
 
 	if (is_pci_subsystem(idev)) {
 		uint16_t vendor, device;
@@ -868,7 +883,8 @@ static struct igt_device *igt_device_from_syspath(const char *syspath)
  */
 static void update_or_add_parent(struct udev *udev,
 				 struct udev_device *dev,
-				 struct igt_device *idev)
+				 struct igt_device *idev,
+				 bool limit_attrs)
 {
 	struct udev_device *parent_dev;
 	struct igt_device *parent_idev;
@@ -899,7 +915,7 @@ static void update_or_add_parent(struct udev *udev,
 		 * return fresh udev device.
 		 */
 		parent_dev = udev_device_new_from_syspath(udev, syspath);
-		parent_idev = igt_device_new_from_udev(parent_dev);
+		parent_idev = igt_device_new_from_udev(parent_dev, limit_attrs);
 		udev_device_unref(parent_dev);
 
 		if (parent_idev)
@@ -1000,7 +1016,7 @@ static void index_pci_devices(void)
  * Function sorts all found devices to keep same order of bus devices
  * for providing predictable search.
  */
-static void scan_drm_devices(void)
+static void scan_drm_devices(bool limit_attrs)
 {
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
@@ -1035,9 +1051,9 @@ static void scan_drm_devices(void)
 
 		path = udev_list_entry_get_name(dev_list_entry);
 		udev_dev = udev_device_new_from_syspath(udev, path);
-		idev = igt_device_new_from_udev(udev_dev);
+		idev = igt_device_new_from_udev(udev_dev, limit_attrs);
 		igt_list_add_tail(&idev->link, &igt_devs.all);
-		update_or_add_parent(udev, udev_dev, idev);
+		update_or_add_parent(udev, udev_dev, idev, limit_attrs);
 
 		udev_device_unref(udev_dev);
 	}
@@ -1092,17 +1108,28 @@ void igt_devices_free(void)
  *
  * Function scans udev in search of gpu devices.
  */
-void igt_devices_scan(void)
+
+static void __igt_devices_scan(bool limit_attrs)
 {
 	if (igt_devs.devs_scanned)
 		igt_devices_free();
 
 	prepare_scan();
-	scan_drm_devices();
+	scan_drm_devices(limit_attrs);
 
 	igt_devs.devs_scanned = true;
 }
 
+void igt_devices_scan(void)
+{
+	__igt_devices_scan(true);
+}
+
+void igt_devices_scan_all_attrs(void)
+{
+	__igt_devices_scan(false);
+}
+
 static inline void _pr_simple(const char *k, const char *v)
 {
 	printf("    %-16s: %s\n", k, v);
diff --git a/lib/igt_device_scan.h b/lib/igt_device_scan.h
index fa90134aa3..92741fe3c7 100644
--- a/lib/igt_device_scan.h
+++ b/lib/igt_device_scan.h
@@ -64,6 +64,7 @@ struct igt_device_card {
 };
 
 void igt_devices_scan(void);
+void igt_devices_scan_all_attrs(void);
 
 void igt_devices_print(const struct igt_devices_print_format *fmt);
 void igt_devices_print_vendors(void);
diff --git a/tools/lsgpu.c b/tools/lsgpu.c
index e482ca6b75..e683900833 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();
+	igt_devices_scan_all_attrs();
 
 	if (igt_device != NULL) {
 		struct igt_device_card card;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-02-03  8:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-30 10:27 [PATCH i-g-t] lib/igt_device_scan: limit fetched device attributes from udev Zbigniew Kempczyński
2025-01-30 12:00 ` Kamil Konieczny
2025-02-03  8:04   ` Bernatowicz, Marcin
2025-01-31  7:06 ` ✓ Xe.CI.BAT: success for lib/igt_device_scan: limit fetched device attributes from udev (rev3) Patchwork
2025-01-31  7:32 ` ✓ i915.CI.BAT: " Patchwork
2025-01-31  8:54 ` ✗ i915.CI.Full: failure " Patchwork
2025-02-03  6:06   ` Zbigniew Kempczyński
2025-01-31  9:11 ` ✓ Xe.CI.Full: success " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2025-01-28  8:30 [PATCH i-g-t] lib/igt_device_scan: limit fetched device attributes from udev Zbigniew Kempczyński
2025-01-29 14:48 ` Kamil Konieczny
2025-01-30 10:17   ` Zbigniew Kempczyński
2025-01-30 10:20   ` Zbigniew Kempczyński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox