[igt-dev] [PATCH RFC 0/2] add sysfs node at subtest results when available

[igt-dev] [PATCH RFC 0/2] add sysfs node at subtest results when available

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

IGT already supports opening more than one GPU and filtering them. However,
on machines with more than one CPU, it is hard to identify what was the
one that had an IGT test report results.

In order to minimize that, store the sysfs name of the opened devnode,
as it contains the bus address of the device and the card number, which
should be unique inside a machine.

Mauro Carvalho Chehab (2):
  lib/igt_core: add a logic to store a GPU string
  lib/igt_core: print card sysfs node at subtest results

 lib/drmtest.c  |  4 +++-
 lib/igt_core.c | 27 +++++++++++++++++++++++++++
 lib/igt_core.h |  6 ++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

-- 
2.37.3

[igt-dev] [PATCH RFC 1/2] lib/igt_core: add a logic to store a GPU string

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

This is helpful on tests that run with multiple GPUs.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/drmtest.c  |  4 +++-
 lib/igt_core.c | 24 ++++++++++++++++++++++++
 lib/igt_core.h |  6 ++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 16e80bdfcfb1..5eb9827278c8 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -222,8 +222,10 @@ static int open_device(const char *name, unsigned int chipset)
 			break;
 		}
 	}
-	if ((chipset & chip) == chip)
+	if ((chipset & chip) == chip) {
+		set_gpu_string(name);
 		return fd;
+	}
 
 err:
 	close(fd);
diff --git a/lib/igt_core.c b/lib/igt_core.c
index e7425326b7f0..0d018758ce38 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1447,6 +1447,30 @@ bool __igt_enter_dynamic_container(void)
 	return true;
 }
 
+static __thread char *gpu_string = NULL;
+
+void set_gpu_string(const char *fname)
+{
+	char sysfs[PATH_MAX], *path, *p;
+
+
+	p = strrchr(fname, '/');
+	if (!p) {
+		path = strdup(fname);
+	} else {
+		p++;
+
+		strcpy(sysfs, "/sys/class/drm/");
+		strcat(sysfs, p);
+		path = realpath(sysfs, NULL);
+	}
+
+	if (gpu_string)
+		free(gpu_string);
+	gpu_string = path;
+	igt_assert(gpu_string);
+}
+
 __noreturn static void exit_subtest(const char *result)
 {
 	struct timespec now;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index aa98e8ed8deb..62e7065429c9 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -1490,4 +1490,10 @@ static inline void igt_pci_system_cleanup(void)
 {
 }
 
+/**
+ * set_gpu_string():
+ * Sets a string to describe the GPU being tested
+ */
+void set_gpu_string(const char *string);
+
 #endif /* IGT_CORE_H */
-- 
2.37.3

[igt-dev] [PATCH RFC 2/2] lib/igt_core: print card sysfs node at subtest results

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

This helps to identify what GPU failed on multi-GPU tests.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 0d018758ce38..6575eabca027 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1481,6 +1481,9 @@ __noreturn static void exit_subtest(const char *result)
 
 	igt_gettime(&now);
 
+	if (gpu_string)
+		igt_info("%s: ", gpu_string);
+
 	igt_info("%s%s %s: %s (%.3fs)%s\n",
 		 (!__igt_plain_output) ? "\x1b[1m" : "",
 		 subtest_text, *subtest_name, result,
-- 
2.37.3

Re: [igt-dev] [PATCH RFC 2/2] lib/igt_core: print card sysfs node at subtest results

[Mauro Carvalho Chehab]

On Wed, 28 Sep 2022 18:26:50 +0200
Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:

> Hi Mauro,
> 
> On 2022-09-28 at 12:43:20 +0300, Petri Latvala wrote:
> > On Wed, Sep 28, 2022 at 10:58:42AM +0200, Mauro Carvalho Chehab wrote:  
> > > From: Mauro Carvalho Chehab <mchehab@kernel.org>
> > > 
> > > This helps to identify what GPU failed on multi-GPU tests.  
> > 
> > And completely breaks igt_runner when it tries to parse the result
> > logs.

Yes. That's btw one of the reasons why I submitted it as a RFC: depending
on how CI check IGT logs, it can break the test results there as well.

Anyway, this is fixable.

The point is, once we have multi-GPU tests, we need to have a way to
report what GPU failed.

Once integrated with some upcoming multi-gpu support patch series,
we can enable this log only if multi-gpu tests are enabled.

On other words, I expect that this series, and in particular patch 2/2,
will be changed and sent together on its final version with the 
multi-GPU patch series that Kamil has been working with.

> First thing is to decide where such info should be printed,
> imho it is worth to have it with maybe some new IGT_PRINT_GPU
> environment var ?
> This var can configure how it will work, what it will store,
> where to print message about used GPU, on begin of error message
> or at end.
>
> If not defined, we should not print any additional info.

No need to have a separate IGT_PRINT_GPU, as the additional GPU log data
is only needed when more than one GPUs is used. So, it can be enabled based
on IGT_DEVICE selection and/or --device parameter, at the tests that use 
multiple GPUs.

> btw I did not received you RFC from mailist server,
> are there any known problems with ML ?

Weird. The series reached patchwork: https://patchwork.freedesktop.org/series/109171/
Yet, as you were c/c, maybe the mail list server decided to exclude
you from its own c/c (this is configurable on some mailing list servers).

> Second thought, after reading RFC on patchwork, please put your
> comments from cover letter into commit description in first
> patch so it will be in git history.

Ok. Btw, if you prefer to place this patch together with the next
version of your multi-gpu RFC, feel free to do so and edit the
patch descriptions to better fit the needs.

Regards,
Mauro

Re: [igt-dev] [PATCH RFC 2/2] lib/igt_core: print card sysfs node at subtest results

[Mauro Carvalho Chehab]

On 9/29/22 12:06, Petri Latvala wrote:
> On Thu, Sep 29, 2022 at 10:21:36AM +0200, Mauro Carvalho Chehab wrote:
>> On Wed, 28 Sep 2022 18:26:50 +0200
>> Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
>>
>>> Hi Mauro,
>>>
>>> On 2022-09-28 at 12:43:20 +0300, Petri Latvala wrote:
>>>> On Wed, Sep 28, 2022 at 10:58:42AM +0200, Mauro Carvalho Chehab wrote:
>>>>> From: Mauro Carvalho Chehab <mchehab@kernel.org>
>>>>>
>>>>> This helps to identify what GPU failed on multi-GPU tests.
>>>> And completely breaks igt_runner when it tries to parse the result
>>>> logs.
>> Yes. That's btw one of the reasons why I submitted it as a RFC: depending
>> on how CI check IGT logs, it can break the test results there as well.
>>
>> Anyway, this is fixable.
>>
>> The point is, once we have multi-GPU tests, we need to have a way to
>> report what GPU failed.
> Imho it's best printed in __igt_fail_assert to get it close to the
> reason why the test is failing.

Yeah, it makes sense to have it there, but, IMO it should also be at the 
result
output, as we could have a test that it skipped on some GPUs and/or fail on
some GPUs while passing on others.


Regards,
Mauro