Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
On Tue, Oct 18, 2016 at 01:48:52PM +0100, Chris Wilson wrote: > On Tue, Oct 18, 2016 at 03:41:54PM +0300, Petri Latvala wrote: > > How should vgem work be flushed properly? With this i915 flushing is > > guaranteed even if vgem is opened first, then i915, but such > > flushing won't be done if only vgem is opened (I see only vgem_slow > > doing that)... > > vgem doesn't have the same delayed close as i915. For vgem, closing the > fd (i.e. on process exit) will first signal all fences and drop > references for that fd, so effectively all work will be completed. The > external references to the vgem.ko's object (via dma-buf) will only > exist if they were constructed by the test and if they were, e.g. i915, > they too should be will be flushed by igt in its exithandlers. > > Other drivers may have similar bridges to cross ofc. One long-term caveat is that exit handlers only run on final exit, not after each subtest. Atm that's no issue because we run each subtest in isolation. But I think that for throughput reasons (some of the testcases run _much_ faster if you just launch the binary once, due to fairly heavy overhead in igt_fixtures) we can't rely on that forever. Mostly thinking of high-throughput complete test runs here. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
Late Tested-By: Jari Tahvanainen I executed in HSW 100 times a test list having only one prime_vgem test case and vgem_basic@unload. With fix below: 100% pass for vgem_basic@unload Without fix below: 100% skip for vgem_basic@unload During last night I started 100 repetitions for fast-feedback.testlist on HSW: 100% pass for vgem_basic@unload with kernel build having the fix. Verdict: fix works as expected and seem to remove the flip-flop behavior on vgem_basic@unload. BR, Jari On 10/18/2016 02:36 PM, Chris Wilson wrote: > On Tue, Oct 18, 2016 at 02:25:21PM +0300, Petri Latvala wrote: >> If executed too soon after prime_vgem tests, the vgem unload test >> fails due to its execbuffers still being handled in the kernel. Retry >> the unload three times with sleeps before reporting a skip. > What happened to igt ensuring that the driver was idle before closing > a test? I guess that is complicated by the use of multiple drivers in > vgem. > > diff --git a/lib/drmtest.c b/lib/drmtest.c index 40bbff6..5d3aaa8 > 100644 > --- a/lib/drmtest.c > +++ b/lib/drmtest.c > @@ -344,14 +344,18 @@ int drm_open_driver(int chipset) > fd = __drm_open_driver(chipset); > igt_skip_on_f(fd<0, "No known gpu found\n"); > > - if (__sync_fetch_and_add(&open_count, 1)) > - return fd; > - > + /* For i915, at least, we ensure that the driver is idle before > +* starting a test and we install an exit handler to wait until > +* idle before quitting. > +*/ > if (is_i915_device(fd)) { > - gem_quiescent_gpu(fd); > - igt_install_exit_handler(quiescent_gpu_at_exit); > + if (__sync_fetch_and_add(&open_count, 1) == 0) { > + gem_quiescent_gpu(fd); > + > + at_exit_drm_fd = __drm_open_driver(chipset); > + igt_install_exit_handler(quiescent_gpu_at_exit); > + } > } > - at_exit_drm_fd = __drm_open_driver(chipset); > > return fd; > } > Well I'll be damned, that's an obvious bugfix, regardless of its effects on vgem unload. Please push that with a descriptive commit message and Reviewed-by: Petri Latvala . How should vgem work be flushed properly? With this i915 flushing is guaranteed even if vgem is opened first, then i915, but such flushing won't be done if only vgem is opened (I see only vgem_slow doing that)... Jari will soon reply about vgem unload with this change applied. - Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
On Tue, Oct 18, 2016 at 01:48:52PM +0100, Chris Wilson wrote: > vgem doesn't have the same delayed close as i915. For vgem, closing the > fd (i.e. on process exit) will first signal all fences and drop > references for that fd, so effectively all work will be completed. The > external references to the vgem.ko's object (via dma-buf) will only > exist if they were constructed by the test and if they were, e.g. i915, > they too should be will be flushed by igt in its exithandlers. > > Other drivers may have similar bridges to cross ofc. > -Chris Ok, excellent. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
On Tue, Oct 18, 2016 at 03:41:54PM +0300, Petri Latvala wrote: > How should vgem work be flushed properly? With this i915 flushing is > guaranteed even if vgem is opened first, then i915, but such > flushing won't be done if only vgem is opened (I see only vgem_slow > doing that)... vgem doesn't have the same delayed close as i915. For vgem, closing the fd (i.e. on process exit) will first signal all fences and drop references for that fd, so effectively all work will be completed. The external references to the vgem.ko's object (via dma-buf) will only exist if they were constructed by the test and if they were, e.g. i915, they too should be will be flushed by igt in its exithandlers. Other drivers may have similar bridges to cross ofc. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
On 10/18/2016 02:36 PM, Chris Wilson wrote: On Tue, Oct 18, 2016 at 02:25:21PM +0300, Petri Latvala wrote: If executed too soon after prime_vgem tests, the vgem unload test fails due to its execbuffers still being handled in the kernel. Retry the unload three times with sleeps before reporting a skip. What happened to igt ensuring that the driver was idle before closing a test? I guess that is complicated by the use of multiple drivers in vgem. diff --git a/lib/drmtest.c b/lib/drmtest.c index 40bbff6..5d3aaa8 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -344,14 +344,18 @@ int drm_open_driver(int chipset) fd = __drm_open_driver(chipset); igt_skip_on_f(fd<0, "No known gpu found\n"); - if (__sync_fetch_and_add(&open_count, 1)) - return fd; - + /* For i915, at least, we ensure that the driver is idle before +* starting a test and we install an exit handler to wait until +* idle before quitting. +*/ if (is_i915_device(fd)) { - gem_quiescent_gpu(fd); - igt_install_exit_handler(quiescent_gpu_at_exit); + if (__sync_fetch_and_add(&open_count, 1) == 0) { + gem_quiescent_gpu(fd); + + at_exit_drm_fd = __drm_open_driver(chipset); + igt_install_exit_handler(quiescent_gpu_at_exit); + } } - at_exit_drm_fd = __drm_open_driver(chipset); return fd; } Well I'll be damned, that's an obvious bugfix, regardless of its effects on vgem unload. Please push that with a descriptive commit message and Reviewed-by: Petri Latvala . How should vgem work be flushed properly? With this i915 flushing is guaranteed even if vgem is opened first, then i915, but such flushing won't be done if only vgem is opened (I see only vgem_slow doing that)... Jari will soon reply about vgem unload with this change applied. -- Petri Latvala ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
On Tue, Oct 18, 2016 at 02:25:21PM +0300, Petri Latvala wrote: > If executed too soon after prime_vgem tests, the vgem unload test > fails due to its execbuffers still being handled in the kernel. Retry > the unload three times with sleeps before reporting a skip. What happened to igt ensuring that the driver was idle before closing a test? I guess that is complicated by the use of multiple drivers in vgem. diff --git a/lib/drmtest.c b/lib/drmtest.c index 40bbff6..5d3aaa8 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -344,14 +344,18 @@ int drm_open_driver(int chipset) fd = __drm_open_driver(chipset); igt_skip_on_f(fd<0, "No known gpu found\n"); - if (__sync_fetch_and_add(&open_count, 1)) - return fd; - + /* For i915, at least, we ensure that the driver is idle before +* starting a test and we install an exit handler to wait until +* idle before quitting. +*/ if (is_i915_device(fd)) { - gem_quiescent_gpu(fd); - igt_install_exit_handler(quiescent_gpu_at_exit); + if (__sync_fetch_and_add(&open_count, 1) == 0) { + gem_quiescent_gpu(fd); + + at_exit_drm_fd = __drm_open_driver(chipset); + igt_install_exit_handler(quiescent_gpu_at_exit); + } } - at_exit_drm_fd = __drm_open_driver(chipset); return fd; } -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/vgem_basic: Retry the initial module unload in unload test
If executed too soon after prime_vgem tests, the vgem unload test fails due to its execbuffers still being handled in the kernel. Retry the unload three times with sleeps before reporting a skip. When tested on HSW, one 1s sleep wasn't enough, 3s was. Signed-off-by: Petri Latvala CC: Chris Wilson CC: Jari Tahvanainen CC: Daniel Vetter --- With this change, we can leave vgem_basic@unload last in the CI testlist, and thus still catch actual unload problems. tests/vgem_basic.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/vgem_basic.c b/tests/vgem_basic.c index 5b54a4a..7f0750d 100644 --- a/tests/vgem_basic.c +++ b/tests/vgem_basic.c @@ -296,10 +296,15 @@ static int module_unload(void) static void test_unload(void) { struct vgem_bo bo; - int vgem, dmabuf; + int vgem, dmabuf, unloaded, retry = 3; uint32_t *ptr; - igt_require(module_unload() == 0); + while ((unloaded = module_unload()) && retry-- > 0) { + /* Sleep for a moment to let previous users finish */ + sleep(1); + } + + igt_require(unloaded == 0); vgem = __drm_open_driver(DRIVER_VGEM); igt_assert(vgem != -1); -- 2.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx