[libvirt] util: allow virProcessSetScheduler to set SCHED_DEADLINE
Martin Polednik
mpolednik at redhat.com
Tue Nov 15 15:26:58 UTC 2016
On 15/11/16 15:02 +0000, Daniel P. Berrange wrote:
>On Tue, Nov 15, 2016 at 03:51:22PM +0100, Martin Polednik wrote:
>> On 10/11/16 16:21 +0100, Martin Kletzander wrote:
>> > On Mon, Nov 07, 2016 at 10:01:13AM +0100, Martin Polednik wrote:
>> > > As sched_deadline is linux specific, it is not set through
>> > > sched_setscheduler but rather the sched_setattr syscall. Additionally,
>> > > the scheduler has new set of parameters: runtime, deadline and period.
>> > >
>> > > In this part of the series, we extend virProcessSetScheduler to
>> > > accommodate the additional parameters and use sched_setattr syscall to
>> > > set SCHED_DEADLINE. Another small addition is sched_attr struct, which
>> > > is required for sched_setattr and not exposed from the kernel or
>> > > libraries.
>> > > ---
>> > > src/qemu/qemu_process.c | 3 ++-
>> > > src/util/virprocess.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++--
>> > > src/util/virprocess.h | 28 +++++++++++++++++++-
>> > > 3 files changed, 97 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> > > index 1b67aee..91a635c 100644
>> > > --- a/src/qemu/qemu_process.c
>> > > +++ b/src/qemu/qemu_process.c
>> > > @@ -2395,7 +2395,8 @@ qemuProcessSetupPid(virDomainObjPtr vm,
>> > >
>> > > /* Set scheduler type and priority. */
>> > > if (sched &&
>> > > - virProcessSetScheduler(pid, sched->policy, sched->priority) < 0)
>> > > + virProcessSetScheduler(pid, sched->policy, sched->priority,
>> > > + sched->runtime, sched->deadline, sched->period) < 0)
>> > > goto cleanup;
>> > >
>> > > ret = 0;
>> > > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>> > > index d68800b..cd1cc9b 100644
>> > > --- a/src/util/virprocess.c
>> > > +++ b/src/util/virprocess.c
>> > > @@ -70,6 +70,7 @@
>> > > VIR_LOG_INIT("util.process");
>> > >
>> > > #ifdef __linux__
>> > > +# include <sys/syscall.h>
>> > > /*
>> > > * Workaround older glibc. While kernel may support the setns
>> > > * syscall, the glibc wrapper might not exist. If that's the
>> > > @@ -91,9 +92,14 @@ VIR_LOG_INIT("util.process");
>> > > # endif
>> > > # endif
>> > >
>> > > +# ifndef __NR_sched_setattr
>> > > +# if defined(__x86_64__)
>> > > +# define __NR_sched_setattr 314
>> > > +# endif
>> > > +# endif
>> > > +
>> >
>> > At least ARMs, POWERs and i386 should be defined from the start so that
>> > tests in CI don't fail right away. And other platforms should error out
>> > right away. But honestly, I'd drop this compatibility code. What
>> > you're adding here is not a bleeding-edge feature.
>>
>> It maybe makes sense to virReportSystemError in case it's not defined
>> -- although the feature isn't cutting edge, older kernels (such as
>> 3.10 on CentOS) do not have the syscall (or the number)
>> unless they're rebases with RT_PREEMPT patch.
>>
>> > > # ifndef HAVE_SETNS
>> > > # if defined(__NR_setns)
>> > > -# include <sys/syscall.h>
>> > >
>> > > static inline int setns(int fd, int nstype)
>> > > {
>> > > @@ -103,6 +109,13 @@ static inline int setns(int fd, int nstype)
>> > > # error Please determine the syscall number for setns on your architecture
>> > > # endif
>> > > # endif
>> > > +
>> > > +static inline int sched_setattr(pid_t pid,
>> > > + const struct sched_attr *attr,
>> > > + unsigned int flags)
>> > > +{
>> > > + return syscall(__NR_sched_setattr, pid, attr, flags);
>> > > +}
>> > > #else /* !__linux__ */
>> > > static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
>> > > {
>> > > @@ -110,6 +123,15 @@ static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype ATTRIBUTE_UNUSED)
>> > > _("Namespaces are not supported on this platform."));
>> > > return -1;
>> > > }
>> > > +
>> > > +static inline int sched_setattr(pid_t pid,
>> > > + const struct sched_attr *attr,
>> > > + unsigned int flags)
>> > > +{
>> > > + virReportSystemError(ENOSYS, "%s",
>> > > + _("Deadline scheduler is not supported on this platform."));
>> > > + return -1;
>> > > +}
>> > > #endif
>> > >
>> > > VIR_ENUM_IMPL(virProcessSchedPolicy, VIR_PROC_POLICY_LAST,
>> > > @@ -1225,7 +1247,10 @@ virProcessSchedTranslatePolicy(virProcessSchedPolicy policy)
>> > > int
>> > > virProcessSetScheduler(pid_t pid,
>> > > virProcessSchedPolicy policy,
>> > > - int priority)
>> > > + int priority,
>> > > + unsigned long long runtime,
>> > > + unsigned long long deadline,
>> > > + unsigned long long period)
>> > > {
>> > > struct sched_param param = {0};
>> > > int pol = virProcessSchedTranslatePolicy(policy);
>> > > @@ -1235,6 +1260,47 @@ virProcessSetScheduler(pid_t pid,
>> > > if (!policy)
>> > > return 0;
>> > >
>> > > + if (pol == SCHED_DEADLINE) {
>> > > + int ret = 0;
>> > > +
>> > > + if (runtime < 1024 || deadline < 1024 || period < 24) {
>> > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> > > + _("Scheduler runtime, deadline and period must be "
>> > > + "higher or equal to 1024 (1 us) (runtime(%llu), "
>> > > + "deadline(%llu), period(%llu))"), runtime, deadline, period);
>> > > + return -1;
>> > > + }
>> > > +
>> > > + if (!(runtime <= deadline && deadline <= period)) {
>> > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> > > + _("Scheduler configuration does not satisfy "
>> > > + "(runtime(%llu) <= deadline(%llu) <= period(%llu))"),
>> > > + runtime, deadline, period);
>> > > + return -1;
>> > > + }
>> > > +
>> > > + struct sched_attr attrs = {
>> > > + .size = sizeof(attrs),
>> > > + .sched_policy = SCHED_DEADLINE,
>> > > + .sched_flags = 0,
>> > > + .sched_nice = 0,
>> > > + .sched_priority = 0,
>> > > + .sched_runtime = runtime,
>> > > + .sched_deadline = deadline,
>> > > + .sched_period = period,
>> > > + };
>> > > +
>> >
>> > C99 initialization is forbidden in libvirt (but we have no syntax-check
>> > rule for it), move it to the top of the code block.
>> >
>> > > + ret = sched_setattr(pid, &attrs, 0);
>> > > + if (ret != 0) {
>> > > + virReportSystemError(errno,
>> > > + _("Cannot set scheduler parameters for pid %d"),
>> > > + pid);
>> > > + return -1;
>> > > + }
>> > > +
>> > > + return 0;
>> > > + }
>> > > +
>> > > if (pol == SCHED_FIFO || pol == SCHED_RR) {
>> > > int min = 0;
>> > > int max = 0;
>> > > diff --git a/src/util/virprocess.h b/src/util/virprocess.h
>> > > index 1eac3e6..e6914e7 100644
>> > > --- a/src/util/virprocess.h
>> > > +++ b/src/util/virprocess.h
>> > > @@ -23,6 +23,9 @@
>> > > # define __VIR_PROCESS_H__
>> > >
>> > > # include <sys/types.h>
>> > > +# ifdef __linux__
>> > > +# include <linux/types.h>
>> > > +# endif
>> > >
>> > > # include "internal.h"
>> > > # include "virbitmap.h"
>> > > @@ -39,6 +42,26 @@ typedef enum {
>> > > VIR_PROC_POLICY_LAST
>> > > } virProcessSchedPolicy;
>> > >
>> > > +# ifdef __linux__
>> > > +struct sched_attr {
>> > > + __u32 size;
>> > > +
>> > > + __u32 sched_policy;
>> > > + __u64 sched_flags;
>> > > +
>> > > + /* SCHED_NORMAL, SCHED_BATCH */
>> > > + __s32 sched_nice;
>> > > +
>> > > + /* SCHED_FIFO, SCHED_RR */
>> > > + __u32 sched_priority;
>> > > +
>> > > + /* SCHED_DEADLINE (nsec) */
>> > > + __u64 sched_runtime;
>> > > + __u64 sched_deadline;
>> > > + __u64 sched_period;
>> > > +};
>> > > +# endif
>> > > +
>> >
>> > I don't like this redefinition. Not only because it uses __uXX instead
>> > of uint##_t. I think we should have a configure.ac check for
>> > HAVE_STRUCT_SCHED_ATTR (as we do with some other structs) and just error
>> > out if it's not available. If we're doing that, we can safely do the
>> > same thing with SCHED_DEADLINE and not have to carry this code on for
>> > decades.
>>
>> The struct doesn't seem to be present in (g)libc or any other
>> library. I'm afraid we can't therefore simply error out.
>
>Can we find out why it isn't present in glibc. Is it simply that it
>is too new ? If so, IMHO desirable to just wait for glibc to support
>it and make libvirt code conditional on the new version.
My feeling is the novelty combined with limited uses in anything but
RT applications. That being said, I was unable to find anything
mentioning sched_attr and glibc together.
I'm slightly afraid of the time for the whole
glibc->libvirt->management layer dance.
>Regards,
>Daniel
>--
>|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org -o- http://virt-manager.org :|
>|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
More information about the libvir-list
mailing list