[Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Steve Hampson]

A buffer in function test_flink_name was both too small and never
checked for overflow.  Both errors are fixed.

Signed-off-by: Steve Hampson <steven.t.hampson@intel.com>
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 tests/i915/gem_flink_race.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/i915/gem_flink_race.c b/tests/i915/gem_flink_race.c
index c1f5d5d51..cf07aedf1 100644
--- a/tests/i915/gem_flink_race.c
+++ b/tests/i915/gem_flink_race.c
@@ -83,7 +83,7 @@ static void test_flink_name(int timeout)
 	struct flink_name *threads;
 	int r, i, num_threads;
 	unsigned long count;
-	char buf[256];
+	char buf[512];
 	void *status;
 	int len;
 
@@ -118,9 +118,13 @@ static void test_flink_name(int timeout)
 	for (i = 0;  i < num_threads; i++) {
 		pthread_join(threads[i].thread, &status);
 		igt_assert(status == 0);
-		len += snprintf(buf + len, sizeof(buf) - len, "%lu, ", threads[i].count);
+		/* Below, constant 11 is 8 digit number, comma, space and null byte */
+		if ((len + 11 + 1) < sizeof(buf))
+			len += snprintf(buf + len, sizeof(buf) - len, "%8lu, ", threads[i].count);
 	}
-	snprintf(buf + len - 2, sizeof(buf) - len + 2, "] races");
+	/* Below, constant 9 is 7 bytes for terminating string plus \n and null byte */
+	if (len + 9 < sizeof(buf))
+		snprintf(buf + len - 2, sizeof(buf) - len + 2, "] races");
 	igt_info("%s\n", buf);
 
 	close(fd);
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Chris Wilson]

Quoting Steve Hampson (2020-11-17 22:23:08)
> A buffer in function test_flink_name was both too small and never
> checked for overflow.  Both errors are fixed.

That many numbers is not interesting. Show the range and median instead.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Hampson, Steven T]

Sent from my iPhone

> On Nov 17, 2020, at 2:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> Quoting Steve Hampson (2020-11-17 22:23:08)
>> A buffer in function test_flink_name was both too small and never
>> checked for overflow.  Both errors are fixed.
> 
> That many numbers is not interesting. Show the range and median instead.
> -Chris

I don’t understand what you are talking about.  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BUILD: failure for i915/gem_flink_race: Fix error in buffer usage

[Patchwork]

== Series Details ==

Series: i915/gem_flink_race: Fix error in buffer usage
URL   : https://patchwork.freedesktop.org/series/83995/
State : failure

== Summary ==

Applying: i915/gem_flink_race: Fix error in buffer usage
error: sha1 information is lacking or useless (tests/i915/gem_flink_race.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 i915/gem_flink_race: Fix error in buffer usage
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Chris Wilson]

Quoting Hampson, Steven T (2020-11-17 23:45:23)
> 
> 
> Sent from my iPhone
> 
> > On Nov 17, 2020, at 2:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > Quoting Steve Hampson (2020-11-17 22:23:08)
> >> A buffer in function test_flink_name was both too small and never
> >> checked for overflow.  Both errors are fixed.
> > 
> > That many numbers is not interesting. Show the range and median instead.
> > -Chris
> 
> I don’t understand what you are talking about.  

The reason I printed the individual numbers was so that we could see the
distribution in case one thread was being starved or not. That is fine
for a few numbers, but beyond that we can summarise with statistics.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Hampson, Steven T]

Chris,

Is this acceptable?  Can it be merged?

-----Original Message-----
From: Hampson, Steven T 
Sent: Wednesday, November 18, 2020 12:41 PM
To: 'Chris Wilson' <chris@chris-wilson.co.uk>
Cc: intel-gfx@ <lists.freedesktop.org intel-gfx@lists.freedesktop.org>
Subject: RE: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

The problem is that the machine it was running on had 32 cpus, so one set of numbers per cpu filled the buffer.

-----Original Message-----
From: Chris Wilson <chris@chris-wilson.co.uk>
Sent: Wednesday, November 18, 2020 2:52 AM
To: Hampson, Steven T <steven.t.hampson@intel.com>
Cc: intel-gfx@ <lists.freedesktop.org intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

Quoting Hampson, Steven T (2020-11-17 23:45:23)
> 
> 
> Sent from my iPhone
> 
> > On Nov 17, 2020, at 2:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > Quoting Steve Hampson (2020-11-17 22:23:08)
> >> A buffer in function test_flink_name was both too small and never 
> >> checked for overflow.  Both errors are fixed.
> > 
> > That many numbers is not interesting. Show the range and median instead.
> > -Chris
> 
> I don’t understand what you are talking about.  

The reason I printed the individual numbers was so that we could see the distribution in case one thread was being starved or not. That is fine for a few numbers, but beyond that we can summarise with statistics.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915/gem_flink_race: Fix error in buffer usage

[Chris Wilson]

Quoting Hampson, Steven T (2020-11-19 18:18:14)
> Chris,
> 
> Is this acceptable?  Can it be merged?

It is of little use to print out that many numbers, so lets not and just
show some statistics instead as this is just meant to be a guide to the
reader as to whether the threads each received a reasonably fair
proportion of the _CPU_ time.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx