[PATCH] tests/gem_ctx_param_basic: fix up non-root-set-no-zeromap subtest

[PATCH] tests/gem_ctx_param_basic: fix up non-root-set-no-zeromap subtest

[Jesse Barnes]

This subtest is trying to set the no-zeromap flag on the context without
root privs.  Rather than expecting an EPERM on what's presumably a
nonzero value, we should expect success on a set call w/o root privs.
This looks like a copy & paste error from when the subtest was added,
since setting the ban period has different expected behavior.

Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 tests/gem_ctx_param_basic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index 6a1694d..f7d9592 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -126,8 +126,8 @@ igt_main
 
 			ctx_param.context = ctx;
 			TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
-			ctx_param.value--;
-			TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
+			ctx_param.value = 0;
+			TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
 		}
 
 		igt_waitchildren();
-- 
1.9.1

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

Re: [PATCH] tests/gem_ctx_param_basic: fix up non-root-set-no-zeromap subtest

[Thomas Wood]

It's helpful to include "i-g-t" in the subject line for
intel-gpu-tools patches so that they are easily identified. This can
be done by using the --subject-prefix "PATCH i-g-t" option when using
git format-patch or send-email and can also be set as a local
configuration option using the following command: git config
format.subjectprefix "PATCH i-g-t"


On 17 September 2015 at 17:41, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> This subtest is trying to set the no-zeromap flag on the context without
> root privs.  Rather than expecting an EPERM on what's presumably a
> nonzero value, we should expect success on a set call w/o root privs.
> This looks like a copy & paste error from when the subtest was added,
> since setting the ban period has different expected behavior.

There is already a patch for this: http://patchwork.freedesktop.org/patch/58991/

I was waiting for confirmation on the expected behaviour, but also
testing both root and non-root for success seems a bit redundant.
Perhaps removing the root-set test would be worthwhile.


>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  tests/gem_ctx_param_basic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index 6a1694d..f7d9592 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -126,8 +126,8 @@ igt_main
>
>                         ctx_param.context = ctx;
>                         TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> -                       ctx_param.value--;
> -                       TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
> +                       ctx_param.value = 0;
> +                       TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
>                 }
>
>                 igt_waitchildren();
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/gem_ctx_param_basic: fix up non-root-set-no-zeromap subtest

[Jesse Barnes]

On 09/18/2015 03:22 AM, Thomas Wood wrote:
> It's helpful to include "i-g-t" in the subject line for
> intel-gpu-tools patches so that they are easily identified. This can
> be done by using the --subject-prefix "PATCH i-g-t" option when using
> git format-patch or send-email and can also be set as a local
> configuration option using the following command: git config
> format.subjectprefix "PATCH i-g-t"

Yeah you mentioned this before and I forgot, sorry.  I'll add git configs to my igt repos so make it happen automatically.

> On 17 September 2015 at 17:41, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> This subtest is trying to set the no-zeromap flag on the context without
>> root privs.  Rather than expecting an EPERM on what's presumably a
>> nonzero value, we should expect success on a set call w/o root privs.
>> This looks like a copy & paste error from when the subtest was added,
>> since setting the ban period has different expected behavior.
> 
> There is already a patch for this: http://patchwork.freedesktop.org/patch/58991/
> 
> I was waiting for confirmation on the expected behaviour, but also
> testing both root and non-root for success seems a bit redundant.
> Perhaps removing the root-set test would be worthwhile.

Yeah that would be ok too.  FWIW the other patch has my r-b too, though I haven't heard back from David.

Do you want to commit Daniele's patch or should I just push mine?

Thanks,
Jesse

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

Re: [PATCH] tests/gem_ctx_param_basic: fix up non-root-set-no-zeromap subtest

[Thomas Wood]

On 18 September 2015 at 17:02, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On 09/18/2015 03:22 AM, Thomas Wood wrote:
>> It's helpful to include "i-g-t" in the subject line for
>> intel-gpu-tools patches so that they are easily identified. This can
>> be done by using the --subject-prefix "PATCH i-g-t" option when using
>> git format-patch or send-email and can also be set as a local
>> configuration option using the following command: git config
>> format.subjectprefix "PATCH i-g-t"
>
> Yeah you mentioned this before and I forgot, sorry.  I'll add git configs to my igt repos so make it happen automatically.
>
>> On 17 September 2015 at 17:41, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>>> This subtest is trying to set the no-zeromap flag on the context without
>>> root privs.  Rather than expecting an EPERM on what's presumably a
>>> nonzero value, we should expect success on a set call w/o root privs.
>>> This looks like a copy & paste error from when the subtest was added,
>>> since setting the ban period has different expected behavior.
>>
>> There is already a patch for this: http://patchwork.freedesktop.org/patch/58991/
>>
>> I was waiting for confirmation on the expected behaviour, but also
>> testing both root and non-root for success seems a bit redundant.
>> Perhaps removing the root-set test would be worthwhile.
>
> Yeah that would be ok too.  FWIW the other patch has my r-b too, though I haven't heard back from David.
>
> Do you want to commit Daniele's patch or should I just push mine?

Thanks for the review, I've pushed Daniele's patch with your
reviewed-by tag as I already had it queued.


>
> Thanks,
> Jesse
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx