Re: [igt-dev] [PATCH i-g-t] benchmarks/gem_userptr_benchmark: Dead code removal

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

On Fri, May 21, 2021 at 04:16:48PM +0100, Tvrtko Ursulin wrote:
> 
> On 21/05/2021 15:17, Zbigniew Kempczyński wrote:
> > It seems nobody cares of this benchmark a long time - it is still having
> > libdrm dependency what is not true nor initializing code is not within
> > igt_fixture. As it is not supposed to work lets rid off it from IGT.
> 
> Not sure what you mean by it is not _supposed_ to work? Is it broken? Asking
> since not sure we should remove it.

There're two reasons I decided to remove this test:

1. kernel - c6bcc0c2fdfdc3eba0d1d9250521fde2a7a31931
   in short:

           if (flags & I915_USERPTR_UNSYNCHRONIZED)
-               return capable(CAP_SYS_ADMIN) ? 0 : -EPERM;
+               return -ENODEV;

2. igt skip info:

   Test requirement not met in function main, file ../benchmarks/gem_userptr_benchmark.c:477:
   Test requirement: !(ret == 0)
   No userptr support - No such device (0)
   Last errno: 19, No such device
   skipping is allowed only in fixtures, subtests or igt_simple_main
   please refer to lib/igt_core documentation
   gem_userptr_benchmark: ../lib/igt_core.c:363: internal_assert: Assertion `0' failed.
   Received signal SIGABRT.

I've quickly checked (2) and skipping in fixtures occured year ago, so 
from my perspective noone took a look to that benchmark since year.
But... kernel patch (1) occured two months ago and it's the reason
we fall in to that skip. 

I concluded noone takes a look to that benchmark only looking at (2) so
I've incidentally assumed we got > year of dead code. But when you've 
put your doubts I dig more in the kernel to find the history of dropping
I915_USERPTR_UNSYNCHRONIZED support and I've realized it is quite fresh
code. So I was wrong regarding benchmark state.


> There is some value to being able to
> check the various impact scenarios. For instance I remember just having
> userptr objects slows down unrelated operations like mmap/munap and so.
> Which I think would be totally surprising to userspace so existence of a
> benchmark which shows this could still be valuable. Long long time ago we
> almost got a way to run these IGT benchmarks on a CI like system in an
> automated fashion (ezbench) but it did not happen. Still it would be nice
> that one day it does happen.

Understood, benchmark still has value but we have to remove all UNSYCHRONIZED
parts because kernel won't support it likely anymore.

I've resent fixed benchmark and meson.build to drop confusing libdrm dependency.

--
Zbigniew

> 
> Regards,
> 
> Tvrtko
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev