| Message ID | 1442508079-14065-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 >
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();
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(-)