Module: xenomai-3
Branch: wip/sstep
Commit: baea05d8ea2b06391f4169b0a391ad77eb55ab9f
URL:
http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=baea05d8ea2b06391f4169b0a391ad77eb55ab9f
Author: Philippe Gerum <r...@xenomai.org>
Date: Wed Aug 2 11:07:01 2017 +0200
cobalt/timer: fix, rework ptrace detection and timer forwarding
Ptracing may cause timer overruns, as the ptraced application cannot
go waiting for the current period in a timely manner when stopped on a
breakpoint or single-stepped. A mechanism was introduced a long time
ago for hiding those overruns from the application, while ptracing is
in effect.
The current implementation dealing with this case has two major flaws:
- it crashes the system when single-stepping (observed on ARM i.MX6q),
revealing a past regression which went unnoticed so far.
- it uses a big hammer to forward (most) timers without running their
respective timeout handler while ptracing, in order to hide this
timespan from the overrun accounting code. This introduces two
issues:
* the timer forwarding code sits in the tick announcement code,
which is a very hot path, despite ptracing an application is
definitely not a common operation.
* all timers are affected / blocked during ptracing, except those
which have been specifically marked (XNTIMER_NOBLCK) at creation,
which turns out to be impractical for the common case.
The new implementation only addresses what is at stake, i.e. hiding
overrun reports due to ptracing from applications. This can be done
simply by noting when a thread should disregard overruns after an exit
from the ptraced mode (XNHICCUP), then discard the pending overruns if
this flag is detected by the code reporting them
(xntimer_get_overrun()).
---
include/cobalt/kernel/clock.h | 2 --
include/cobalt/kernel/timer.h | 13 +++++-----
include/cobalt/uapi/kernel/thread.h | 1 +
kernel/cobalt/clock.c | 37 +++++-----------------------
kernel/cobalt/posix/process.c | 46 +++++++++--------------------------
kernel/cobalt/posix/signal.c | 2 +-
kernel/cobalt/posix/syscall.c | 2 +-
kernel/cobalt/posix/timer.c | 5 ++--
kernel/cobalt/posix/timer.h | 3 ++-
kernel/cobalt/posix/timerfd.c | 3 ++-
kernel/cobalt/sched-quota.c | 6 ++---
kernel/cobalt/sched-tp.c | 2 +-
kernel/cobalt/sched.c | 2 +-
kernel/cobalt/thread.c | 2 +-
kernel/cobalt/timer.c | 19 ++++++++-------
15 files changed, 49 insertions(+), 96 deletions(-)
diff --git a/include/cobalt/kernel/clock.h b/include/cobalt/kernel/clock.h
index 3dfff38..bb56f52 100644
--- a/include/cobalt/kernel/clock.h
+++ b/include/cobalt/kernel/clock.h
@@ -94,8 +94,6 @@ extern struct xnclock nkclock;
extern unsigned long nktimerlat;
-extern unsigned int nkclock_lock;
-
int xnclock_register(struct xnclock *clock,
const cpumask_t *affinity);
diff --git a/include/cobalt/kernel/timer.h b/include/cobalt/kernel/timer.h
index 3c75354..0897d28 100644
--- a/include/cobalt/kernel/timer.h
+++ b/include/cobalt/kernel/timer.h
@@ -47,14 +47,13 @@ typedef enum xntmode {
#define XNTIMER_PERIODIC 0x00000004
#define XNTIMER_REALTIME 0x00000008
#define XNTIMER_FIRED 0x00000010
-#define XNTIMER_NOBLCK 0x00000020
-#define XNTIMER_RUNNING 0x00000040
-#define XNTIMER_KGRAVITY 0x00000080
-#define XNTIMER_UGRAVITY 0x00000100
+#define XNTIMER_RUNNING 0x00000020
+#define XNTIMER_KGRAVITY 0x00000040
+#define XNTIMER_UGRAVITY 0x00000080
#define XNTIMER_IGRAVITY 0 /* most conservative */
#define XNTIMER_GRAVITY_MASK (XNTIMER_KGRAVITY|XNTIMER_UGRAVITY)
-#define XNTIMER_INIT_MASK (XNTIMER_GRAVITY_MASK|XNTIMER_NOBLCK)
+#define XNTIMER_INIT_MASK XNTIMER_GRAVITY_MASK
/* These flags are available to the real-time interfaces */
#define XNTIMER_SPARE0 0x01000000
@@ -505,7 +504,9 @@ static inline void xntimer_dequeue(struct xntimer *timer,
/** @} */
-unsigned long long xntimer_get_overruns(struct xntimer *timer, xnticks_t now);
+unsigned long long xntimer_get_overruns(struct xntimer *timer,
+ struct xnthread *waiter,
+ xnticks_t now);
#ifdef CONFIG_SMP
diff --git a/include/cobalt/uapi/kernel/thread.h
b/include/cobalt/uapi/kernel/thread.h
index cf2dfd3..946bbfb 100644
--- a/include/cobalt/uapi/kernel/thread.h
+++ b/include/cobalt/uapi/kernel/thread.h
@@ -78,6 +78,7 @@
#define XNLBALERT 0x00000002 /**< Scheduler lock break alert (SIGDEBUG sent) */
#define XNDESCENT 0x00000004 /**< Adaptive transitioning to secondary mode */
#define XNSYSRST 0x00000008 /**< Thread awaiting syscall restart after signal
*/
+#define XNHICCUP 0x00000010 /**< Just left from ptracing */
/** @} */
diff --git a/kernel/cobalt/clock.c b/kernel/cobalt/clock.c
index 5bc46b3..3e031e4 100644
--- a/kernel/cobalt/clock.c
+++ b/kernel/cobalt/clock.c
@@ -35,8 +35,6 @@
*/
unsigned long nktimerlat;
-unsigned int nkclock_lock;
-
static unsigned long long clockfreq;
#ifdef XNARCH_HAVE_LLMULSHFT
@@ -501,17 +499,16 @@ static struct xnvfile_directory clock_vfroot;
void print_core_clock_status(struct xnclock *clock,
struct xnvfile_regular_iterator *it)
{
- const char *tm_status, *wd_status = "";
+ const char *wd_status = "off";
- tm_status = nkclock_lock > 0 ? "locked" : "on";
#ifdef CONFIG_XENO_OPT_WATCHDOG
- wd_status = "+watchdog";
+ wd_status = "on";
#endif /* CONFIG_XENO_OPT_WATCHDOG */
- xnvfile_printf(it, "%7s: timer=%s, clock=%s\n",
+ xnvfile_printf(it, "%8s: timer=%s, clock=%s\n",
"devices", ipipe_timer_name(), ipipe_clock_name());
- xnvfile_printf(it, "%7s: %s%s\n", "status", tm_status, wd_status);
- xnvfile_printf(it, "%7s: %Lu\n", "setup",
+ xnvfile_printf(it, "%8s: %s\n", "watchdog", wd_status);
+ xnvfile_printf(it, "%8s: %Lu\n", "setup",
xnclock_ticks_to_ns(&nkclock, nktimerlat));
}
@@ -790,28 +787,6 @@ void xnclock_tick(struct xnclock *clock)
continue;
}
- /* Check for a locked clock state (i.e. ptracing). */
- if (unlikely(nkclock_lock > 0)) {
- if (timer->status & XNTIMER_NOBLCK)
- goto fire;
- if (timer->status & XNTIMER_PERIODIC)
- goto advance;
- /*
- * We have no period for this blocked timer,
- * so have it tick again at a reasonably close
- * date in the future, waiting for the clock
- * to be unlocked at some point. Since clocks
- * are blocked when single-stepping into an
- * application using a debugger, it is fine to
- * wait for 250 ms for the user to continue
- * program execution.
- */
- xntimerh_date(&timer->aplink) +=
- xnclock_ns_to_ticks(xntimer_clock(timer),
- 250000000);
- goto requeue;
- }
- fire:
timer->handler(timer);
now = xnclock_read_raw(clock);
timer->status |= XNTIMER_FIRED;
@@ -828,7 +803,7 @@ void xnclock_tick(struct xnclock *clock)
timer->periodic_ticks++;
xntimer_update_date(timer);
} while (xntimerh_date(&timer->aplink) < now);
- requeue:
+
#ifdef CONFIG_SMP
/*
* If the timer was migrated over its timeout handler,
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index 152c07b..99f3de7 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -1007,28 +1007,6 @@ static inline void init_hostrt(void) { }
#endif /* !CONFIG_XENO_OPT_HOSTRT */
-/* called with nklock held */
-static void register_debugged_thread(struct xnthread *thread)
-{
- nkclock_lock++;
-
- xnthread_set_state(thread, XNSSTEP);
-}
-
-static void unregister_debugged_thread(struct xnthread *thread)
-{
- spl_t s;
-
- xnlock_get_irqsave(&nklock, s);
-
- xnthread_clear_state(thread, XNSSTEP);
-
- XENO_BUG_ON(COBALT, nkclock_lock == 0);
- nkclock_lock--;
-
- xnlock_put_irqrestore(&nklock, s);
-}
-
static void __handle_taskexit_event(struct task_struct *p)
{
struct cobalt_ppd *sys_ppd;
@@ -1044,9 +1022,6 @@ static void __handle_taskexit_event(struct task_struct *p)
XENO_BUG_ON(COBALT, thread == NULL);
trace_cobalt_shadow_unmap(thread);
- if (xnthread_test_state(thread, XNSSTEP))
- unregister_debugged_thread(thread);
-
xnthread_run_handler_stack(thread, exit_thread);
xnsched_run();
@@ -1095,6 +1070,7 @@ static int handle_schedule_event(struct task_struct
*next_task)
struct task_struct *prev_task;
struct xnthread *next;
sigset_t pending;
+ spl_t s;
signal_yield();
@@ -1104,12 +1080,9 @@ static int handle_schedule_event(struct task_struct
*next_task)
goto out;
/*
- * Check whether we need to unlock the timers, each time a
- * Linux task resumes from a stopped state, excluding tasks
- * resuming shortly for entering a stopped state asap due to
- * ptracing. To identify the latter, we need to check for
- * SIGSTOP and SIGINT in order to encompass both the NPTL and
- * LinuxThreads behaviours.
+ * Track tasks leaving the ptraced state. Check both SIGSTOP
+ * (NPTL) and SIGINT (LinuxThreads) to detect ptrace
+ * continuation.
*/
if (xnthread_test_state(next, XNSSTEP)) {
if (signal_pending(next_task)) {
@@ -1124,12 +1097,15 @@ static int handle_schedule_event(struct task_struct
*next_task)
&next_task->signal->shared_pending.signal);
if (sigismember(&pending, SIGSTOP) ||
sigismember(&pending, SIGINT))
- goto no_ptrace;
+ goto check;
}
- unregister_debugged_thread(next);
+ xnlock_get_irqsave(&nklock, s);
+ xnthread_clear_state(next, XNSSTEP);
+ xnlock_put_irqrestore(&nklock, s);
+ xnthread_set_localinfo(next, XNHICCUP);
}
-no_ptrace:
+check:
/*
* Do basic sanity checks on the incoming thread state.
* NOTE: we allow ptraced threads to run shortly in order to
@@ -1182,7 +1158,7 @@ static int handle_sigwake_event(struct task_struct *p)
if (sigismember(&pending, SIGTRAP) ||
sigismember(&pending, SIGSTOP)
|| sigismember(&pending, SIGINT))
- register_debugged_thread(thread);
+ xnthread_set_state(thread, XNSSTEP);
}
if (xnthread_test_state(thread, XNRELAX)) {
diff --git a/kernel/cobalt/posix/signal.c b/kernel/cobalt/posix/signal.c
index 416d3e4..dc23a8e 100644
--- a/kernel/cobalt/posix/signal.c
+++ b/kernel/cobalt/posix/signal.c
@@ -289,7 +289,7 @@ done:
*/
switch (sip->si_code) {
case SI_TIMER:
- overrun = cobalt_timer_deliver(sip->si_tid);
+ overrun = cobalt_timer_deliver(curr, sip->si_tid);
break;
case SI_USER:
case SI_MESGQ:
diff --git a/kernel/cobalt/posix/syscall.c b/kernel/cobalt/posix/syscall.c
index 8186a0f..7144cd5 100644
--- a/kernel/cobalt/posix/syscall.c
+++ b/kernel/cobalt/posix/syscall.c
@@ -765,7 +765,7 @@ restart:
ret_handled:
/* Update the stats and userland-visible state. */
if (thread) {
- xnthread_clear_localinfo(thread, XNDESCENT);
+ xnthread_clear_localinfo(thread, XNDESCENT|XNHICCUP);
xnstat_counter_inc(&thread->stat.xsc);
xnthread_sync_window(thread);
}
diff --git a/kernel/cobalt/posix/timer.c b/kernel/cobalt/posix/timer.c
index c6190d1..0a935f9 100644
--- a/kernel/cobalt/posix/timer.c
+++ b/kernel/cobalt/posix/timer.c
@@ -539,7 +539,7 @@ fail:
return -EINVAL;
}
-int cobalt_timer_deliver(timer_t timerid) /* nklocked, IRQs off. */
+int cobalt_timer_deliver(struct cobalt_thread *waiter, timer_t timerid) /*
nklocked, IRQs off. */
{
struct cobalt_timer *timer;
xnticks_t now;
@@ -553,7 +553,8 @@ int cobalt_timer_deliver(timer_t timerid) /* nklocked, IRQs
off. */
timer->overruns = 0;
else {
now = xnclock_read_raw(xntimer_clock(&timer->timerbase));
- timer->overruns = xntimer_get_overruns(&timer->timerbase, now);
+ timer->overruns = xntimer_get_overruns(&timer->timerbase,
+ &waiter->threadbase, now);
if ((unsigned int)timer->overruns > COBALT_DELAYMAX)
timer->overruns = COBALT_DELAYMAX;
}
diff --git a/kernel/cobalt/posix/timer.h b/kernel/cobalt/posix/timer.h
index f0b41f0..0f5402a 100644
--- a/kernel/cobalt/posix/timer.h
+++ b/kernel/cobalt/posix/timer.h
@@ -35,7 +35,8 @@ struct cobalt_timer {
struct cobalt_extref extref;
};
-int cobalt_timer_deliver(timer_t timerid);
+int cobalt_timer_deliver(struct cobalt_thread *waiter,
+ timer_t timerid);
void cobalt_timer_reclaim(struct cobalt_process *p);
diff --git a/kernel/cobalt/posix/timerfd.c b/kernel/cobalt/posix/timerfd.c
index 132aba8..3cb6a7c 100644
--- a/kernel/cobalt/posix/timerfd.c
+++ b/kernel/cobalt/posix/timerfd.c
@@ -83,7 +83,8 @@ static ssize_t timerfd_read(struct rtdm_fd *fd, void __user
*buf, size_t size)
if (xntimer_periodic_p(&tfd->timer)) {
now = xnclock_read_raw(xntimer_clock(&tfd->timer));
- ticks = 1 + xntimer_get_overruns(&tfd->timer, now);
+ ticks = 1 + xntimer_get_overruns(&tfd->timer,
+ xnthread_current(), now);
} else
ticks = 1;
diff --git a/kernel/cobalt/sched-quota.c b/kernel/cobalt/sched-quota.c
index d0c9ac8..e1f76eb 100644
--- a/kernel/cobalt/sched-quota.c
+++ b/kernel/cobalt/sched-quota.c
@@ -225,14 +225,12 @@ static void xnsched_quota_init(struct xnsched *sched)
strcpy(limiter_name, "[quota-limit]");
#endif
xntimer_init(&qs->refill_timer,
- &nkclock, quota_refill_handler, sched,
- XNTIMER_NOBLCK|XNTIMER_IGRAVITY);
+ &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_NOBLCK|XNTIMER_IGRAVITY);
+ &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-tp.c b/kernel/cobalt/sched-tp.c
index 1c70092..45951c4 100644
--- a/kernel/cobalt/sched-tp.c
+++ b/kernel/cobalt/sched-tp.c
@@ -102,7 +102,7 @@ static void xnsched_tp_init(struct xnsched *sched)
tp->gps = NULL;
INIT_LIST_HEAD(&tp->threads);
xntimer_init(&tp->tf_timer, &nkclock, tp_tick_handler,
- sched, XNTIMER_NOBLCK|XNTIMER_IGRAVITY);
+ sched, XNTIMER_IGRAVITY);
xntimer_set_sched(&tp->tf_timer, sched);
xntimer_set_name(&tp->tf_timer, timer_name);
}
diff --git a/kernel/cobalt/sched.c b/kernel/cobalt/sched.c
index bc7a7e8..7afd603 100644
--- a/kernel/cobalt/sched.c
+++ b/kernel/cobalt/sched.c
@@ -213,7 +213,7 @@ void xnsched_init(struct xnsched *sched, int cpu)
#ifdef CONFIG_XENO_OPT_WATCHDOG
xntimer_init(&sched->wdtimer, &nkclock, watchdog_handler,
- sched, XNTIMER_NOBLCK|XNTIMER_IGRAVITY);
+ sched, XNTIMER_IGRAVITY);
xntimer_set_name(&sched->wdtimer, "[watchdog]");
xntimer_set_priority(&sched->wdtimer, XNTIMER_LOPRIO);
#endif /* CONFIG_XENO_OPT_WATCHDOG */
diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c
index 067026e..7be7752 100644
--- a/kernel/cobalt/thread.c
+++ b/kernel/cobalt/thread.c
@@ -1417,7 +1417,7 @@ int xnthread_wait_period(unsigned long *overruns_r)
now = xnclock_read_raw(clock);
}
- overruns = xntimer_get_overruns(&thread->ptimer, now);
+ overruns = xntimer_get_overruns(&thread->ptimer, thread, now);
if (overruns) {
ret = -ETIMEDOUT;
trace_cobalt_thread_missed_period(thread);
diff --git a/kernel/cobalt/timer.c b/kernel/cobalt/timer.c
index aa3bc9e..324627a 100644
--- a/kernel/cobalt/timer.c
+++ b/kernel/cobalt/timer.c
@@ -305,14 +305,9 @@ EXPORT_SYMBOL_GPL(__xntimer_get_timeout);
* @a sched is bound to, otherwise it will fire either on the current
* CPU if real-time, or on the first real-time CPU.
*
- * @param flags A set of flags describing the timer. The valid flags are:
- *
- * - XNTIMER_NOBLCK, the timer won't be frozen while GDB takes over
- * control of the application.
- *
- * A set of clock gravity hints can be passed via the @a flags
- * argument, used for optimizing the built-in heuristics aimed at
- * latency reduction:
+ * @param flags A set of flags describing the timer. A set of clock
+ * gravity hints can be passed via the @a flags argument, used for
+ * optimizing the built-in heuristics aimed at latency reduction:
*
* - XNTIMER_IGRAVITY, the timer activates a leaf timer handler.
* - XNTIMER_KGRAVITY, the timer activates a kernel thread.
@@ -598,7 +593,9 @@ void xntimer_release_ipi(void)
*
* @coretags{unrestricted, atomic-entry}
*/
-unsigned long long xntimer_get_overruns(struct xntimer *timer, xnticks_t now)
+unsigned long long xntimer_get_overruns(struct xntimer *timer,
+ struct xnthread *waiter,
+ xnticks_t now)
{
xnticks_t period = timer->interval;
unsigned long long overruns = 0;
@@ -627,6 +624,10 @@ unsigned long long xntimer_get_overruns(struct xntimer
*timer, xnticks_t now)
timer->pexpect_ticks++;
+ /* Hide overruns due to the most recent ptracing session. */
+ if (xnthread_test_localinfo(waiter, XNHICCUP))
+ return 0;
+
return overruns;
}
EXPORT_SYMBOL_GPL(xntimer_get_overruns);
_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
https://xenomai.org/mailman/listinfo/xenomai-git
