From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org Cc: igt-dev@lists.freedesktop.org Subject: Re: [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations Date: Mon, 11 Feb 2019 17:18:02 +0000 [thread overview] Message-ID: <b3c5f189-1e2e-5c16-835b-65ad699bed02@linux.intel.com> (raw) In-Reply-To: <20190211143544.16184-1-chris@chris-wilson.co.uk> On 11/02/2019 14:35, Chris Wilson wrote: > basic-allocations was written to demonstrate a flaw in our continual > reallocation of cmdparser shadow bo, largely fixed by keeping a small > cache of bo of different lengths (to speed up the search for the correct > sized bo). We only care enough to exercise the slowdown by submitting > lots of execbufs, and can see the effect of bo caching on the rate, so > replace the fixed number of iterations with a timeout and count how many > batches we could submit instead. > > Similarly, we now do not need to wait for all of our queue to complete > as we can tell the kernel to drop the queue instead. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=107936 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/i915/gem_exec_parse.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c > index b653b1bdc..62e8d0a51 100644 > --- a/tests/i915/gem_exec_parse.c > +++ b/tests/i915/gem_exec_parse.c > @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test) > > static void test_allocations(int fd) > { > - uint32_t bbe = MI_BATCH_BUFFER_END; > + const uint32_t bbe = MI_BATCH_BUFFER_END; > struct drm_i915_gem_execbuffer2 execbuf; > struct drm_i915_gem_exec_object2 obj[17]; > - int i, j; > + unsigned long count; > > intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM); > > memset(obj, 0, sizeof(obj)); > - for (i = 0; i < ARRAY_SIZE(obj); i++) { > + for (int i = 0; i < ARRAY_SIZE(obj); i++) { > uint64_t size = 1ull << (12 + i); > > obj[i].handle = gem_create(fd, size); > @@ -322,17 +322,21 @@ static void test_allocations(int fd) > > memset(&execbuf, 0, sizeof(execbuf)); > execbuf.buffer_count = 1; > - for (j = 0; j < 16384; j++) { > - igt_progress("allocations ", j, 16384); > - i = rand() % ARRAY_SIZE(obj); > + > + count = 0; > + igt_until_timeout(20) { > + int i = rand() % ARRAY_SIZE(obj); > execbuf.buffers_ptr = to_user_pointer(&obj[i]); > execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12; > execbuf.batch_start_offset += 64 * (rand() % 64); > execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset; > gem_execbuf(fd, &execbuf); > + count++; > } > + igt_info("Submitted %lu execbufs\n", count); > + igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */ Downside here is that tests start to exercise a lot more driver paths. Or is that an upside? It's confusing these days. I'd prefer if we just let it run and don't involve wedge/unwedge. Well actually... we could modify the submit loop to sync a bit rather than build a queue for 20 seconds? Would sync after each execbuf be detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth and then sync? Regards, Tvrtko > > - for (i = 0; i < ARRAY_SIZE(obj); i++) { > + for (int i = 0; i < ARRAY_SIZE(obj); i++) { > gem_sync(fd, obj[i].handle); > gem_close(fd, obj[i].handle); > } > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org Cc: igt-dev@lists.freedesktop.org Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations Date: Mon, 11 Feb 2019 17:18:02 +0000 [thread overview] Message-ID: <b3c5f189-1e2e-5c16-835b-65ad699bed02@linux.intel.com> (raw) In-Reply-To: <20190211143544.16184-1-chris@chris-wilson.co.uk> On 11/02/2019 14:35, Chris Wilson wrote: > basic-allocations was written to demonstrate a flaw in our continual > reallocation of cmdparser shadow bo, largely fixed by keeping a small > cache of bo of different lengths (to speed up the search for the correct > sized bo). We only care enough to exercise the slowdown by submitting > lots of execbufs, and can see the effect of bo caching on the rate, so > replace the fixed number of iterations with a timeout and count how many > batches we could submit instead. > > Similarly, we now do not need to wait for all of our queue to complete > as we can tell the kernel to drop the queue instead. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=107936 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/i915/gem_exec_parse.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c > index b653b1bdc..62e8d0a51 100644 > --- a/tests/i915/gem_exec_parse.c > +++ b/tests/i915/gem_exec_parse.c > @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test) > > static void test_allocations(int fd) > { > - uint32_t bbe = MI_BATCH_BUFFER_END; > + const uint32_t bbe = MI_BATCH_BUFFER_END; > struct drm_i915_gem_execbuffer2 execbuf; > struct drm_i915_gem_exec_object2 obj[17]; > - int i, j; > + unsigned long count; > > intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM); > > memset(obj, 0, sizeof(obj)); > - for (i = 0; i < ARRAY_SIZE(obj); i++) { > + for (int i = 0; i < ARRAY_SIZE(obj); i++) { > uint64_t size = 1ull << (12 + i); > > obj[i].handle = gem_create(fd, size); > @@ -322,17 +322,21 @@ static void test_allocations(int fd) > > memset(&execbuf, 0, sizeof(execbuf)); > execbuf.buffer_count = 1; > - for (j = 0; j < 16384; j++) { > - igt_progress("allocations ", j, 16384); > - i = rand() % ARRAY_SIZE(obj); > + > + count = 0; > + igt_until_timeout(20) { > + int i = rand() % ARRAY_SIZE(obj); > execbuf.buffers_ptr = to_user_pointer(&obj[i]); > execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12; > execbuf.batch_start_offset += 64 * (rand() % 64); > execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset; > gem_execbuf(fd, &execbuf); > + count++; > } > + igt_info("Submitted %lu execbufs\n", count); > + igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */ Downside here is that tests start to exercise a lot more driver paths. Or is that an upside? It's confusing these days. I'd prefer if we just let it run and don't involve wedge/unwedge. Well actually... we could modify the submit loop to sync a bit rather than build a queue for 20 seconds? Would sync after each execbuf be detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth and then sync? Regards, Tvrtko > > - for (i = 0; i < ARRAY_SIZE(obj); i++) { > + for (int i = 0; i < ARRAY_SIZE(obj); i++) { > gem_sync(fd, obj[i].handle); > gem_close(fd, obj[i].handle); > } > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev
next prev parent reply other threads:[~2019-02-11 17:18 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-11 14:35 [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations Chris Wilson 2019-02-11 14:35 ` [igt-dev] " Chris Wilson 2019-02-11 15:31 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork 2019-02-11 17:18 ` Tvrtko Ursulin [this message] 2019-02-11 17:18 ` [igt-dev] [Intel-gfx] [PATCH i-g-t] " Tvrtko Ursulin 2019-02-11 17:23 ` Chris Wilson 2019-02-11 17:23 ` [igt-dev] [Intel-gfx] " Chris Wilson 2019-02-11 20:37 ` Chris Wilson 2019-02-11 20:37 ` [igt-dev] [Intel-gfx] " Chris Wilson 2019-02-11 18:23 ` [igt-dev] ✓ Fi.CI.IGT: success for " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b3c5f189-1e2e-5c16-835b-65ad699bed02@linux.intel.com \ --to=tvrtko.ursulin@linux.intel.com \ --cc=chris@chris-wilson.co.uk \ --cc=igt-dev@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.