Module: xenomai-3
Branch: next
Commit: 3c2ffd0b7d3fe82401d4036a5ad45b6eca51185b
URL:    
http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=3c2ffd0b7d3fe82401d4036a5ad45b6eca51185b

Author: Philippe Gerum <[email protected]>
Date:   Mon Mar 19 11:20:13 2018 +0100

cobalt/posix/timer: fix CPU affinity tracking

Timers may have specific CPU affinity requirements in that their
backing clock device might not beat on all available CPUs, but only on
a subset of them.

The CPU affinity of every timer bound to a particular thread has to be
tracked each time such timer is started, so that no core timer is
queued to a per-CPU list which won't receive any event from the
backing clock device.

Such tracking was missing for timerfd and POSIX timers, along with
internal timers running the sporadic scheduling policy.

At this chance, the timer affinity code was cleaned up by folding all
the affinity selection logic into a single call,
i.e. xntimer_set_affinity().

---

 include/cobalt/kernel/clock.h  |    9 ----
 include/cobalt/kernel/timer.h  |   21 +++++---
 kernel/cobalt/clock.c          |   28 ----------
 kernel/cobalt/posix/timer.c    |   11 ++--
 kernel/cobalt/posix/timerfd.c  |    4 +-
 kernel/cobalt/sched-quota.c    |    2 -
 kernel/cobalt/sched-sporadic.c |    4 +-
 kernel/cobalt/sched-tp.c       |    1 -
 kernel/cobalt/thread.c         |   31 +++--------
 kernel/cobalt/timer.c          |  113 ++++++++++++++++++++++++----------------
 10 files changed, 99 insertions(+), 125 deletions(-)

diff --git a/include/cobalt/kernel/clock.h b/include/cobalt/kernel/clock.h
index bbdec93..8792dcf 100644
--- a/include/cobalt/kernel/clock.h
+++ b/include/cobalt/kernel/clock.h
@@ -263,15 +263,6 @@ static inline int xnclock_adjust_time(struct xnclock 
*clock,
        return clock->ops.adjust_time(clock, tx);
 }
 
