* [igt-dev] [PATCH i-g-t v4 2/3] tests/gem_ctx_param: Update invalid param
2018-01-25 1:00 [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array Antonio Argenziano
@ 2018-01-25 1:00 ` Antonio Argenziano
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users Antonio Argenziano
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Antonio Argenziano @ 2018-01-25 1:00 UTC (permalink / raw)
To: igt-dev
Since commit: drm/i915/scheduler: Support user-defined priorities, the
driver support an extra context param to set context's priority. Add
tests for that interface and update invalid tests.
v2:
- Add arg size validation test. (Chris)
- Add arg value overflow test. (Chris)
- Add test for unsupported platforms. (Chris)
- Feed interface with all priority values and in random order. (Chris)
v3:
- Parametrize tests. (Chris)
v4:
- Code-style refactoring. (Chris)
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
tests/gem_ctx_param.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 188 insertions(+), 1 deletion(-)
diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index c20ae1ee..180202b9 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -25,9 +25,147 @@
*/
#include "igt.h"
+#include <limits.h>
IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
+#define MAX_USER_SET_PRIO I915_CONTEXT_DEFAULT_PRIORITY /* Current max prio for non-root users */
+#define PRIO_RANGE (I915_CONTEXT_MAX_USER_PRIORITY - I915_CONTEXT_MIN_USER_PRIORITY)
+#define USER_PRIO_RANGE (MAX_USER_SET_PRIO - I915_CONTEXT_MIN_USER_PRIORITY)
+
+#define ROOT (0x1 << 3)
+#define NEW_CTX (0x1 << 2)
+#define VALID_PRIO (0x1 << 1)
+#define OVERFLOW_PRIO (0x1 << 0)
+
+#define IS_ROOT(flags) (flags & ROOT)
+
+static int is_priority_valid(int64_t value, unsigned flags)
+{
+ if (IS_ROOT(flags)) {
+ if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
+ (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+ return 0;
+
+ return -EINVAL;
+ } else {
+ if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= USER_PRIO_RANGE &&
+ (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+ return 0;
+
+ if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
+ (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
+ return -EPERM;
+
+ return -EINVAL;
+ }
+}
+
+static void
+get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+ const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
+ int64_t *values;
+
+ *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
+ values = (int64_t*) calloc(*size, sizeof(int64_t));
+ igt_assert(values);
+
+ for (int i = 0; i < *size; i++) {
+ values[i] = overflow ^ (i + I915_CONTEXT_MIN_USER_PRIORITY);
+ }
+
+ *prio_values = values;
+}
+
+static void
+get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+ const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
+ int64_t *values;
+
+ if (IS_ROOT(flags)) {
+ int64_t test_values[] = { /* Test space too big pick significant values */
+ INT_MIN,
+ I915_CONTEXT_MIN_USER_PRIORITY - 1,
+ I915_CONTEXT_MAX_USER_PRIORITY + 1,
+ INT_MAX
+ };
+
+ *size = ARRAY_SIZE(test_values);
+ values = (int64_t*) calloc(*size, sizeof(int64_t));
+
+ for (int i = 0; i < *size; i++)
+ values[i] = overflow ^ test_values[i];
+ } else {
+ *size = PRIO_RANGE - USER_PRIO_RANGE;
+ values = (int64_t*) calloc(*size, sizeof(int64_t));
+
+ for (int i = 0; i < *size; i++)
+ values[i] = overflow ^ (i + (MAX_USER_SET_PRIO + 1));
+ }
+
+ *prio_values = values;
+}
+
+static void
+get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
+{
+ if (flags & VALID_PRIO)
+ get_prio_values_valid(prio_values, size, flags);
+ else
+ get_prio_values_invalid(prio_values, size, flags);
+
+ igt_permute_array(*prio_values, *size, igt_exchange_int64);
+}
+
+static void
+set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
+{
+ uint32_t ctx = 0;
+ int expected_ret;
+ int64_t old_value;
+ int64_t *prio_values;
+ unsigned size;
+
+ igt_fork(child, 1) {
+ get_prio_values(&prio_values, &size, flags);
+
+ if (flags & NEW_CTX)
+ ctx = gem_context_create(fd);
+ arg.ctx_id = ctx;
+
+ if (!(flags & ROOT)) {
+ igt_debug("Dropping root privilege\n");
+ igt_drop_root();
+ }
+
+ gem_context_get_param(fd, &arg);
+ old_value = arg.value;
+
+ for (int i = 0; i < size; i++) {
+ arg.value = prio_values[i];
+ expected_ret = is_priority_valid(arg.value, flags);
+
+ igt_assert_eq(__gem_context_set_param(fd, &arg), expected_ret);
+
+ gem_context_get_param(fd, &arg);
+ if (expected_ret)
+ igt_assert_eq(arg.value, old_value); /* unchanged */
+ else
+ igt_assert_eq(arg.value, prio_values[i]); /* updated */
+ }
+
+ arg.value = 0;
+ gem_context_set_param(fd, &arg);
+
+ if (flags & NEW_CTX)
+ gem_context_destroy(fd, ctx);
+ }
+
+ igt_waitchildren();
+}
+
igt_main
{
struct drm_i915_gem_context_param arg;
@@ -136,11 +274,60 @@ igt_main
gem_context_set_param(fd, &arg);
}
+ arg.param = I915_CONTEXT_PARAM_PRIORITY;
+
+ igt_subtest("set-priority-not-supported") {
+ igt_require(!gem_scheduler_has_ctx_priority(fd));
+
+ arg.ctx_id = ctx;
+ arg.size = 0;
+
+ igt_assert_eq(__gem_context_set_param(fd, &arg), -ENODEV);
+ }
+
+ igt_subtest_group {
+ igt_fixture {
+ igt_require(gem_scheduler_has_ctx_priority(fd));
+ }
+
+ igt_subtest("get-priority-new-ctx") {
+ struct drm_i915_gem_context_param local_arg = arg;
+ uint32_t local_ctx = gem_context_create(fd);
+
+ local_arg.ctx_id = local_ctx;
+
+ gem_context_get_param(fd, &local_arg);
+ igt_assert_eq(local_arg.value, 0);
+
+ gem_context_destroy(fd, local_ctx);
+ }
+
+ igt_subtest("set-priority-invalid-size") {
+ struct drm_i915_gem_context_param local_arg = arg;
+ local_arg.ctx_id = ctx;
+ local_arg.value = 0;
+ local_arg.size = ~0;
+
+ igt_assert_eq(__gem_context_set_param(fd, &local_arg), -EINVAL);
+ }
+
+ for (unsigned flags = 0;
+ flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
+ flags++) {
+ igt_subtest_f("set-priority%s%s%s%s",
+ (flags & ROOT) ? "-root" : "-user",
+ (flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
+ (flags & VALID_PRIO) ? "" : "-invalid",
+ (flags & OVERFLOW_PRIO) ? "-overflow" : "")
+ set_priority(fd, arg, flags);
+ }
+ }
+
/* NOTE: This testcase intentionally tests for the next free parameter
* to catch ABI extensions. Don't "fix" this testcase without adding all
* the tests for the new param first.
*/
- arg.param = I915_CONTEXT_PARAM_BANNABLE + 1;
+ arg.param = I915_CONTEXT_PARAM_PRIORITY + 1;
igt_subtest("invalid-param-get") {
arg.ctx_id = ctx;
--
2.14.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 16+ messages in thread* [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-25 1:00 [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array Antonio Argenziano
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 2/3] tests/gem_ctx_param: Update invalid param Antonio Argenziano
@ 2018-01-25 1:00 ` Antonio Argenziano
2018-01-25 11:37 ` Chris Wilson
2018-02-28 14:10 ` Arkadiusz Hiler
2018-01-25 1:18 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array Patchwork
` (2 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Antonio Argenziano @ 2018-01-25 1:00 UTC (permalink / raw)
To: igt-dev
Adds tests for !SYS_NICE users trying to change the priority level of a
context using the provided IOCTL interface.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
---
README | 1 +
configure.ac | 8 ++++++++
meson.build | 3 +++
tests/Makefile.am | 7 +++++++
tests/Makefile.sources | 1 -
tests/gem_ctx_param.c | 42 ++++++++++++++++++++++++++++++++++++------
6 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/README b/README
index 7cd78e44..89c10beb 100644
--- a/README
+++ b/README
@@ -147,6 +147,7 @@ the default configuration (package names may vary):
libkmod-dev
libpciaccess-dev
libprocps-dev
+ libcap-dev
libunwind-dev
python-docutils
x11proto-dri2-dev
diff --git a/configure.ac b/configure.ac
index e13a3b74..2d67e3f1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
])
AC_SUBST(TIMER_LIBS)
+AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
+if test x$have_libcap = xyes; then
+ AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
+fi
+
+AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
+
dnl Check for CPUID
cpuid="yes"
AC_TRY_LINK([
@@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
PKG_CHECK_MODULES(PROCPS, [libprocps])
PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
+#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
if test x$have_valgrind = xyes; then
AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
diff --git a/meson.build b/meson.build
index 9036feb1..937edf52 100644
--- a/meson.build
+++ b/meson.build
@@ -106,6 +106,9 @@ endif
if cc.has_header('sys/io.h')
config.set('HAVE_SYS_IO_H', 1)
endif
+if cc.has_header('sys/capability.h')
+ config.set('HAVE_LIBCAP', 1)
+endif
if cc.has_header('cpuid.h')
# FIXME: Do we need the example link test from configure.ac?
config.set('HAVE_CPUID_H', 1)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1b9a7b0a..c482e970 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -14,6 +14,12 @@ if BUILD_VC4
TESTS_progs += $(VC4_TESTS)
endif
+if HAVE_LIBCAP
+TESTS_progs += \
+ gem_ctx_param \
+ $(NULL)
+endif
+
if HAVE_CHAMELIUM
TESTS_progs += \
kms_chamelium \
@@ -106,6 +112,7 @@ gem_close_race_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_close_race_LDADD = $(LDADD) -lpthread
gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_ctx_thrash_LDADD = $(LDADD) -lpthread
+gem_ctx_param_LDADD = $(LDADD) -lcap
gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_exec_parallel_LDADD = $(LDADD) -lpthread
gem_fence_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index e4e06d01..4fd3c7e8 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -57,7 +57,6 @@ TESTS_progs = \
gem_ctx_bad_exec \
gem_ctx_create \
gem_ctx_exec \
- gem_ctx_param \
gem_ctx_switch \
gem_ctx_thrash \
gem_double_irq_loop \
diff --git a/tests/gem_ctx_param.c b/tests/gem_ctx_param.c
index 180202b9..c51b3290 100644
--- a/tests/gem_ctx_param.c
+++ b/tests/gem_ctx_param.c
@@ -26,6 +26,7 @@
#include "igt.h"
#include <limits.h>
+#include <sys/capability.h>
IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
@@ -33,16 +34,17 @@ IGT_TEST_DESCRIPTION("Basic test for context set/get param input validation.");
#define PRIO_RANGE (I915_CONTEXT_MAX_USER_PRIORITY - I915_CONTEXT_MIN_USER_PRIORITY)
#define USER_PRIO_RANGE (MAX_USER_SET_PRIO - I915_CONTEXT_MIN_USER_PRIORITY)
+#define NICE (0x1 << 4)
#define ROOT (0x1 << 3)
#define NEW_CTX (0x1 << 2)
#define VALID_PRIO (0x1 << 1)
#define OVERFLOW_PRIO (0x1 << 0)
-#define IS_ROOT(flags) (flags & ROOT)
+#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))
static int is_priority_valid(int64_t value, unsigned flags)
{
- if (IS_ROOT(flags)) {
+ if (IS_ROOT_AND_NICE(flags)) {
if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
(value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
return 0;
@@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
int64_t *values;
- *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
+ *size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
values = (int64_t*) calloc(*size, sizeof(int64_t));
igt_assert(values);
@@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
int64_t *values;
- if (IS_ROOT(flags)) {
+ if (IS_ROOT_AND_NICE(flags)) {
int64_t test_values[] = { /* Test space too big pick significant values */
INT_MIN,
I915_CONTEXT_MIN_USER_PRIORITY - 1,
@@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
igt_permute_array(*prio_values, *size, igt_exchange_int64);
}
+static void lower_sys_nice(void)
+{
+ cap_t caps;
+ cap_value_t cap_list = CAP_SYS_NICE;
+ pid_t pid = getpid();
+ cap_flag_value_t cap_val;
+
+ caps = cap_get_pid(pid);
+ igt_require(caps);
+
+ cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
+ if (cap_val == CAP_CLEAR)
+ return; /* CAP_SYS_NICE already unset */
+
+ igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
+ igt_assert(cap_set_proc(caps) == 0);
+ cap_free(caps);
+}
+
static void
set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
{
@@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
igt_drop_root();
}
+ if (!(flags & NICE)) {
+ igt_debug("Dropping SYS_NICE capability\n");
+ lower_sys_nice();
+ }
+
gem_context_get_param(fd, &arg);
old_value = arg.value;
@@ -312,9 +338,13 @@ igt_main
}
for (unsigned flags = 0;
- flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
+ flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
flags++) {
- igt_subtest_f("set-priority%s%s%s%s",
+ if (!(flags & NICE) && !(flags & ROOT))
+ continue; /* Needs to be rot to set properties */
+
+ igt_subtest_f("set-priority%s%s%s%s%s",
+ (flags & NICE) ? "" : "-not-nice",
(flags & ROOT) ? "-root" : "-user",
(flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
(flags & VALID_PRIO) ? "" : "-invalid",
--
2.14.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users Antonio Argenziano
@ 2018-01-25 11:37 ` Chris Wilson
2018-01-31 0:47 ` Antonio Argenziano
2018-02-28 14:10 ` Arkadiusz Hiler
1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2018-01-25 11:37 UTC (permalink / raw)
To: Antonio Argenziano, igt-dev
Quoting Antonio Argenziano (2018-01-25 01:00:03)
> Adds tests for !SYS_NICE users trying to change the priority level of a
> context using the provided IOCTL interface.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> ---
> README | 1 +
> configure.ac | 8 ++++++++
> meson.build | 3 +++
> tests/Makefile.am | 7 +++++++
> tests/Makefile.sources | 1 -
> tests/gem_ctx_param.c | 42 ++++++++++++++++++++++++++++++++++++------
> 6 files changed, 55 insertions(+), 7 deletions(-)
>
> diff --git a/README b/README
> index 7cd78e44..89c10beb 100644
> --- a/README
> +++ b/README
> @@ -147,6 +147,7 @@ the default configuration (package names may vary):
> libkmod-dev
> libpciaccess-dev
> libprocps-dev
> + libcap-dev
> libunwind-dev
> python-docutils
> x11proto-dri2-dev
> diff --git a/configure.ac b/configure.ac
> index e13a3b74..2d67e3f1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
> ])
> AC_SUBST(TIMER_LIBS)
>
> +AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
> +if test x$have_libcap = xyes; then
> + AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
> +fi
> +
> +AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
> +
> dnl Check for CPUID
> cpuid="yes"
> AC_TRY_LINK([
> @@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
> PKG_CHECK_MODULES(PROCPS, [libprocps])
> PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
> PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
> +#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
>
> if test x$have_valgrind = xyes; then
> AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
> diff --git a/meson.build b/meson.build
> index 9036feb1..937edf52 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -106,6 +106,9 @@ endif
> if cc.has_header('sys/io.h')
> config.set('HAVE_SYS_IO_H', 1)
> endif
> +if cc.has_header('sys/capability.h')
> + config.set('HAVE_LIBCAP', 1)
> +endif
> if cc.has_header('cpuid.h')
> # FIXME: Do we need the example link test from configure.ac?
> config.set('HAVE_CPUID_H', 1)
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 1b9a7b0a..c482e970 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -14,6 +14,12 @@ if BUILD_VC4
> TESTS_progs += $(VC4_TESTS)
> endif
>
> +if HAVE_LIBCAP
> +TESTS_progs += \
> + gem_ctx_param \
> + $(NULL)
> +endif
CI doesn't have libcap-dev so this nerfed the tests.
Only one subtest requires LIBCAP...
> +#define NICE (0x1 << 4)
> #define ROOT (0x1 << 3)
> #define NEW_CTX (0x1 << 2)
> #define VALID_PRIO (0x1 << 1)
> #define OVERFLOW_PRIO (0x1 << 0)
Hmm, this is opposite to the usual ordering :)
>
> -#define IS_ROOT(flags) (flags & ROOT)
> +#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))
Nah, you meant IS_ROOT_OR_NICE in most places.
> static int is_priority_valid(int64_t value, unsigned flags)
> {
> - if (IS_ROOT(flags)) {
> + if (IS_ROOT_AND_NICE(flags)) {
> if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
> (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
> return 0;
> @@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
> const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
> int64_t *values;
>
> - *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
> + *size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
> values = (int64_t*) calloc(*size, sizeof(int64_t));
> igt_assert(values);
>
> @@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
> const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
> int64_t *values;
>
> - if (IS_ROOT(flags)) {
> + if (IS_ROOT_AND_NICE(flags)) {
> int64_t test_values[] = { /* Test space too big pick significant values */
> INT_MIN,
> I915_CONTEXT_MIN_USER_PRIORITY - 1,
> @@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
> igt_permute_array(*prio_values, *size, igt_exchange_int64);
> }
>
> +static void lower_sys_nice(void)
> +{
#if HAVE_LIBCAP
> + cap_t caps;
> + cap_value_t cap_list = CAP_SYS_NICE;
> + pid_t pid = getpid();
> + cap_flag_value_t cap_val;
> +
> + caps = cap_get_pid(pid);
> + igt_require(caps);
> +
> + cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
> + if (cap_val == CAP_CLEAR)
> + return; /* CAP_SYS_NICE already unset */
> +
> + igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
> + igt_assert(cap_set_proc(caps) == 0);
> + cap_free(caps);
#else
return false;
#endif
> +}
> +
> static void
> set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
> {
> @@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
> igt_drop_root();
> }
>
> + if (!(flags & NICE)) {
> + igt_debug("Dropping SYS_NICE capability\n");
> + lower_sys_nice();
Rewrite as igt_require(drop_sys_nice());
> + }
> +
> gem_context_get_param(fd, &arg);
> old_value = arg.value;
>
> @@ -312,9 +338,13 @@ igt_main
> }
>
> for (unsigned flags = 0;
> - flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> + flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> flags++) {
> - igt_subtest_f("set-priority%s%s%s%s",
> + if (!(flags & NICE) && !(flags & ROOT))
> + continue; /* Needs to be rot to set properties */
Nope, just use igt_require as above.
> +
> + igt_subtest_f("set-priority%s%s%s%s%s",
> + (flags & NICE) ? "" : "-not-nice",
> (flags & ROOT) ? "-root" : "-user",
> (flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
> (flags & VALID_PRIO) ? "" : "-invalid",
> --
> 2.14.2
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-25 11:37 ` Chris Wilson
@ 2018-01-31 0:47 ` Antonio Argenziano
2018-01-31 8:01 ` Daniel Vetter
0 siblings, 1 reply; 16+ messages in thread
From: Antonio Argenziano @ 2018-01-31 0:47 UTC (permalink / raw)
To: Chris Wilson, igt-dev
On 25/01/18 03:37, Chris Wilson wrote:
> Quoting Antonio Argenziano (2018-01-25 01:00:03)
>> Adds tests for !SYS_NICE users trying to change the priority level of a
>> context using the provided IOCTL interface.
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> ---
>> README | 1 +
>> configure.ac | 8 ++++++++
>> meson.build | 3 +++
>> tests/Makefile.am | 7 +++++++
>> tests/Makefile.sources | 1 -
>> tests/gem_ctx_param.c | 42 ++++++++++++++++++++++++++++++++++++------
>> 6 files changed, 55 insertions(+), 7 deletions(-)
>>
>> diff --git a/README b/README
>> index 7cd78e44..89c10beb 100644
>> --- a/README
>> +++ b/README
>> @@ -147,6 +147,7 @@ the default configuration (package names may vary):
>> libkmod-dev
>> libpciaccess-dev
>> libprocps-dev
>> + libcap-dev
>> libunwind-dev
>> python-docutils
>> x11proto-dri2-dev
>> diff --git a/configure.ac b/configure.ac
>> index e13a3b74..2d67e3f1 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -81,6 +81,13 @@ AC_CHECK_FUNCS(timer_create, [], [
>> ])
>> AC_SUBST(TIMER_LIBS)
>>
>> +AC_CHECK_HEADER(sys/capability.h, [have_libcap=yes])
>> +if test x$have_libcap = xyes; then
>> + AC_DEFINE(HAVE_LIBCAP, 1, [Enable libcap support.])
>> +fi
>> +
>> +AM_CONDITIONAL(HAVE_LIBCAP, [test "x$have_libcap" = xyes])
>> +
>> dnl Check for CPUID
>> cpuid="yes"
>> AC_TRY_LINK([
>> @@ -126,6 +133,7 @@ PKG_CHECK_MODULES(KMOD, [libkmod])
>> PKG_CHECK_MODULES(PROCPS, [libprocps])
>> PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
>> PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
>> +#PKG_CHECK_MODULES(LIBCAP, [libcap-dev], [have_libcap=yes], [have_libcap=no])
>>
>> if test x$have_valgrind = xyes; then
>> AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
>> diff --git a/meson.build b/meson.build
>> index 9036feb1..937edf52 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -106,6 +106,9 @@ endif
>> if cc.has_header('sys/io.h')
>> config.set('HAVE_SYS_IO_H', 1)
>> endif
>> +if cc.has_header('sys/capability.h')
>> + config.set('HAVE_LIBCAP', 1)
>> +endif
>> if cc.has_header('cpuid.h')
>> # FIXME: Do we need the example link test from configure.ac?
>> config.set('HAVE_CPUID_H', 1)
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 1b9a7b0a..c482e970 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -14,6 +14,12 @@ if BUILD_VC4
>> TESTS_progs += $(VC4_TESTS)
>> endif
>>
>> +if HAVE_LIBCAP
>> +TESTS_progs += \
>> + gem_ctx_param \
>> + $(NULL)
>> +endif
>
> CI doesn't have libcap-dev so this nerfed the tests.
> Only one subtest requires LIBCAP...
>
>> +#define NICE (0x1 << 4)
>> #define ROOT (0x1 << 3)
>> #define NEW_CTX (0x1 << 2)
>> #define VALID_PRIO (0x1 << 1)
>> #define OVERFLOW_PRIO (0x1 << 0)
>
> Hmm, this is opposite to the usual ordering :)
>>
>> -#define IS_ROOT(flags) (flags & ROOT)
>> +#define IS_ROOT_AND_NICE(flags) ((flags & ROOT) && (flags & NICE))
>
> Nah, you meant IS_ROOT_OR_NICE in most places.
Or even just IS_NICE if I filter flags with NICE and !ROOT when creating
sub-tests.
>
>> static int is_priority_valid(int64_t value, unsigned flags)
>> {
>> - if (IS_ROOT(flags)) {
>> + if (IS_ROOT_AND_NICE(flags)) {
>> if ((value - I915_CONTEXT_MIN_USER_PRIORITY) <= PRIO_RANGE &&
>> (value - I915_CONTEXT_MIN_USER_PRIORITY) >= 0)
>> return 0;
>> @@ -67,7 +69,7 @@ get_prio_values_valid(int64_t **prio_values, unsigned *size, unsigned flags)
>> const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>> int64_t *values;
>>
>> - *size = (IS_ROOT(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
>> + *size = (IS_ROOT_AND_NICE(flags) ? PRIO_RANGE : USER_PRIO_RANGE) + 1;
>> values = (int64_t*) calloc(*size, sizeof(int64_t));
>> igt_assert(values);
>>
>> @@ -84,7 +86,7 @@ get_prio_values_invalid(int64_t **prio_values, unsigned *size, unsigned flags)
>> const int64_t overflow = (flags & OVERFLOW_PRIO) ? (int64_t)1 << 32 : 0;
>> int64_t *values;
>>
>> - if (IS_ROOT(flags)) {
>> + if (IS_ROOT_AND_NICE(flags)) {
>> int64_t test_values[] = { /* Test space too big pick significant values */
>> INT_MIN,
>> I915_CONTEXT_MIN_USER_PRIORITY - 1,
>> @@ -119,6 +121,25 @@ get_prio_values(int64_t **prio_values, unsigned *size, unsigned flags)
>> igt_permute_array(*prio_values, *size, igt_exchange_int64);
>> }
>>
>> +static void lower_sys_nice(void)
>> +{
>
> #if HAVE_LIBCAP
>
>> + cap_t caps;
>> + cap_value_t cap_list = CAP_SYS_NICE;
>> + pid_t pid = getpid();
>> + cap_flag_value_t cap_val;
>> +
>> + caps = cap_get_pid(pid);
>> + igt_require(caps);
>> +
>> + cap_get_flag(caps, cap_list, CAP_EFFECTIVE, &cap_val);
>> + if (cap_val == CAP_CLEAR)
>> + return; /* CAP_SYS_NICE already unset */
>> +
>> + igt_assert(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_list, CAP_CLEAR) == 0);
>> + igt_assert(cap_set_proc(caps) == 0);
>> + cap_free(caps);
>
> #else
> return false;
> #endif
>
>> +}
>> +
>> static void
>> set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>> {
>> @@ -140,6 +161,11 @@ set_priority(int fd, struct drm_i915_gem_context_param arg, unsigned flags)
>> igt_drop_root();
>> }
>>
>> + if (!(flags & NICE)) {
>> + igt_debug("Dropping SYS_NICE capability\n");
>> + lower_sys_nice();
>
> Rewrite as igt_require(drop_sys_nice());
>
>> + }
>> +
>> gem_context_get_param(fd, &arg);
>> old_value = arg.value;
>>
>> @@ -312,9 +338,13 @@ igt_main
>> }
>>
>> for (unsigned flags = 0;
>> - flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>> + flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>> flags++) {
>> - igt_subtest_f("set-priority%s%s%s%s",
>> + if (!(flags & NICE) && !(flags & ROOT))
>> + continue; /* Needs to be rot to set properties */
>
> Nope, just use igt_require as above.
I didn't want any "*-not-nice-user*" subtests to appear in the list at
all, with igt_require they would just skip but still be there right?
Thanks,
Antonio
>
>> +
>> + igt_subtest_f("set-priority%s%s%s%s%s",
>> + (flags & NICE) ? "" : "-not-nice",
>> (flags & ROOT) ? "-root" : "-user",
>> (flags & NEW_CTX) ? "-new-ctx" : "-default-ctx",
>> (flags & VALID_PRIO) ? "" : "-invalid",
>> --
>> 2.14.2
>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-31 0:47 ` Antonio Argenziano
@ 2018-01-31 8:01 ` Daniel Vetter
2018-02-02 22:24 ` Antonio Argenziano
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2018-01-31 8:01 UTC (permalink / raw)
To: Antonio Argenziano; +Cc: igt-dev
On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
> On 25/01/18 03:37, Chris Wilson wrote:
> > Quoting Antonio Argenziano (2018-01-25 01:00:03)
> > > + }
> > > +
> > > gem_context_get_param(fd, &arg);
> > > old_value = arg.value;
> > > @@ -312,9 +338,13 @@ igt_main
> > > }
> > > for (unsigned flags = 0;
> > > - flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> > > + flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> > > flags++) {
> > > - igt_subtest_f("set-priority%s%s%s%s",
> > > + if (!(flags & NICE) && !(flags & ROOT))
> > > + continue; /* Needs to be rot to set properties */
> >
> > Nope, just use igt_require as above.
>
> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
> with igt_require they would just skip but still be there right?
subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
Hence why igt_require/igt_skip result in a special subtest result and will
not supress the subtest outright.
I know that vpg is struggling with heaps of skipped tests in their
reporting, but as soon as you do cross-platform/kernel/OS testing, it's
the only reasonable way to handle things.
So yeah, this needs to be an igt_require, and probably moved into the
subtest stanza (or wrapped with an igt_subtest_group depending upon the
overall test logic).
Aside: The igt intro docs should explain this, patch would be great if
it's unclear/missing.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-31 8:01 ` Daniel Vetter
@ 2018-02-02 22:24 ` Antonio Argenziano
2018-02-02 22:33 ` Chris Wilson
0 siblings, 1 reply; 16+ messages in thread
From: Antonio Argenziano @ 2018-02-02 22:24 UTC (permalink / raw)
To: Daniel Vetter; +Cc: igt-dev
On 31/01/18 00:01, Daniel Vetter wrote:
> On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
>> On 25/01/18 03:37, Chris Wilson wrote:
>>> Quoting Antonio Argenziano (2018-01-25 01:00:03)
>>>> + }
>>>> +
>>>> gem_context_get_param(fd, &arg);
>>>> old_value = arg.value;
>>>> @@ -312,9 +338,13 @@ igt_main
>>>> }
>>>> for (unsigned flags = 0;
>>>> - flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>> + flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
>>>> flags++) {
>>>> - igt_subtest_f("set-priority%s%s%s%s",
>>>> + if (!(flags & NICE) && !(flags & ROOT))
>>>> + continue; /* Needs to be rot to set properties */
>>>
>>> Nope, just use igt_require as above.
>>
>> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
>> with igt_require they would just skip but still be there right?
>
> subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
> Hence why igt_require/igt_skip result in a special subtest result and will
> not supress the subtest outright.
In this case the combination I am trying to suppress is actually invalid
for all platforms/kernels that is why I wanted to avoid skipping as it
would hint that the test can actually run if some conditions are met
which is not the case here. That is why I was confused about the first
suggestion (which I might be completely misinterpreting). I'll send the
new revision so we can continue the discussion over the new code.
>
> I know that vpg is struggling with heaps of skipped tests in their
(whispers) I actually agree with running everything and letting the test
skip when not applicable ;).
Thanks,
Antonio
> reporting, but as soon as you do cross-platform/kernel/OS testing, it's
> the only reasonable way to handle things.
>
> So yeah, this needs to be an igt_require, and probably moved into the
> subtest stanza (or wrapped with an igt_subtest_group depending upon the
> overall test logic).
>
>
> Aside: The igt intro docs should explain this, patch would be great if
> it's unclear/missing.
> -Daniel
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-02-02 22:24 ` Antonio Argenziano
@ 2018-02-02 22:33 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2018-02-02 22:33 UTC (permalink / raw)
To: Antonio Argenziano, Daniel Vetter; +Cc: igt-dev
Quoting Antonio Argenziano (2018-02-02 22:24:44)
>
>
> On 31/01/18 00:01, Daniel Vetter wrote:
> > On Tue, Jan 30, 2018 at 04:47:57PM -0800, Antonio Argenziano wrote:
> >> On 25/01/18 03:37, Chris Wilson wrote:
> >>> Quoting Antonio Argenziano (2018-01-25 01:00:03)
> >>>> + }
> >>>> +
> >>>> gem_context_get_param(fd, &arg);
> >>>> old_value = arg.value;
> >>>> @@ -312,9 +338,13 @@ igt_main
> >>>> }
> >>>> for (unsigned flags = 0;
> >>>> - flags <= (ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> >>>> + flags <= (NICE | ROOT | NEW_CTX | VALID_PRIO | OVERFLOW_PRIO);
> >>>> flags++) {
> >>>> - igt_subtest_f("set-priority%s%s%s%s",
> >>>> + if (!(flags & NICE) && !(flags & ROOT))
> >>>> + continue; /* Needs to be rot to set properties */
> >>>
> >>> Nope, just use igt_require as above.
> >>
> >> I didn't want any "*-not-nice-user*" subtests to appear in the list at all,
> >> with igt_require they would just skip but still be there right?
> >
> > subtest lists shouldn't be dynamic, it confuses CI infrastructure badly.
> > Hence why igt_require/igt_skip result in a special subtest result and will
> > not supress the subtest outright.
>
> In this case the combination I am trying to suppress is actually invalid
> for all platforms/kernels that is why I wanted to avoid skipping as it
> would hint that the test can actually run if some conditions are met
> which is not the case here. That is why I was confused about the first
> suggestion (which I might be completely misinterpreting). I'll send the
> new revision so we can continue the discussion over the new code.
Then I'm missing something as those tests should still work for the
user, just the valid range is different. (Still vital to probe overflows
against the restricted range etc.) I think if you call them USER, NICE
it's clearer (as the first drops-all-caps, the second drops-all-caps-but-nice).
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users Antonio Argenziano
2018-01-25 11:37 ` Chris Wilson
@ 2018-02-28 14:10 ` Arkadiusz Hiler
2018-02-28 14:15 ` Chris Wilson
1 sibling, 1 reply; 16+ messages in thread
From: Arkadiusz Hiler @ 2018-02-28 14:10 UTC (permalink / raw)
To: Antonio Argenziano; +Cc: igt-dev
On Wed, Jan 24, 2018 at 05:00:03PM -0800, Antonio Argenziano wrote:
> Adds tests for !SYS_NICE users trying to change the priority level of a
> context using the provided IOCTL interface.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
Hey, what's up with this patch? Rest of the series was already merged
through inclusion in other series (AFAIUI). Are you going to do a
followup or can I mark it as not applicable?
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
2018-02-28 14:10 ` Arkadiusz Hiler
@ 2018-02-28 14:15 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2018-02-28 14:15 UTC (permalink / raw)
To: Arkadiusz Hiler, Antonio Argenziano; +Cc: igt-dev
Quoting Arkadiusz Hiler (2018-02-28 14:10:14)
> On Wed, Jan 24, 2018 at 05:00:03PM -0800, Antonio Argenziano wrote:
> > Adds tests for !SYS_NICE users trying to change the priority level of a
> > context using the provided IOCTL interface.
> >
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
>
> Hey, what's up with this patch? Rest of the series was already merged
> through inclusion in other series (AFAIUI). Are you going to do a
> followup or can I mark it as not applicable?
It's a good test that just needed a bit more work, to sort out the
libcap dependency problem and cleaning up the (root, nice, user) test
modes.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 1:00 [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array Antonio Argenziano
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 2/3] tests/gem_ctx_param: Update invalid param Antonio Argenziano
2018-01-25 1:00 ` [igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users Antonio Argenziano
@ 2018-01-25 1:18 ` Patchwork
2018-01-25 2:38 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2018-01-25 11:21 ` [igt-dev] [PATCH i-g-t v4 1/3] " Ville Syrjälä
4 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-01-25 1:18 UTC (permalink / raw)
To: Antonio Argenziano; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
URL : https://patchwork.freedesktop.org/series/37082/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
ec7d71660694157af698741eac3764203bf338ec igt/kms_frontbuffer_tracking: Show FBC status during the wait
with latest DRM-Tip kernel build CI_DRM_3683
9d8467fe5626 drm-tip: 2018y-01m-24d-19h-59m-41s UTC integration manifest
Testlist changes:
-igt@gem_ctx_param@basic
-igt@gem_ctx_param@basic-default
-igt@gem_ctx_param@invalid-ctx-get
-igt@gem_ctx_param@invalid-ctx-set
-igt@gem_ctx_param@invalid-param-get
-igt@gem_ctx_param@invalid-param-set
-igt@gem_ctx_param@invalid-size-get
-igt@gem_ctx_param@invalid-size-set
-igt@gem_ctx_param@non-root-set
-igt@gem_ctx_param@non-root-set-no-zeromap
-igt@gem_ctx_param@root-set
-igt@gem_ctx_param@root-set-no-zeromap-disabled
-igt@gem_ctx_param@root-set-no-zeromap-enabled
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test gem_mmap_gtt:
Subgroup basic-small-bo-tiledx:
fail -> PASS (fi-gdg-551) fdo#102575
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fi-bdw-5557u total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:21 time:421s
fi-bdw-gvtdvm total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:24 time:432s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:62 time:373s
fi-bsw-n3050 total:288 pass:240 dwarn:0 dfail:0 fail:0 skip:46 time:492s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:103 time:285s
fi-bxt-dsi total:288 pass:256 dwarn:0 dfail:0 fail:0 skip:30 time:486s
fi-bxt-j4205 total:288 pass:257 dwarn:0 dfail:0 fail:0 skip:29 time:489s
fi-byt-j1900 total:288 pass:251 dwarn:0 dfail:0 fail:0 skip:35 time:472s
fi-byt-n2820 total:288 pass:247 dwarn:0 dfail:0 fail:0 skip:39 time:458s
fi-elk-e7500 total:224 pass:168 dwarn:9 dfail:1 fail:0 skip:43
fi-gdg-551 total:288 pass:180 dwarn:0 dfail:0 fail:0 skip:106 time:279s
fi-glk-1 total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:28 time:515s
fi-hsw-4770 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:27 time:393s
fi-hsw-4770r total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:27 time:403s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:58 time:413s
fi-ivb-3520m total:288 pass:257 dwarn:0 dfail:0 fail:0 skip:29 time:461s
fi-ivb-3770 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:33 time:413s
fi-kbl-7500u total:288 pass:261 dwarn:1 dfail:0 fail:0 skip:24 time:461s
fi-kbl-7560u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:19 time:497s
fi-kbl-7567u total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:20 time:450s
fi-kbl-r total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:27 time:500s
fi-pnv-d510 total:288 pass:222 dwarn:1 dfail:0 fail:0 skip:63 time:578s
fi-skl-6260u total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:20 time:427s
fi-skl-6600u total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:27 time:511s
fi-skl-6700hq total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:26 time:528s
fi-skl-6700k2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:24 time:492s
fi-skl-6770hq total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:20 time:486s
fi-skl-guc total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:28 time:418s
fi-skl-gvtdvm total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:23 time:434s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:288 pass:246 dwarn:0 dfail:0 fail:0 skip:40 time:403s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:26 time:568s
fi-glk-dsi total:288 pass:256 dwarn:0 dfail:0 fail:0 skip:30 time:477s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_822/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 1:00 [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array Antonio Argenziano
` (2 preceding siblings ...)
2018-01-25 1:18 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array Patchwork
@ 2018-01-25 2:38 ` Patchwork
2018-01-25 11:21 ` [igt-dev] [PATCH i-g-t v4 1/3] " Ville Syrjälä
4 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-01-25 2:38 UTC (permalink / raw)
To: Antonio Argenziano; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,v4,1/3] lib/igt_aux: Add function to swap int64 in array
URL : https://patchwork.freedesktop.org/series/37082/
State : failure
== Summary ==
Test pm_rc6_residency:
Subgroup rc6-accuracy:
skip -> PASS (shard-snb)
Test kms_flip:
Subgroup 2x-plain-flip-fb-recreate:
pass -> FAIL (shard-hsw) fdo#100368 +2
Subgroup 2x-flip-vs-modeset-interruptible:
dmesg-warn -> PASS (shard-hsw)
Subgroup flip-vs-expired-vblank-interruptible:
pass -> FAIL (shard-apl) fdo#102887
Subgroup 2x-wf_vblank-ts-check-interruptible:
pass -> FAIL (shard-hsw) fdo#103928
Test kms_sysfs_edid_timing:
pass -> WARN (shard-apl) fdo#100047
Test gem_pwrite:
Subgroup big-gtt-forwards:
pass -> SKIP (shard-apl)
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-crc-atomic:
fail -> PASS (shard-apl)
Test gem_eio:
Subgroup in-flight-contexts:
fail -> PASS (shard-hsw) fdo#104676
Test perf_pmu:
Subgroup busy-double-start-vcs0:
pass -> INCOMPLETE (shard-apl)
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#104676 https://bugs.freedesktop.org/show_bug.cgi?id=104676
shard-apl total:2807 pass:1725 dwarn:1 dfail:0 fail:21 skip:1058 time:12333s
shard-hsw total:2825 pass:1723 dwarn:1 dfail:0 fail:10 skip:1090 time:12022s
shard-snb total:2825 pass:1319 dwarn:1 dfail:0 fail:9 skip:1496 time:6554s
Blacklisted hosts:
shard-kbl total:2825 pass:1826 dwarn:36 dfail:0 fail:23 skip:940 time:9533s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_822/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 1:00 [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array Antonio Argenziano
` (3 preceding siblings ...)
2018-01-25 2:38 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-01-25 11:21 ` Ville Syrjälä
2018-01-25 11:32 ` Chris Wilson
2018-01-25 11:35 ` Jani Nikula
4 siblings, 2 replies; 16+ messages in thread
From: Ville Syrjälä @ 2018-01-25 11:21 UTC (permalink / raw)
To: Antonio Argenziano; +Cc: igt-dev
On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> ---
> lib/igt_aux.c | 19 +++++++++++++++++++
> lib/igt_aux.h | 1 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 8ca0b60d..8012d8ae 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
> int_arr[j] = tmp;
> }
>
> +/**
> + * igt_exchange_int64:
> + * @array: pointer to the array of int64_t
> + * @i: first position
> + * @j: second position
> + *
> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
> + * function for igt_permute_array().
> + */
> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
> +{
> + int64_t *int_arr, tmp;
Why void* when you really mean int64_t*?
> + int_arr = array;
> +
> + tmp = int_arr[i];
> + int_arr[i] = int_arr[j];
> + int_arr[j] = tmp;
igt_swap()
> +}
> +
> /**
> * igt_permute_array:
> * @array: pointer to array
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 02e70126..60d4de71 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
> for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>
> void igt_exchange_int(void *array, unsigned i, unsigned j);
> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
> void igt_permute_array(void *array, unsigned size,
> void (*exchange_func)(void *array,
> unsigned i,
> --
> 2.14.2
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Ville Syrjälä
Intel OTC
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 11:21 ` [igt-dev] [PATCH i-g-t v4 1/3] " Ville Syrjälä
@ 2018-01-25 11:32 ` Chris Wilson
2018-01-25 11:35 ` Jani Nikula
1 sibling, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2018-01-25 11:32 UTC (permalink / raw)
To: Ville Syrjälä, Antonio Argenziano; +Cc: igt-dev
Quoting Ville Syrjälä (2018-01-25 11:21:50)
> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
> > Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > ---
> > lib/igt_aux.c | 19 +++++++++++++++++++
> > lib/igt_aux.h | 1 +
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 8ca0b60d..8012d8ae 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
> > int_arr[j] = tmp;
> > }
> >
> > +/**
> > + * igt_exchange_int64:
> > + * @array: pointer to the array of int64_t
> > + * @i: first position
> > + * @j: second position
> > + *
> > + * Exchanges the two values at array indices @i and @j. Useful as an exchange
> > + * function for igt_permute_array().
> > + */
> > +void igt_exchange_int64(void *array, unsigned i, unsigned j)
> > +{
> > + int64_t *int_arr, tmp;
>
> Why void* when you really mean int64_t*?
To fit the function prototype for igt_permute_array().
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 11:21 ` [igt-dev] [PATCH i-g-t v4 1/3] " Ville Syrjälä
2018-01-25 11:32 ` Chris Wilson
@ 2018-01-25 11:35 ` Jani Nikula
2018-01-26 16:50 ` Antonio Argenziano
1 sibling, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2018-01-25 11:35 UTC (permalink / raw)
To: Ville Syrjälä, Antonio Argenziano; +Cc: igt-dev
On Thu, 25 Jan 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>> ---
>> lib/igt_aux.c | 19 +++++++++++++++++++
>> lib/igt_aux.h | 1 +
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>> index 8ca0b60d..8012d8ae 100644
>> --- a/lib/igt_aux.c
>> +++ b/lib/igt_aux.c
>> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
>> int_arr[j] = tmp;
>> }
>>
>> +/**
>> + * igt_exchange_int64:
>> + * @array: pointer to the array of int64_t
>> + * @i: first position
>> + * @j: second position
>> + *
>> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
>> + * function for igt_permute_array().
>> + */
>> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
>> +{
>> + int64_t *int_arr, tmp;
>
> Why void* when you really mean int64_t*?
>
>> + int_arr = array;
>> +
>> + tmp = int_arr[i];
>> + int_arr[i] = int_arr[j];
>> + int_arr[j] = tmp;
>
> igt_swap()
All of the igt_exchange_whatever functions could be replaced with a
single macro that does typeof on the array parameter.
BR,
Jani.
>
>> +}
>> +
>> /**
>> * igt_permute_array:
>> * @array: pointer to array
>> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>> index 02e70126..60d4de71 100644
>> --- a/lib/igt_aux.h
>> +++ b/lib/igt_aux.h
>> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
>> for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>>
>> void igt_exchange_int(void *array, unsigned i, unsigned j);
>> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
>> void igt_permute_array(void *array, unsigned size,
>> void (*exchange_func)(void *array,
>> unsigned i,
>> --
>> 2.14.2
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [igt-dev] [PATCH i-g-t v4 1/3] lib/igt_aux: Add function to swap int64 in array
2018-01-25 11:35 ` Jani Nikula
@ 2018-01-26 16:50 ` Antonio Argenziano
0 siblings, 0 replies; 16+ messages in thread
From: Antonio Argenziano @ 2018-01-26 16:50 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä; +Cc: igt-dev
On 25/01/18 03:35, Jani Nikula wrote:
> On Thu, 25 Jan 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Wed, Jan 24, 2018 at 05:00:01PM -0800, Antonio Argenziano wrote:
>>> Signed-off-by: Antonio Argenziano <antonio.argenziano@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>>> ---
>>> lib/igt_aux.c | 19 +++++++++++++++++++
>>> lib/igt_aux.h | 1 +
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>>> index 8ca0b60d..8012d8ae 100644
>>> --- a/lib/igt_aux.c
>>> +++ b/lib/igt_aux.c
>>> @@ -577,6 +577,25 @@ void igt_exchange_int(void *array, unsigned i, unsigned j)
>>> int_arr[j] = tmp;
>>> }
>>>
>>> +/**
>>> + * igt_exchange_int64:
>>> + * @array: pointer to the array of int64_t
>>> + * @i: first position
>>> + * @j: second position
>>> + *
>>> + * Exchanges the two values at array indices @i and @j. Useful as an exchange
>>> + * function for igt_permute_array().
>>> + */
>>> +void igt_exchange_int64(void *array, unsigned i, unsigned j)
>>> +{
>>> + int64_t *int_arr, tmp;
>>
>> Why void* when you really mean int64_t*?
>>
>>> + int_arr = array;
>>> +
>>> + tmp = int_arr[i];
>>> + int_arr[i] = int_arr[j];
>>> + int_arr[j] = tmp;
>>
>> igt_swap()
>
>
> All of the igt_exchange_whatever functions could be replaced with a
> single macro that does typeof on the array parameter.
Basically that would be igt_swap(). You would have to replace the
permute interface to make it not take a void pointer but a generic array.
I tried the following:
#define igt_permute_array(array, size) do { \
int _i; \
typeof(array) _array; \
_array = array; \
\
for (_i = size - 1; _i > 0; _i--) { \
/* yes, not perfectly uniform, who cares */ \
long _l = hars_petruska_f54_1_random_unsafe() % (_i +1); \
if (_i != _l) \
igt_swap(_array[_i], _array[_l]); \
} \
} while (0)
The problem I had is that typeof() doesn't seems to play nice with
arrays but wants a pointer type.
So it would be a pervasive change and I would prefer to make it into a
separate patchset.
Thanks,
Antonio
>
> BR,
> Jani.
>
>>
>>> +}
>>> +
>>> /**
>>> * igt_permute_array:
>>> * @array: pointer to array
>>> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>>> index 02e70126..60d4de71 100644
>>> --- a/lib/igt_aux.h
>>> +++ b/lib/igt_aux.h
>>> @@ -117,6 +117,7 @@ bool __igt_sigiter_continue(struct __igt_sigiter *iter, bool interrupt);
>>> for (struct timespec t__={}; igt_nsec_elapsed(&t__)>>20 < (t); )
>>>
>>> void igt_exchange_int(void *array, unsigned i, unsigned j);
>>> +void igt_exchange_int64(void *array, unsigned i, unsigned j);
>>> void igt_permute_array(void *array, unsigned size,
>>> void (*exchange_func)(void *array,
>>> unsigned i,
>>> --
>>> 2.14.2
>>>
>>> _______________________________________________
>>> igt-dev mailing list
>>> igt-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 16+ messages in thread