[igt-dev] [PATCH i-g-t v4 3/3] tests/gem_ctx_param: Add set_priority tests for non SYS_NICE users
Antonio Argenziano
antonio.argenziano at intel.com
Wed Jan 31 00:47:57 UTC 2018
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 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.
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
>>
More information about the igt-dev
mailing list