[Intel-gfx] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Intel-gfx] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Fix a possible oversight.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_device_scan.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 3c23fe0eb520..a30433ae2cff 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -814,6 +814,11 @@ void igt_devices_free(void)
 		igt_device_free(dev);
 		free(dev);
 	}
+
+	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
+		igt_list_del(&dev->link);
+		free(dev);
+	}
 }
 
 /**
-- 
2.32.0

[igt-dev] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Fix a possible oversight.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_device_scan.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 3c23fe0eb520..a30433ae2cff 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -814,6 +814,11 @@ void igt_devices_free(void)
 		igt_device_free(dev);
 		free(dev);
 	}
+
+	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
+		igt_list_del(&dev->link);
+		free(dev);
+	}
 }
 
 /**
-- 
2.32.0

[Intel-gfx] [PATCH i-g-t 2/3] lib/drm_fdinfo: Ensure buffer is null terminated

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Ensure buffer is null terminated at the point where the read ended and not
at the end of the whole buffer. Otherwise string parsing can stray into
un-initialised memory.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_drm_fdinfo.c | 8 ++++----
 lib/igt_drm_fdinfo.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
index b422f67a4ace..250d9e8917f2 100644
--- a/lib/igt_drm_fdinfo.c
+++ b/lib/igt_drm_fdinfo.c
@@ -44,12 +44,12 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
 	if (fd < 0)
 		return 0;
 
-	buf[sz - 1] = 0;
-	count = read(fd, buf, sz);
-	buf[sz - 1] = 0;
+	count = read(fd, buf, sz - 1);
+	if (count > 0)
+		buf[count - 1] = 0;
 	close(fd);
 
-	return count;
+	return count > 0 ? count : 0;
 }
 
 static int parse_engine(char *line, struct drm_client_fdinfo *info,
diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
index 5db63e28b07e..8759471615bd 100644
--- a/lib/igt_drm_fdinfo.h
+++ b/lib/igt_drm_fdinfo.h
@@ -46,7 +46,7 @@ struct drm_client_fdinfo {
  * igt_parse_drm_fdinfo: Parses the drm fdinfo file
  *
  * @drm_fd: DRM file descriptor
- * @info: Structure to populate with read data
+ * @info: Structure to populate with read data. Must be zeroed.
  *
  * Returns the number of valid drm fdinfo keys found or zero if not all
  * mandatory keys were present or no engines found.
@@ -58,7 +58,7 @@ unsigned int igt_parse_drm_fdinfo(int drm_fd, struct drm_client_fdinfo *info);
  *
  * @dir: File descriptor pointing to /proc/pid/fdinfo directory
  * @fd: String representation of the file descriptor number to parse.
- * @info: Structure to populate with read data
+ * @info: Structure to populate with read data. Must be zeroed.
  *
  * Returns the number of valid drm fdinfo keys found or zero if not all
  * mandatory keys were present or no engines found.
-- 
2.32.0

[igt-dev] [PATCH i-g-t 2/3] lib/drm_fdinfo: Ensure buffer is null terminated

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Ensure buffer is null terminated at the point where the read ended and not
at the end of the whole buffer. Otherwise string parsing can stray into
un-initialised memory.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/igt_drm_fdinfo.c | 8 ++++----
 lib/igt_drm_fdinfo.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
index b422f67a4ace..250d9e8917f2 100644
--- a/lib/igt_drm_fdinfo.c
+++ b/lib/igt_drm_fdinfo.c
@@ -44,12 +44,12 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
 	if (fd < 0)
 		return 0;
 
-	buf[sz - 1] = 0;
-	count = read(fd, buf, sz);
-	buf[sz - 1] = 0;
+	count = read(fd, buf, sz - 1);
+	if (count > 0)
+		buf[count - 1] = 0;
 	close(fd);
 
-	return count;
+	return count > 0 ? count : 0;
 }
 
 static int parse_engine(char *line, struct drm_client_fdinfo *info,
diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
index 5db63e28b07e..8759471615bd 100644
--- a/lib/igt_drm_fdinfo.h
+++ b/lib/igt_drm_fdinfo.h
@@ -46,7 +46,7 @@ struct drm_client_fdinfo {
  * igt_parse_drm_fdinfo: Parses the drm fdinfo file
  *
  * @drm_fd: DRM file descriptor
- * @info: Structure to populate with read data
+ * @info: Structure to populate with read data. Must be zeroed.
  *
  * Returns the number of valid drm fdinfo keys found or zero if not all
  * mandatory keys were present or no engines found.
@@ -58,7 +58,7 @@ unsigned int igt_parse_drm_fdinfo(int drm_fd, struct drm_client_fdinfo *info);
  *
  * @dir: File descriptor pointing to /proc/pid/fdinfo directory
  * @fd: String representation of the file descriptor number to parse.
- * @info: Structure to populate with read data
+ * @info: Structure to populate with read data. Must be zeroed.
  *
  * Returns the number of valid drm fdinfo keys found or zero if not all
  * mandatory keys were present or no engines found.
-- 
2.32.0

[Intel-gfx] [PATCH i-g-t 3/3] intel_gpu_top: Free all memory on exit

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Be nice and explicitly free all memory on exit.

Also fix a Valgrind reported unitilised conditional jump.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tools/intel_gpu_top.c | 51 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 26986a822bb7..997aff582ff7 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -437,6 +437,36 @@ static struct engines *discover_engines(char *device)
 	return engines;
 }
 
+static void free_engines(struct engines *engines)
+{
+	struct pmu_counter **pmu, *free_list[] = {
+		&engines->r_gpu,
+		&engines->r_pkg,
+		&engines->imc_reads,
+		&engines->imc_writes,
+		NULL
+	};
+	unsigned int i;
+
+	for (pmu = &free_list[0]; *pmu; pmu++) {
+		if ((*pmu)->present)
+			free((char *)(*pmu)->units);
+	}
+
+	for (i = 0; i < engines->num_engines; i++) {
+		struct engine *engine = engine_ptr(engines, i);
+
+		free((char *)engine->name);
+		free((char *)engine->short_name);
+		free((char *)engine->display_name);
+	}
+
+	closedir(engines->root);
+
+	free(engines->class);
+	free(engines);
+}
+
 #define _open_pmu(type, cnt, pmu, fd) \
 ({ \
 	int fd__; \
@@ -1073,7 +1103,7 @@ static size_t freadat2buf(char *buf, const size_t sz, DIR *at, const char *name)
 	return count;
 }
 
-static struct clients *scan_clients(struct clients *clients)
+static struct clients *scan_clients(struct clients *clients, bool display)
 {
 	struct dirent *proc_dent;
 	struct client *c;
@@ -1181,7 +1211,7 @@ next:
 			break;
 	}
 
-	return display_clients(clients);
+	return display ? display_clients(clients) : clients;
 }
 
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -2391,7 +2421,7 @@ static void process_stdin(unsigned int timeout_us)
 
 static bool has_drm_fdinfo(const struct igt_device_card *card)
 {
-	struct drm_client_fdinfo info;
+	struct drm_client_fdinfo info = { };
 	unsigned int cnt;
 	int fd;
 
@@ -2572,7 +2602,7 @@ int main(int argc, char **argv)
 	}
 
 	pmu_sample(engines);
-	scan_clients(clients);
+	scan_clients(clients, false);
 	codename = igt_device_get_pretty_name(&card, false);
 
 	while (!stop_top) {
@@ -2599,7 +2629,7 @@ int main(int argc, char **argv)
 		pmu_sample(engines);
 		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
 
-		disp_clients = scan_clients(clients);
+		disp_clients = scan_clients(clients, true);
 
 		if (stop_top)
 			break;
@@ -2649,21 +2679,24 @@ int main(int argc, char **argv)
 			pops->close_struct();
 		}
 
-		if (stop_top)
-			break;
-
 		if (disp_clients != clients)
 			free_clients(disp_clients);
 
+		if (stop_top)
+			break;
+
 		if (output_mode == INTERACTIVE)
 			process_stdin(period_us);
 		else
 			usleep(period_us);
 	}
 
+	if (clients)
+		free_clients(clients);
+
 	free(codename);
 err:
-	free(engines);
+	free_engines(engines);
 	free(pmu_device);
 exit:
 	igt_devices_free();
-- 
2.32.0

[igt-dev] [PATCH i-g-t 3/3] intel_gpu_top: Free all memory on exit

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Be nice and explicitly free all memory on exit.

Also fix a Valgrind reported unitilised conditional jump.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tools/intel_gpu_top.c | 51 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 26986a822bb7..997aff582ff7 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -437,6 +437,36 @@ static struct engines *discover_engines(char *device)
 	return engines;
 }
 
+static void free_engines(struct engines *engines)
+{
+	struct pmu_counter **pmu, *free_list[] = {
+		&engines->r_gpu,
+		&engines->r_pkg,
+		&engines->imc_reads,
+		&engines->imc_writes,
+		NULL
+	};
+	unsigned int i;
+
+	for (pmu = &free_list[0]; *pmu; pmu++) {
+		if ((*pmu)->present)
+			free((char *)(*pmu)->units);
+	}
+
+	for (i = 0; i < engines->num_engines; i++) {
+		struct engine *engine = engine_ptr(engines, i);
+
+		free((char *)engine->name);
+		free((char *)engine->short_name);
+		free((char *)engine->display_name);
+	}
+
+	closedir(engines->root);
+
+	free(engines->class);
+	free(engines);
+}
+
 #define _open_pmu(type, cnt, pmu, fd) \
 ({ \
 	int fd__; \
@@ -1073,7 +1103,7 @@ static size_t freadat2buf(char *buf, const size_t sz, DIR *at, const char *name)
 	return count;
 }
 
-static struct clients *scan_clients(struct clients *clients)
+static struct clients *scan_clients(struct clients *clients, bool display)
 {
 	struct dirent *proc_dent;
 	struct client *c;
@@ -1181,7 +1211,7 @@ next:
 			break;
 	}
 
-	return display_clients(clients);
+	return display ? display_clients(clients) : clients;
 }
 
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
@@ -2391,7 +2421,7 @@ static void process_stdin(unsigned int timeout_us)
 
 static bool has_drm_fdinfo(const struct igt_device_card *card)
 {
-	struct drm_client_fdinfo info;
+	struct drm_client_fdinfo info = { };
 	unsigned int cnt;
 	int fd;
 
@@ -2572,7 +2602,7 @@ int main(int argc, char **argv)
 	}
 
 	pmu_sample(engines);
-	scan_clients(clients);
+	scan_clients(clients, false);
 	codename = igt_device_get_pretty_name(&card, false);
 
 	while (!stop_top) {
@@ -2599,7 +2629,7 @@ int main(int argc, char **argv)
 		pmu_sample(engines);
 		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
 
-		disp_clients = scan_clients(clients);
+		disp_clients = scan_clients(clients, true);
 
 		if (stop_top)
 			break;
@@ -2649,21 +2679,24 @@ int main(int argc, char **argv)
 			pops->close_struct();
 		}
 
-		if (stop_top)
-			break;
-
 		if (disp_clients != clients)
 			free_clients(disp_clients);
 
+		if (stop_top)
+			break;
+
 		if (output_mode == INTERACTIVE)
 			process_stdin(period_us);
 		else
 			usleep(period_us);
 	}
 
+	if (clients)
+		free_clients(clients);
+
 	free(codename);
 err:
-	free(engines);
+	free_engines(engines);
 	free(pmu_device);
 exit:
 	igt_devices_free();
-- 
2.32.0

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 13186 bytes --]

== Series Details ==

Series: series starting with [i-g-t,1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free
URL   : https://patchwork.freedesktop.org/series/104456/
State : failure

== Summary ==

CI Bug Log - changes from IGT_6496 -> IGTPW_7180
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_7180 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_7180, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/index.html

Participating hosts (47 -> 45)
------------------------------

  Additional (1): fi-icl-u2 
  Missing    (3): bat-adlm-1 bat-jsl-2 bat-atsm-1 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_7180:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_force_connector_basic@force-connector-state:
    - fi-icl-u2:          NOTRUN -> [WARN][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@kms_force_connector_basic@force-connector-state.html

  
Known issues
------------

  Here are the changes found in IGTPW_7180 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-icl-u2:          NOTRUN -> [SKIP][2] ([i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@random-engines:
    - fi-icl-u2:          NOTRUN -> [SKIP][3] ([i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@gem_lmem_swapping@random-engines.html

  * igt@i915_module_load@reload:
    - fi-cfl-8109u:       [PASS][4] -> [DMESG-WARN][5] ([i915#5904] / [i915#62])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-cfl-8109u/igt@i915_module_load@reload.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-cfl-8109u/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-cfl-8109u:       [PASS][6] -> [DMESG-WARN][7] ([i915#62]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gtt:
    - fi-bdw-5557u:       [PASS][8] -> [INCOMPLETE][9] ([i915#5685])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-bdw-5557u/igt@i915_selftest@live@gtt.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-bdw-5557u/igt@i915_selftest@live@gtt.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [PASS][10] -> [INCOMPLETE][11] ([i915#4785])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-5:          NOTRUN -> [DMESG-FAIL][12] ([i915#4494] / [i915#4957])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-dg1-5/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [PASS][13] -> [DMESG-WARN][14] ([i915#5904]) +35 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_selftest@live@requests:
    - fi-pnv-d510:        [PASS][15] -> [DMESG-FAIL][16] ([i915#4528])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-dg1-5:          NOTRUN -> [INCOMPLETE][17] ([i915#6011])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-dg1-5/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-icl-u2:          NOTRUN -> [SKIP][18] ([i915#5903])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][19] ([fdo#111827]) +8 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_flip@basic-flip-vs-dpms@a-edp1:
    - fi-tgl-u2:          [PASS][20] -> [DMESG-WARN][21] ([i915#402])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html

  * igt@kms_flip@basic-flip-vs-modeset@b-edp1:
    - bat-adlp-4:         [PASS][22] -> [DMESG-WARN][23] ([i915#3576])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html

  * igt@kms_flip@basic-plain-flip@c-dp2:
    - fi-cfl-8109u:       [PASS][24] -> [DMESG-WARN][25] ([i915#165]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-u2:          NOTRUN -> [SKIP][26] ([fdo#109285])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cfl-8109u:       [PASS][27] -> [DMESG-WARN][28] ([i915#165] / [i915#62]) +15 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-icl-u2:          NOTRUN -> [SKIP][29] ([fdo#109278]) +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-icl-u2:          NOTRUN -> [SKIP][30] ([i915#3555])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-u2:          NOTRUN -> [SKIP][31] ([fdo#109295] / [i915#3301])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-icl-u2/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][32] ([fdo#109271] / [i915#4312] / [i915#5594])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-hsw-4770/igt@runner@aborted.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][33] ([fdo#109271] / [i915#2403] / [i915#4312])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-pnv-d510/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_engines:
    - bat-dg1-5:          [INCOMPLETE][34] ([i915#4418]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/bat-dg1-5/igt@i915_selftest@live@gt_engines.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-dg1-5/igt@i915_selftest@live@gt_engines.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - {bat-dg2-8}:        [DMESG-WARN][36] ([i915#5763]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/bat-dg2-8/igt@i915_suspend@basic-s2idle-without-i915.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-dg2-8/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_busy@basic@modeset:
    - bat-adlp-4:         [DMESG-WARN][38] ([i915#3576]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/bat-adlp-4/igt@kms_busy@basic@modeset.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-adlp-4/igt@kms_busy@basic@modeset.html
    - {bat-adlp-6}:       [DMESG-WARN][40] ([i915#3576]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/bat-adlp-6/igt@kms_busy@basic@modeset.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/bat-adlp-6/igt@kms_busy@basic@modeset.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - fi-tgl-u2:          [DMESG-WARN][42] ([i915#402]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6496/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/fi-tgl-u2/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4418]: https://gitlab.freedesktop.org/drm/intel/issues/4418
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#5171]: https://gitlab.freedesktop.org/drm/intel/issues/5171
  [i915#5174]: https://gitlab.freedesktop.org/drm/intel/issues/5174
  [i915#5181]: https://gitlab.freedesktop.org/drm/intel/issues/5181
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
  [i915#5606]: https://gitlab.freedesktop.org/drm/intel/issues/5606
  [i915#5685]: https://gitlab.freedesktop.org/drm/intel/issues/5685
  [i915#5703]: https://gitlab.freedesktop.org/drm/intel/issues/5703
  [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763
  [i915#5775]: https://gitlab.freedesktop.org/drm/intel/issues/5775
  [i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903
  [i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
  [i915#6011]: https://gitlab.freedesktop.org/drm/intel/issues/6011
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6496 -> IGTPW_7180

  CI-20190529: 20190529
  CI_DRM_11705: 18a2e6dbca526f996da04741cf5ef169e810a50e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_7180: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/index.html
  IGT_6496: 58a5a2d5be19b916311541401aaa48b787f9a185 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_7180/index.html

[-- Attachment #2: Type: text/html, Size: 13447 bytes --]

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Zbigniew Kempczyński]

On Fri, May 27, 2022 at 11:50:40AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Fix a possible oversight.

Yes, properly coded in igt_device_scan() only. Thanks for spotting this.

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/igt_device_scan.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 3c23fe0eb520..a30433ae2cff 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -814,6 +814,11 @@ void igt_devices_free(void)
>  		igt_device_free(dev);
>  		free(dev);
>  	}
> +
> +	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
> +		igt_list_del(&dev->link);
> +		free(dev);
> +	}

Small nit - I would change the order (filtered list I would remove first).
igt_device_free() also frees dev->devnode, ... so if we would change the 
code to be more "parallel" it would be better to avoid use-after-free.

With this:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

>  }
>  
>  /**
> -- 
> 2.32.0
> 

Re: [igt-dev] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Zbigniew Kempczyński]

On Fri, May 27, 2022 at 11:50:40AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Fix a possible oversight.

Yes, properly coded in igt_device_scan() only. Thanks for spotting this.

> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/igt_device_scan.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 3c23fe0eb520..a30433ae2cff 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -814,6 +814,11 @@ void igt_devices_free(void)
>  		igt_device_free(dev);
>  		free(dev);
>  	}
> +
> +	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
> +		igt_list_del(&dev->link);
> +		free(dev);
> +	}

Small nit - I would change the order (filtered list I would remove first).
igt_device_free() also frees dev->devnode, ... so if we would change the 
code to be more "parallel" it would be better to avoid use-after-free.

With this:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

>  }
>  
>  /**
> -- 
> 2.32.0
> 

Re: [Intel-gfx] [PATCH i-g-t 2/3] lib/drm_fdinfo: Ensure buffer is null terminated

[Petri Latvala]

On Fri, May 27, 2022 at 11:50:41AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Ensure buffer is null terminated at the point where the read ended and not
> at the end of the whole buffer. Otherwise string parsing can stray into
> un-initialised memory.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

> ---
>  lib/igt_drm_fdinfo.c | 8 ++++----
>  lib/igt_drm_fdinfo.h | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> index b422f67a4ace..250d9e8917f2 100644
> --- a/lib/igt_drm_fdinfo.c
> +++ b/lib/igt_drm_fdinfo.c
> @@ -44,12 +44,12 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
>  	if (fd < 0)
>  		return 0;
>  
> -	buf[sz - 1] = 0;
> -	count = read(fd, buf, sz);
> -	buf[sz - 1] = 0;
> +	count = read(fd, buf, sz - 1);
> +	if (count > 0)
> +		buf[count - 1] = 0;
>  	close(fd);
>  
> -	return count;
> +	return count > 0 ? count : 0;
>  }
>  
>  static int parse_engine(char *line, struct drm_client_fdinfo *info,
> diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
> index 5db63e28b07e..8759471615bd 100644
> --- a/lib/igt_drm_fdinfo.h
> +++ b/lib/igt_drm_fdinfo.h
> @@ -46,7 +46,7 @@ struct drm_client_fdinfo {
>   * igt_parse_drm_fdinfo: Parses the drm fdinfo file
>   *
>   * @drm_fd: DRM file descriptor
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
>   *
>   * Returns the number of valid drm fdinfo keys found or zero if not all
>   * mandatory keys were present or no engines found.
> @@ -58,7 +58,7 @@ unsigned int igt_parse_drm_fdinfo(int drm_fd, struct drm_client_fdinfo *info);
>   *
>   * @dir: File descriptor pointing to /proc/pid/fdinfo directory
>   * @fd: String representation of the file descriptor number to parse.
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
>   *
>   * Returns the number of valid drm fdinfo keys found or zero if not all
>   * mandatory keys were present or no engines found.
> -- 
> 2.32.0
> 

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 2/3] lib/drm_fdinfo: Ensure buffer is null terminated

[Petri Latvala]

On Fri, May 27, 2022 at 11:50:41AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Ensure buffer is null terminated at the point where the read ended and not
> at the end of the whole buffer. Otherwise string parsing can stray into
> un-initialised memory.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

> ---
>  lib/igt_drm_fdinfo.c | 8 ++++----
>  lib/igt_drm_fdinfo.h | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
> index b422f67a4ace..250d9e8917f2 100644
> --- a/lib/igt_drm_fdinfo.c
> +++ b/lib/igt_drm_fdinfo.c
> @@ -44,12 +44,12 @@ static size_t read_fdinfo(char *buf, const size_t sz, int at, const char *name)
>  	if (fd < 0)
>  		return 0;
>  
> -	buf[sz - 1] = 0;
> -	count = read(fd, buf, sz);
> -	buf[sz - 1] = 0;
> +	count = read(fd, buf, sz - 1);
> +	if (count > 0)
> +		buf[count - 1] = 0;
>  	close(fd);
>  
> -	return count;
> +	return count > 0 ? count : 0;
>  }
>  
>  static int parse_engine(char *line, struct drm_client_fdinfo *info,
> diff --git a/lib/igt_drm_fdinfo.h b/lib/igt_drm_fdinfo.h
> index 5db63e28b07e..8759471615bd 100644
> --- a/lib/igt_drm_fdinfo.h
> +++ b/lib/igt_drm_fdinfo.h
> @@ -46,7 +46,7 @@ struct drm_client_fdinfo {
>   * igt_parse_drm_fdinfo: Parses the drm fdinfo file
>   *
>   * @drm_fd: DRM file descriptor
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
>   *
>   * Returns the number of valid drm fdinfo keys found or zero if not all
>   * mandatory keys were present or no engines found.
> @@ -58,7 +58,7 @@ unsigned int igt_parse_drm_fdinfo(int drm_fd, struct drm_client_fdinfo *info);
>   *
>   * @dir: File descriptor pointing to /proc/pid/fdinfo directory
>   * @fd: String representation of the file descriptor number to parse.
> - * @info: Structure to populate with read data
> + * @info: Structure to populate with read data. Must be zeroed.
>   *
>   * Returns the number of valid drm fdinfo keys found or zero if not all
>   * mandatory keys were present or no engines found.
> -- 
> 2.32.0
> 

Re: [Intel-gfx] [PATCH i-g-t 3/3] intel_gpu_top: Free all memory on exit

[Petri Latvala]

On Fri, May 27, 2022 at 11:50:42AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Be nice and explicitly free all memory on exit.
> 
> Also fix a Valgrind reported unitilised conditional jump.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

> ---
>  tools/intel_gpu_top.c | 51 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 26986a822bb7..997aff582ff7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -437,6 +437,36 @@ static struct engines *discover_engines(char *device)
>  	return engines;
>  }
>  
> +static void free_engines(struct engines *engines)
> +{
> +	struct pmu_counter **pmu, *free_list[] = {
> +		&engines->r_gpu,
> +		&engines->r_pkg,
> +		&engines->imc_reads,
> +		&engines->imc_writes,
> +		NULL
> +	};
> +	unsigned int i;
> +
> +	for (pmu = &free_list[0]; *pmu; pmu++) {
> +		if ((*pmu)->present)
> +			free((char *)(*pmu)->units);
> +	}
> +
> +	for (i = 0; i < engines->num_engines; i++) {
> +		struct engine *engine = engine_ptr(engines, i);
> +
> +		free((char *)engine->name);
> +		free((char *)engine->short_name);
> +		free((char *)engine->display_name);
> +	}
> +
> +	closedir(engines->root);
> +
> +	free(engines->class);
> +	free(engines);
> +}
> +
>  #define _open_pmu(type, cnt, pmu, fd) \
>  ({ \
>  	int fd__; \
> @@ -1073,7 +1103,7 @@ static size_t freadat2buf(char *buf, const size_t sz, DIR *at, const char *name)
>  	return count;
>  }
>  
> -static struct clients *scan_clients(struct clients *clients)
> +static struct clients *scan_clients(struct clients *clients, bool display)
>  {
>  	struct dirent *proc_dent;
>  	struct client *c;
> @@ -1181,7 +1211,7 @@ next:
>  			break;
>  	}
>  
> -	return display_clients(clients);
> +	return display ? display_clients(clients) : clients;
>  }
>  
>  static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -2391,7 +2421,7 @@ static void process_stdin(unsigned int timeout_us)
>  
>  static bool has_drm_fdinfo(const struct igt_device_card *card)
>  {
> -	struct drm_client_fdinfo info;
> +	struct drm_client_fdinfo info = { };
>  	unsigned int cnt;
>  	int fd;
>  
> @@ -2572,7 +2602,7 @@ int main(int argc, char **argv)
>  	}
>  
>  	pmu_sample(engines);
> -	scan_clients(clients);
> +	scan_clients(clients, false);
>  	codename = igt_device_get_pretty_name(&card, false);
>  
>  	while (!stop_top) {
> @@ -2599,7 +2629,7 @@ int main(int argc, char **argv)
>  		pmu_sample(engines);
>  		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>  
> -		disp_clients = scan_clients(clients);
> +		disp_clients = scan_clients(clients, true);
>  
>  		if (stop_top)
>  			break;
> @@ -2649,21 +2679,24 @@ int main(int argc, char **argv)
>  			pops->close_struct();
>  		}
>  
> -		if (stop_top)
> -			break;
> -
>  		if (disp_clients != clients)
>  			free_clients(disp_clients);
>  
> +		if (stop_top)
> +			break;
> +
>  		if (output_mode == INTERACTIVE)
>  			process_stdin(period_us);
>  		else
>  			usleep(period_us);
>  	}
>  
> +	if (clients)
> +		free_clients(clients);
> +
>  	free(codename);
>  err:
> -	free(engines);
> +	free_engines(engines);
>  	free(pmu_device);
>  exit:
>  	igt_devices_free();
> -- 
> 2.32.0
> 

Re: [igt-dev] [PATCH i-g-t 3/3] intel_gpu_top: Free all memory on exit

[Petri Latvala]

On Fri, May 27, 2022 at 11:50:42AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Be nice and explicitly free all memory on exit.
> 
> Also fix a Valgrind reported unitilised conditional jump.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

> ---
>  tools/intel_gpu_top.c | 51 +++++++++++++++++++++++++++++++++++--------
>  1 file changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 26986a822bb7..997aff582ff7 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -437,6 +437,36 @@ static struct engines *discover_engines(char *device)
>  	return engines;
>  }
>  
> +static void free_engines(struct engines *engines)
> +{
> +	struct pmu_counter **pmu, *free_list[] = {
> +		&engines->r_gpu,
> +		&engines->r_pkg,
> +		&engines->imc_reads,
> +		&engines->imc_writes,
> +		NULL
> +	};
> +	unsigned int i;
> +
> +	for (pmu = &free_list[0]; *pmu; pmu++) {
> +		if ((*pmu)->present)
> +			free((char *)(*pmu)->units);
> +	}
> +
> +	for (i = 0; i < engines->num_engines; i++) {
> +		struct engine *engine = engine_ptr(engines, i);
> +
> +		free((char *)engine->name);
> +		free((char *)engine->short_name);
> +		free((char *)engine->display_name);
> +	}
> +
> +	closedir(engines->root);
> +
> +	free(engines->class);
> +	free(engines);
> +}
> +
>  #define _open_pmu(type, cnt, pmu, fd) \
>  ({ \
>  	int fd__; \
> @@ -1073,7 +1103,7 @@ static size_t freadat2buf(char *buf, const size_t sz, DIR *at, const char *name)
>  	return count;
>  }
>  
> -static struct clients *scan_clients(struct clients *clients)
> +static struct clients *scan_clients(struct clients *clients, bool display)
>  {
>  	struct dirent *proc_dent;
>  	struct client *c;
> @@ -1181,7 +1211,7 @@ next:
>  			break;
>  	}
>  
> -	return display_clients(clients);
> +	return display ? display_clients(clients) : clients;
>  }
>  
>  static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> @@ -2391,7 +2421,7 @@ static void process_stdin(unsigned int timeout_us)
>  
>  static bool has_drm_fdinfo(const struct igt_device_card *card)
>  {
> -	struct drm_client_fdinfo info;
> +	struct drm_client_fdinfo info = { };
>  	unsigned int cnt;
>  	int fd;
>  
> @@ -2572,7 +2602,7 @@ int main(int argc, char **argv)
>  	}
>  
>  	pmu_sample(engines);
> -	scan_clients(clients);
> +	scan_clients(clients, false);
>  	codename = igt_device_get_pretty_name(&card, false);
>  
>  	while (!stop_top) {
> @@ -2599,7 +2629,7 @@ int main(int argc, char **argv)
>  		pmu_sample(engines);
>  		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>  
> -		disp_clients = scan_clients(clients);
> +		disp_clients = scan_clients(clients, true);
>  
>  		if (stop_top)
>  			break;
> @@ -2649,21 +2679,24 @@ int main(int argc, char **argv)
>  			pops->close_struct();
>  		}
>  
> -		if (stop_top)
> -			break;
> -
>  		if (disp_clients != clients)
>  			free_clients(disp_clients);
>  
> +		if (stop_top)
> +			break;
> +
>  		if (output_mode == INTERACTIVE)
>  			process_stdin(period_us);
>  		else
>  			usleep(period_us);
>  	}
>  
> +	if (clients)
> +		free_clients(clients);
> +
>  	free(codename);
>  err:
> -	free(engines);
> +	free_engines(engines);
>  	free(pmu_device);
>  exit:
>  	igt_devices_free();
> -- 
> 2.32.0
> 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Petri Latvala]

On Mon, May 30, 2022 at 04:40:06AM +0200, Zbigniew Kempczyński wrote:
> On Fri, May 27, 2022 at 11:50:40AM +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Fix a possible oversight.
> 
> Yes, properly coded in igt_device_scan() only. Thanks for spotting this.
> 
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >  lib/igt_device_scan.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 3c23fe0eb520..a30433ae2cff 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -814,6 +814,11 @@ void igt_devices_free(void)
> >  		igt_device_free(dev);
> >  		free(dev);
> >  	}
> > +
> > +	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
> > +		igt_list_del(&dev->link);
> > +		free(dev);
> > +	}
> 
> Small nit - I would change the order (filtered list I would remove first).
> igt_device_free() also frees dev->devnode, ... so if we would change the 
> code to be more "parallel" it would be better to avoid use-after-free.
> 
> With this:
> 
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Tvrtko is away this week so I made this change and merged.


-- 
Petri Latvala


> 
> --
> Zbigniew
> 
> >  }
> >  
> >  /**
> > -- 
> > 2.32.0
> > 

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free

[Petri Latvala]

On Mon, May 30, 2022 at 04:40:06AM +0200, Zbigniew Kempczyński wrote:
> On Fri, May 27, 2022 at 11:50:40AM +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Fix a possible oversight.
> 
> Yes, properly coded in igt_device_scan() only. Thanks for spotting this.
> 
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >  lib/igt_device_scan.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index 3c23fe0eb520..a30433ae2cff 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -814,6 +814,11 @@ void igt_devices_free(void)
> >  		igt_device_free(dev);
> >  		free(dev);
> >  	}
> > +
> > +	igt_list_for_each_entry_safe(dev, tmp, &igt_devs.filtered, link) {
> > +		igt_list_del(&dev->link);
> > +		free(dev);
> > +	}
> 
> Small nit - I would change the order (filtered list I would remove first).
> igt_device_free() also frees dev->devnode, ... so if we would change the 
> code to be more "parallel" it would be better to avoid use-after-free.
> 
> With this:
> 
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Tvrtko is away this week so I made this change and merged.


-- 
Petri Latvala


> 
> --
> Zbigniew
> 
> >  }
> >  
> >  /**
> > -- 
> > 2.32.0
> >