[igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
Chris Wilson
chris at chris-wilson.co.uk
Thu Jan 25 11:37:24 UTC 2018
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 at chris-wilson.co.uk>
> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski at 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
>
More information about the igt-dev
mailing list