-#ifdef CONFIG_SMP
-int xnclock_get_default_cpu(struct xnclock *clock, int cpu);
-#else
-static inline int xnclock_get_default_cpu(struct xnclock *clock, int cpu)
-{
-       return cpu;
-}
-#endif
-
 static inline xnticks_t xnclock_get_offset(struct xnclock *clock)
 {
        return clock->wallclock_offset;
diff --git a/include/cobalt/kernel/timer.h b/include/cobalt/kernel/timer.h
index 0897d28..3ea2d3c 100644
--- a/include/cobalt/kernel/timer.h
+++ b/include/cobalt/kernel/timer.h
@@ -523,14 +523,23 @@ int xntimer_setup_ipi(void);
 
 void xntimer_release_ipi(void);
 
-bool xntimer_set_sched(struct xntimer *timer,
-                      struct xnsched *sched);
+void __xntimer_set_affinity(struct xntimer *timer,
+                           struct xnsched *sched);
+
+static inline void xntimer_set_affinity(struct xntimer *timer,
+                                       struct xnsched *sched)
+{
+       if (sched != xntimer_sched(timer))
+               __xntimer_set_affinity(timer, sched);
+}
 
 #else /* ! CONFIG_SMP */
 
 static inline void xntimer_migrate(struct xntimer *timer,
                                   struct xnsched *sched)
-{ }
+{
+       timer->sched = sched;
+}
 
 static inline int xntimer_setup_ipi(void)
 {
@@ -539,10 +548,10 @@ static inline int xntimer_setup_ipi(void)
 
 static inline void xntimer_release_ipi(void) { }
 
-static inline bool xntimer_set_sched(struct xntimer *timer,
-                                    struct xnsched *sched)
+static inline void xntimer_set_affinity(struct xntimer *timer,
+                                       struct xnsched *sched)
 {
-       return true;
+       xntimer_migrate(timer, sched);
 }
 
 #endif /* CONFIG_SMP */
diff --git a/kernel/cobalt/clock.c b/kernel/cobalt/clock.c
index 3e031e4..30ac264 100644
--- a/kernel/cobalt/clock.c
+++ b/kernel/cobalt/clock.c
@@ -325,34 +325,6 @@ xnticks_t xnclock_core_read_monotonic(void)
 }
 EXPORT_SYMBOL_GPL(xnclock_core_read_monotonic);
 
-#ifdef CONFIG_SMP
-
-int xnclock_get_default_cpu(struct xnclock *clock, int cpu)
-{
-       cpumask_t set;
-       /*
-        * Check a CPU number against the possible set of CPUs
-        * receiving events from the underlying clock device. If the
-        * suggested CPU does not receive events from this device,
-        * return the first one which does.  We also account for the
-        * dynamic set of real-time CPUs.
-        *
-        * A clock device with no percpu semantics causes this routine
-        * to return CPU0 unconditionally.
-        */
-       if (cpumask_empty(&clock->affinity))
-               return 0;
-       
-       cpumask_and(&set, &clock->affinity, &cobalt_cpu_affinity);
-       if (!cpumask_empty(&set) && !cpumask_test_cpu(cpu, &set))
-               cpu = cpumask_first(&set);
-
-       return cpu;
-}
-EXPORT_SYMBOL_GPL(xnclock_get_default_cpu);
-
-#endif /* !CONFIG_SMP */
-
 #ifdef CONFIG_XENO_OPT_STATS
 
 static struct xnvfile_directory timerlist_vfroot;
diff --git a/kernel/cobalt/posix/timer.c b/kernel/cobalt/posix/timer.c
index 0a935f9..3aca90d 100644
--- a/kernel/cobalt/posix/timer.c
+++ b/kernel/cobalt/posix/timer.c
@@ -326,11 +326,8 @@ static inline int timer_set(struct cobalt_timer *timer, 
int flags,
        /*
         * No extension, or operation not handled. Default to plain
         * POSIX behavior.
-        */
-
-       /*
-        * If the target thread vanished, simply don't start the
-        * timer.
+        *
+        * If the target thread vanished, just don't start the timer.
         */
        thread = cobalt_thread_find(timer->target);
        if (thread == NULL)
@@ -338,9 +335,9 @@ static inline int timer_set(struct cobalt_timer *timer, int 
flags,
 
        /*
         * Make the timer affine to the CPU running the thread to be
-        * signaled.
+        * signaled if possible.
         */
-       xntimer_set_sched(&timer->timerbase, thread->threadbase.sched);
+       xntimer_set_affinity(&timer->timerbase, thread->threadbase.sched);
 
        return __cobalt_timer_setval(&timer->timerbase,
                                     clock_flag(flags, timer->clockid), value);
diff --git a/kernel/cobalt/posix/timerfd.c b/kernel/cobalt/posix/timerfd.c
index 217e68a..f4411d4 100644
--- a/kernel/cobalt/posix/timerfd.c
+++ b/kernel/cobalt/posix/timerfd.c
@@ -120,7 +120,7 @@ timerfd_select(struct rtdm_fd *fd, struct xnselector 
*selector,
                return -ENOMEM;
 
        xnlock_get_irqsave(&nklock, s);
-       xntimer_set_sched(&tfd->timer, xnsched_current());
+       xntimer_set_affinity(&tfd->timer, xnthread_current()->sched);
        err = xnselect_bind(&tfd->read_select, binding, selector, type,
                        index, tfd->flags & COBALT_TFD_TICKED);
        xnlock_put_irqrestore(&nklock, s);
@@ -269,7 +269,7 @@ int __cobalt_timerfd_settime(int fd, int flags,
        if (ovalue)
                __cobalt_timer_getval(&tfd->timer, ovalue);
 
-       xntimer_set_sched(&tfd->timer, xnsched_current());
+       xntimer_set_affinity(&tfd->timer, xnthread_current()->sched);
 
        ret = __cobalt_timer_setval(&tfd->timer,
                                    clock_flag(cflag, tfd->clockid), value);
diff --git a/kernel/cobalt/sched-quota.c b/kernel/cobalt/sched-quota.c
index 5a40df2..3f69f8b 100644
--- a/kernel/cobalt/sched-quota.c
+++ b/kernel/cobalt/sched-quota.c
@@ -226,12 +226,10 @@ static void xnsched_quota_init(struct xnsched *sched)
 #endif
        xntimer_init(&qs->refill_timer,
                     &nkclock, quota_refill_handler, sched, XNTIMER_IGRAVITY);
-       xntimer_set_sched(&qs->refill_timer, sched);
        xntimer_set_name(&qs->refill_timer, refiller_name);
 
        xntimer_init(&qs->limit_timer,
                     &nkclock, quota_limit_handler, sched, XNTIMER_IGRAVITY);
-       xntimer_set_sched(&qs->limit_timer, sched);
        xntimer_set_name(&qs->limit_timer, limiter_name);
 }
 
diff --git a/kernel/cobalt/sched-sporadic.c b/kernel/cobalt/sched-sporadic.c
index 8a74ee4..4d307a8 100644
--- a/kernel/cobalt/sched-sporadic.c
+++ b/kernel/cobalt/sched-sporadic.c
@@ -106,6 +106,7 @@ static void sporadic_schedule_drop(struct xnthread *thread)
         * the thread is running, trading cycles at firing time
         * against cycles when arming the timer.
         */
+       xntimer_set_affinity(&pss->drop_timer, thread->sched);
        ret = xntimer_start(&pss->drop_timer, now + pss->budget,
                            XN_INFINITE, XN_ABSOLUTE);
        if (ret == -ETIMEDOUT) {
@@ -141,7 +142,7 @@ retry:
        } while(--pss->repl_pending > 0);
 
        if (pss->repl_pending > 0) {
-               xntimer_set_sched(&pss->repl_timer, thread->sched);
+               xntimer_set_affinity(&pss->repl_timer, thread->sched);
                ret = xntimer_start(&pss->repl_timer, pss->repl_data[r].date,
                                    XN_INFINITE, XN_ABSOLUTE);
                if (ret == -ETIMEDOUT)
@@ -188,6 +189,7 @@ static void sporadic_post_recharge(struct xnthread *thread, 
xnticks_t budget)
        pss->repl_in = (r + 1) % MAX_REPLENISH;
 
        if (pss->repl_pending++ == 0) {
+               xntimer_set_affinity(&pss->repl_timer, thread->sched);
                ret = xntimer_start(&pss->repl_timer, pss->repl_data[r].date,
                                    XN_INFINITE, XN_ABSOLUTE);
                /*
diff --git a/kernel/cobalt/sched-tp.c b/kernel/cobalt/sched-tp.c
index d65ac82..982d95e 100644
--- a/kernel/cobalt/sched-tp.c
+++ b/kernel/cobalt/sched-tp.c
@@ -103,7 +103,6 @@ static void xnsched_tp_init(struct xnsched *sched)
        INIT_LIST_HEAD(&tp->threads);
        xntimer_init(&tp->tf_timer, &nkclock, tp_tick_handler,
                     sched, XNTIMER_IGRAVITY);
-       xntimer_set_sched(&tp->tf_timer, sched);
        xntimer_set_name(&tp->tf_timer, timer_name);
 }
 
diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index 3d84ddd..6e6a659 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -58,23 +58,6 @@ static void timeout_handler(struct xntimer *timer)
        xnthread_resume(thread, XNDELAY);
 }
 
-static inline void fixup_ptimer_affinity(struct xnthread *thread)
-{
-#ifdef CONFIG_SMP
-       struct xntimer *timer = &thread->ptimer;
-       int cpu;
-       /*
-        * The thread a periodic timer is affine to might have been
-        * migrated to another CPU while passive. Fix this up.
-        */
-       if (thread->sched != timer->sched) {
-               cpu = xnclock_get_default_cpu(xntimer_clock(timer),
-                                             xnsched_cpu(thread->sched));
-               xntimer_set_sched(timer, xnsched_struct(cpu));
-       }
-#endif
-}
-
 static void periodic_handler(struct xntimer *timer)
 {
        struct xnthread *thread = container_of(timer, struct xnthread, ptimer);
@@ -85,7 +68,11 @@ static void periodic_handler(struct xntimer *timer)
        if (xnthread_test_state(thread, XNDELAY|XNPEND) == XNDELAY)
                xnthread_resume(thread, XNDELAY);
 
-       fixup_ptimer_affinity(thread);
+       /*
+        * The periodic thread might have migrated to another CPU
+        * while passive, fix the timer affinity if need be.
+        */
+       xntimer_set_affinity(&thread->ptimer, thread->sched);
 }
 
 static inline void enlist_new_thread(struct xnthread *thread)
@@ -941,7 +928,7 @@ void xnthread_suspend(struct xnthread *thread, int mask,
         * Don't start the timer for a thread delayed indefinitely.
         */
        if (timeout != XN_INFINITE || timeout_mode != XN_RELATIVE) {
-               xntimer_set_sched(&thread->rtimer, thread->sched);
+               xntimer_set_affinity(&thread->rtimer, thread->sched);
                if (xntimer_start(&thread->rtimer, timeout, XN_INFINITE,
                                  timeout_mode)) {
                        /* (absolute) timeout value in the past, bail out. */
@@ -1322,7 +1309,7 @@ EXPORT_SYMBOL_GPL(xnthread_unblock);
 int xnthread_set_periodic(struct xnthread *thread, xnticks_t idate,
                          xntmode_t timeout_mode, xnticks_t period)
 {
-       int ret = 0, cpu;
+       int ret = 0;
        spl_t s;
 
        if (thread == NULL) {
@@ -1358,9 +1345,7 @@ int xnthread_set_periodic(struct xnthread *thread, 
xnticks_t idate,
         * from the clock device backing the timer, among the dynamic
         * set of real-time CPUs currently enabled.
         */
-       cpu = xnclock_get_default_cpu(xntimer_clock(&thread->ptimer),
-                                     xnsched_cpu(thread->sched));
-       xntimer_set_sched(&thread->ptimer, xnsched_struct(cpu));
+       xntimer_set_affinity(&thread->ptimer, thread->sched);
 
        if (idate == XN_INFINITE)
                xntimer_start(&thread->ptimer, period, period, XN_RELATIVE);
diff --git a/kernel/cobalt/timer.c b/kernel/cobalt/timer.c
index 324627a..30ab31c 100644
--- a/kernel/cobalt/timer.c
+++ b/kernel/cobalt/timer.c
@@ -332,7 +332,6 @@ void __xntimer_init(struct xntimer *timer,
                    int flags)
 {
        spl_t s __maybe_unused;
-       int cpu;
 
 #ifdef CONFIG_XENO_OPT_EXTCLOCK
        timer->clock = clock;
@@ -343,19 +342,16 @@ void __xntimer_init(struct xntimer *timer,
        timer->status = (XNTIMER_DEQUEUED|(flags & XNTIMER_INIT_MASK));
        timer->handler = handler;
        timer->interval_ns = 0;
+       timer->sched = NULL;
+
        /*
-        * If the CPU the caller is affine to does not receive timer
-        * events, or no affinity was specified (i.e. sched == NULL),
-        * assign the timer to the first possible CPU which can
-        * receive interrupt events from the clock device backing this
-        * timer.
-        *
-        * If the clock device has no percpu semantics,
-        * xnclock_get_default_cpu() makes the timer always affine to
-        * CPU0 unconditionally.
+        * Set the timer affinity, preferably to xnsched_cpu(sched) if
+        * sched was given, CPU0 otherwise.
         */
-       cpu = xnclock_get_default_cpu(clock, sched ? xnsched_cpu(sched) : 0);
-       timer->sched = xnsched_struct(cpu);
+       if (sched)
+               xntimer_set_affinity(timer, sched);
+       else
+               timer->sched = xnsched_struct(0);
 
 #ifdef CONFIG_XENO_OPT_STATS
 #ifdef CONFIG_XENO_OPT_EXTCLOCK
@@ -422,21 +418,6 @@ void __xntimer_switch_tracking(struct xntimer *timer,
 
 #endif /* CONFIG_XENO_OPT_STATS */
 
-static inline void __xntimer_set_clock(struct xntimer *timer,
-                                      struct xnclock *newclock)
-{
-#ifdef CONFIG_SMP
-       int cpu;
-       /*
-        * Make sure the timer lives on a CPU the backing clock device
-        * ticks on.
-        */
-       cpu = xnclock_get_default_cpu(newclock, xnsched_cpu(timer->sched));
-       xntimer_migrate(timer, xnsched_struct(cpu));
-#endif
-       __xntimer_switch_tracking(timer, newclock);
-}
-
 /**
  * @brief Set the reference clock of a timer.
  *
@@ -452,9 +433,15 @@ static inline void __xntimer_set_clock(struct xntimer 
*timer,
 void xntimer_set_clock(struct xntimer *timer,
                       struct xnclock *newclock)
 {
-       xntimer_stop(timer);
-       timer->clock = newclock;
-       __xntimer_set_clock(timer, newclock);
+       if (timer->clock != newclock) {
+               xntimer_stop(timer);
+               timer->clock = newclock;
+               /*
+                * Since the timer was stopped, we can wait until it
+                * is restarted for fixing its CPU affinity.
+                */
+               __xntimer_switch_tracking(timer, newclock);
+       }
 }
 
 #endif /* CONFIG_XENO_OPT_EXTCLOCK */
@@ -507,13 +494,10 @@ EXPORT_SYMBOL_GPL(xntimer_destroy);
  * @coretags{unrestricted, atomic-entry}
  */
 void __xntimer_migrate(struct xntimer *timer, struct xnsched *sched)
-{                              /* nklocked, IRQs off */
+{                              /* nklocked, IRQs off, sched != timer->sched */
        struct xnclock *clock;
        xntimerq_t *q;
 
-       if (sched == timer->sched)
-               return;
-
        trace_cobalt_timer_migrate(timer, xnsched_cpu(sched));
 
        /*
@@ -543,23 +527,60 @@ void __xntimer_migrate(struct xntimer *timer, struct 
xnsched *sched)
 }
 EXPORT_SYMBOL_GPL(__xntimer_migrate);
 
-bool xntimer_set_sched(struct xntimer *timer,
-                      struct xnsched *sched)
+static inline int get_clock_cpu(struct xnclock *clock, int cpu)
 {
        /*
-        * We may deny the request if the target CPU does not receive
-        * any event from the clock device backing the timer, or the
-        * clock device has no percpu semantics.
+        * Check a CPU number against the possible set of CPUs
+        * receiving events from the underlying clock device. If the
+        * suggested CPU does not receive events from this device,
+        * return the first one which does.  We also account for the
+        * dynamic set of real-time CPUs.
+        *
+        * A clock device with no percpu semantics causes this routine
+        * to return CPU0 unconditionally.
+        *
+        * NOTE: we have scheduler slots initialized for all online
+        * CPUs, we can program and receive clock ticks on any other
+        * them. So there is no point in restricting the valid CPU set
+        * to cobalt_cpu_affinity, which specifically refers to the
+        * set of CPUs which may run real-time threads. Although
+        * receiving a clock tick for waking up a thread living on a
+        * remote CPU is not optimal since this involves IPI-signaled
+        * rescheds, this is still a valid case.
         */
-       if (cpumask_test_cpu(xnsched_cpu(sched),
-                            &xntimer_clock(timer)->affinity)) {
-               xntimer_migrate(timer, sched);
-               return true;
-       }
+       if (cpumask_empty(&clock->affinity))
+               return 0;
 
-       return false;
+       if (cpumask_test_cpu(cpu, &clock->affinity))
+               return cpu;
+       
+       return cpumask_first(&clock->affinity);
+}
+
+void __xntimer_set_affinity(struct xntimer *timer, struct xnsched *sched)
+{                              /* nklocked, IRQs off */
+       struct xnclock *clock = xntimer_clock(timer);
+       int cpu;
+
+       /*
+        * Figure out which CPU is best suited for managing this
+        * timer, preferably picking xnsched_cpu(sched) if the ticking
+        * device moving the timer clock beats on that CPU. Otherwise,
+        * default to CPU0 which is always valid for timing, since:
+        *
+        * - either the underlying clock device has per-CPU semantics
+        * and therefore does tick on CPU0 too,
+        *
+        * - or the clock device is global, in which case timers based
+        * on it are forcibly queued to CPU0.
+        */
+       cpu = 0;
+       if (!cpumask_empty(&clock->affinity))
+               cpu = get_clock_cpu(clock, xnsched_cpu(sched));
+
+       xntimer_migrate(timer, xnsched_struct(cpu));
 }
-EXPORT_SYMBOL_GPL(xntimer_set_sched);
+EXPORT_SYMBOL_GPL(__xntimer_set_affinity);
 
 int xntimer_setup_ipi(void)
 {


_______________________________________________
Xenomai-git mailing list
[email protected]
https://xenomai.org/mailman/listinfo/xenomai-git

Reply via email to