Re: [igt-dev] [PATCH i-g-t] lib: Show other clients when SETMASTER fails

Martin Peres
2019-05-15T19:48:24Z

On 15/05/2019 13:21, Chris Wilson wrote:
> Currently we only show the clients at DEBUG level which is not
> automatically shown for a SKIP. However, failing to SETMASTER is more
> often a mistake and we do want to see the other clients. The choice is
> at what level to show them? If we want to avoid having extra WARNs in
> CI, we should keep them to INFO, but on the hand failing to acquire
> SETMASTER is tantamount to failure, so WARN.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=110682
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Martin Peres <martin.peres@free.fr>

Thanks for doing this!

Acked-by: Martin Peres <martin.peres@linux.intel.com>

> ---
> lib/igt_debugfs.c | 4 ++--
> lib/igt_debugfs.h | 3 ++-
> lib/igt_device.c | 17 +++++++++++++----
> 3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index dd229c099..82ce18344 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -1145,7 +1145,7 @@ int igt_get_stable_obj_count(int driver)
> return obj_count;
> }
>
> -void igt_debugfs_dump(int device, const char *filename)
> +void __igt_debugfs_dump(int device, const char *filename, int level)
> {
> char *contents;
> int dir;
> @@ -1154,6 +1154,6 @@ void igt_debugfs_dump(int device, const char *filename)
> contents = igt_sysfs_get(dir, filename);
> close(dir);
>
> - igt_debug("%s:\n%s\n", filename, contents);
> + igt_log(IGT_LOG_DOMAIN, level, "%s:\n%s\n", filename, contents);
> free(contents);
> }
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index f8e57a6bf..52520b3c5 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -201,6 +201,7 @@ void igt_enable_prefault(void);
> * gem buffer objects
> */
> int igt_get_stable_obj_count(int driver);
> -void igt_debugfs_dump(int device, const char *filename);
> +void __igt_debugfs_dump(int device, const char *filename, int level);
> +#define igt_debugfs_dump(d, f) __igt_debugfs_dump(d, f, IGT_LOG_DEBUG)
>
> #endif /* __IGT_DEBUGFS_H__ */
> diff --git a/lib/igt_device.c b/lib/igt_device.c
> index 08f39c8b1..9469e5de2 100644
> --- a/lib/igt_device.c
> +++ b/lib/igt_device.c
> @@ -32,13 +32,20 @@ int __igt_device_set_master(int fd)
> int err;
>
> err = 0;
> - if (drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL))
> + if (drmIoctl(fd, DRM_IOCTL_SET_MASTER, NULL)) {
> err = -errno;
> + igt_assume(err);
> + }
>
> errno = 0;
> return err;
> }
>
> +static void show_clients(int fd)
> +{
> + __igt_debugfs_dump(fd, "clients", IGT_LOG_WARN);
> +}
> +
> /**
> * igt_device_set_master: Set the device fd to be DRM master
> * @fd: the device
> @@ -48,7 +55,7 @@ int __igt_device_set_master(int fd)
> void igt_device_set_master(int fd)
> {
> if (__igt_device_set_master(fd)) {
> - igt_debugfs_dump(fd, "clients");
> + show_clients(fd);
> igt_require_f(__igt_device_set_master(fd) == 0,
> "Can't become DRM master, "
> "please check if no other DRM client is running.\n");
> @@ -60,8 +67,10 @@ int __igt_device_drop_master(int fd)
> int err;
>
> err = 0;
> - if (drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL))
> + if (drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL)) {
> err = -errno;
> + igt_assume(err);
> + }
>
> errno = 0;
> return err;
> @@ -81,7 +90,7 @@ void igt_device_drop_master(int fd)
> return;
>
> if (__igt_device_drop_master(fd)) {
> - igt_debugfs_dump(fd, "clients");
> + show_clients(fd);
> igt_assert_f(__igt_device_drop_master(fd) == 0,
> "Failed to drop DRM master.\n");
> }
>