[PATCH i-g-t] lib/igt_device_scan: limit fetched device attributes from udev
Kamil Konieczny
kamil.konieczny at linux.intel.com
Thu Jan 30 12:00:54 UTC 2025
Hi Zbigniew,
On 2025-01-30 at 11:27:41 +0100, Zbigniew Kempczyński wrote:
> 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 at intel.com>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
+Cc sriov developers, please wait with merge until you hear
opinion from them
Cc: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
Cc: Lukasz Laguna <lukasz.laguna at intel.com>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> ---
> v2: refactor code a bit (Kamil)
Now it looks better, change is smaller and easy to read.
Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> ---
> 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
>
More information about the igt-dev
mailing list