Re: [PATCH v2 1/3] util: Add virProcessSetScheduler() function for scheduler settings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10.02.2015 16:35, Martin Kletzander wrote:
> This function uses sched_setscheduler() function so it works with
> processes and threads as well (even threads not created by us, which is
> what we'll need in the future).
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
>  configure.ac             |   4 +-
>  src/libvirt_private.syms |   1 +
>  src/util/virprocess.c    | 104 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virprocess.h    |  20 ++++++++-
>  4 files changed, 124 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 99a2283..b3e99e7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,6 +1,6 @@
>  dnl Process this file with autoconf to produce a configure script.
> 
> -dnl Copyright (C) 2005-2014 Red Hat, Inc.
> +dnl Copyright (C) 2005-2015 Red Hat, Inc.
>  dnl
>  dnl This library is free software; you can redistribute it and/or
>  dnl modify it under the terms of the GNU Lesser General Public
> @@ -275,7 +275,7 @@ dnl and various less common threadsafe functions
>  AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
>    getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
>    posix_memalign prlimit regexec sched_getaffinity setgroups setns \
> -  setrlimit symlink sysctlbyname getifaddrs])
> +  setrlimit symlink sysctlbyname getifaddrs sched_setscheduler])
> 
>  dnl Availability of pthread functions. Because of $LIB_PTHREAD, we
>  dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 376c69b..79fc14f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1915,6 +1915,7 @@ virProcessSetMaxFiles;
>  virProcessSetMaxMemLock;
>  virProcessSetMaxProcesses;
>  virProcessSetNamespaces;
> +virProcessSetScheduler;
>  virProcessTranslateStatus;
>  virProcessWait;
> 
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index d0a1500..c030d74 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1,7 +1,7 @@
>  /*
>   * virprocess.c: interaction with processes
>   *
> - * Copyright (C) 2010-2014 Red Hat, Inc.
> + * Copyright (C) 2010-2015 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -32,7 +32,9 @@
>  # include <sys/time.h>
>  # include <sys/resource.h>
>  #endif
> -#include <sched.h>
> +#if HAVE_SCHED_SETSCHEDULER
> +# include <sched.h>
> +#endif
> 
>  #if defined(__FreeBSD__) || HAVE_BSD_CPU_AFFINITY
>  # include <sys/param.h>
> @@ -104,6 +106,13 @@ static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
>  }
>  #endif
> 
> +VIR_ENUM_IMPL(virProcessSchedPolicy, VIR_PROC_POLICY_LAST,
> +              "none",
> +              "batch",
> +              "idle",
> +              "fifo",
> +              "rr");
> +
>  /**
>   * virProcessTranslateStatus:
>   * @status: child exit status to translate
> @@ -1052,3 +1061,94 @@ virProcessExitWithStatus(int status)
>      }
>      exit(value);
>  }
> +
> +#if HAVE_SCHED_SETSCHEDULER
> +
> +static int
> +virProcessSchedTranslatePolicy(virProcessSchedPolicy policy)
> +{
> +    switch (policy) {
> +    case VIR_PROC_POLICY_NONE:
> +        return SCHED_OTHER;
> +
> +    case VIR_PROC_POLICY_BATCH:
> +        return SCHED_BATCH;
> +
> +    case VIR_PROC_POLICY_IDLE:
> +        return SCHED_IDLE;
> +
> +    case VIR_PROC_POLICY_FIFO:
> +        return SCHED_FIFO;
> +
> +    case VIR_PROC_POLICY_RR:
> +        return SCHED_RR;
> +
> +    case VIR_PROC_POLICY_LAST:
> +        /* nada */
> +        break;
> +    }
> +
> +    return -1;
> +}
> +
> +int
> +virProcessSetScheduler(pid_t pid, virProcessSchedPolicy policy, int priority)
> +{
> +    struct sched_param param = {0};
> +    int pol = virProcessSchedTranslatePolicy(policy);
> +
> +    VIR_DEBUG("pid=%d, policy=%d, priority=%u", pid, policy, priority);
> +
> +    if (!policy)
> +        return 0;

In order to do this, you have to make sure enum has a zero value
element. I think compilers don't guarantee enum values <-> integer
mapping unless told so.

> +
> +    if (pol == SCHED_FIFO || pol == SCHED_RR) {
> +        int min = 0;
> +        int max = 0;
> +
> +        if ((min = sched_get_priority_min(pol)) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("Cannot get minimum scheduler "
> +                                   "priority value"));
> +            return -1;
> +        }
> +
> +        if ((max = sched_get_priority_max(pol)) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("Cannot get maximum scheduler "
> +                                   "priority value"));
> +            return -1;
> +        }
> +
> +        if (priority < min || priority > max) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Scheduler priority %d out of range [%d, %d]"),
> +                           priority, min, max);

CONFIG? I'd say ARGUMENT, but we have the mess in error codes anyway and
this doesn't make situation any worse :)

> +            return -1;
> +        }
> +
> +        param.sched_priority = priority;
> +    }
> +
> +    if (sched_setscheduler(pid, pol, &param) < 0) {
> +        virReportSystemError(errno,
> +                             _("Cannot set scheduler parameters for pid %d"),
> +                             pid);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +#else /* ! HAVE_SCHED_SETSCHEDULER */
> +
> +int
> +virProcessSetScheduler(pid_t pid, int policy, int priority)

ATTRIBUTE_UNUSED for arguments ^^. Although, would it be worth to copy
the check I've pointed out in here too? That is, even if
sched_setscheduler() is not available, and virProcessSetScheduler(pid,
0, prio); is called, return success instead of error (of course return
error for any other policy than VIR_PROC_POLICY_NONE).

> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Process CPU scheduling is not supported "
> +                           "on this platform"));
> +    return -1;
> +}
> +
> +#endif /* !HAVE_SCHED_SETSCHEDULER */
> diff --git a/src/util/virprocess.h b/src/util/virprocess.h
> index bcaede5..8b95560 100644
> --- a/src/util/virprocess.h
> +++ b/src/util/virprocess.h
> @@ -1,7 +1,7 @@
>  /*
>   * virprocess.h: interaction with processes
>   *
> - * Copyright (C) 2010-2014 Red Hat, Inc.
> + * Copyright (C) 2010-2015 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -26,6 +26,19 @@
> 
>  # include "internal.h"
>  # include "virbitmap.h"
> +# include "virutil.h"
> +
> +typedef enum {
> +    VIR_PROC_POLICY_NONE,
> +    VIR_PROC_POLICY_BATCH,
> +    VIR_PROC_POLICY_IDLE,
> +    VIR_PROC_POLICY_FIFO,
> +    VIR_PROC_POLICY_RR,
> +
> +    VIR_PROC_POLICY_LAST
> +} virProcessSchedPolicy;
> +
> +VIR_ENUM_DECL(virProcessSchedPolicy);

This ^^ introduces virProcessSchedPolicyType{From,To}String(). However,
I don't see the symbols exported.

> 
>  char *
>  virProcessTranslateStatus(int status);
> @@ -73,4 +86,9 @@ typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
>  int virProcessRunInMountNamespace(pid_t pid,
>                                    virProcessNamespaceCallback cb,
>                                    void *opaque);
> +
> +int virProcessSetScheduler(pid_t pid,
> +                           virProcessSchedPolicy policy,
> +                           int priority);
> +
>  #endif /* __VIR_PROCESS_H__ */
> 

ACK with all of that fixed.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]