diff mbox series

[i-g-t,3/3] intel_gpu_top: Free all memory on exit

Message ID 20220527105042.2233128-3-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [i-g-t,1/3] lib/igt_device_scan: Free filtered devices in igt_devices_free | expand

Commit Message

Tvrtko Ursulin May 27, 2022, 10:50 a.m. UTC
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(-)

Comments

Petri Latvala May 30, 2022, 1:17 p.m. UTC | #1
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
>
diff mbox series

Patch

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();