| Message ID | 20201117222308.31551-1-steven.t.hampson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i915/gem_flink_race: Fix error in buffer usage | expand |
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
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.
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
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
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
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);