[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