Skip to contentSkip to conversation

Add sigdebug unit test

PATCH In review Jan Kiszka 3 participants 19 messages Jan 25, 2012 – Jan 26, 2012

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 Add sigdebug unit test

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

---
 src/testsuite/unit/Makefile.am |   16 +++-
 src/testsuite/unit/sigdebug.c  |  233 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 248 insertions(+), 1 deletions(-)
 create mode 100644 src/testsuite/unit/sigdebug.c
src/testsuite/unit/Makefile.am +15 -1
index 1bf5d8d..6e8203d 100644
@@ -11,7 +11,8 @@ test_PROGRAMS = \
 	cond-torture-posix \
 	cond-torture-native \
 	check-vdso \
-	rtdm
+	rtdm \
+	sigdebug
 
 arith_SOURCES = arith.c arith-noinline.c arith-noinline.h
 
@@ -119,3 +120,16 @@ rtdm_LDADD = \
 	../../skins/native/libnative.la \
 	../../skins/common/libxenomai.la \
 	-lpthread -lrt -lm
+
+sigdebug_SOURCES = sigdebug.c
+
+sigdebug_CPPFLAGS = \
+	@XENO_USER_CFLAGS@ \
+	-I$(top_srcdir)/include
+
+sigdebug_LDFLAGS = @XENO_USER_LDFLAGS@
+
+sigdebug_LDADD = \
+	../../skins/native/libnative.la \
+	../../skins/common/libxenomai.la \
+	-lpthread -lm
src/testsuite/unit/sigdebug.c new file +233 -0

233 lines not on this page · raw message

Signature · 1 line
1.7.3.4
#1 lore raw
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test
- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)
- please make the watchdog test mandatory by default (adding a command line option to skip it for instance), the test should fail if the watchdog is not enabled, because otherwise, it will be easy to forget testing this feature. The wathdog is enabled by default with xenomai 2.6 anyway.

Signature · 1 line
					    Gilles.
author
#2 lore raw
Quoted 8 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

Quoted 4 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

It seems to have failed in that important last discipline. Need to check why.

Quoted 5 lines

- please make the watchdog test mandatory by default (adding a command line option to skip it for instance), the test should fail if the watchdog is not enabled, because otherwise, it will be easy to forget testing this feature. The wathdog is enabled by default with xenomai 2.6 anyway.

OK.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
author
#3 lore raw
Quoted 12 lines · Jan Kiszka
Quoted 8 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

Quoted 7 lines
Quoted 4 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#4 lore raw
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Signature · 1 line
					    Gilles.
author
#5 lore raw
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind? The task killing of the watchdog is thought as a last resort to keep the system analyzable, but I have a bad feeling if we want this in a regular test case.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#6 lore raw
Quoted 39 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Quoted 6 lines

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Signature · 1 line
					    Gilles.
author
#7 lore raw
Quoted 43 lines · Gilles Chanteperdrix
Quoted 42 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

Quoted 6 lines
Quoted 3 lines

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Jan

author
#8 lore raw
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#9 lore raw
Quoted 61 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

Quoted 9 lines

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

Tick-based timing is going to be the problem for determining the spinning delay, unless we expose it in the vdso on a per-skin basis, which won't be pretty.

Quoted 2 lines

Jan

Signature · 1 line
Philippe.
author
#10 lore raw
Quoted 81 lines · Philippe Gerum
Quoted 80 lines · Jan Kiszka
Quoted 59 lines · Jan Kiszka
Quoted 58 lines · Gilles Chanteperdrix
Quoted 56 lines · Jan Kiszka
Quoted 39 lines · Gilles Chanteperdrix
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Gilles Chanteperdrix
Quoted 3 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible
SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could
move unit tests under regression.

Quoted 12 lines
Quoted 11 lines
Quoted 7 lines

- we already have a regression test for the watchdog called mayday.c,
which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in
the mayday page, a nice feature)

It seems to have failed in that important last discipline. Need to check
why.

Quoted 5 lines

Because it didn't check the page content for correctness. But that's now
done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 8 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second
case, so, I agree that mayday should be removed, but whatever it tests
should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the
test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 8 lines
Quoted 3 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Quoted 6 lines
Quoted 2 lines

Thoughts?

Tick-based timing is going to be the problem for determining the spinning delay, unless we expose it in the vdso on a per-skin basis, which won't be pretty.

I see. But we should possibly add some signal-pending || amok test to that kernel loop. That would also kill my test design, but it makes otherwise some sense I guess.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
author
#11 lore raw
Quoted 10 lines · Jan Kiszka
Quoted 10 lines
Quoted 9 lines

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

Quoted 7 lines
Quoted 3 lines

Tick-based timing is going to be the problem for determining the spinning delay, unless we expose it in the vdso on a per-skin basis, which won't be pretty.

I see. But we should possibly add some signal-pending || amok test to that kernel loop. That would also kill my test design, but it makes otherwise some sense I guess.

I'm thinking of a change like this:

ksrc/skins/native/syscall.c +9 -1
index acf0375..39204b4 100644
@@ -1020,13 +1020,21 @@ static int __rt_timer_inquire(struct pt_regs *regs)
 
 static int __rt_timer_spin(struct pt_regs *regs)
 {
+	xnthread_t *thread = xnpod_current_thread();
+	struct task_struct *p = current;
+	RTIME etime;
 	RTIME ns;
 
 	if (__xn_safe_copy_from_user(&ns, (void __user *)__xn_reg_arg1(regs),
 				     sizeof(ns)))
 		return -EFAULT;
 
-	rt_timer_spin(ns);
+	etime = xnarch_get_cpu_tsc() + xnarch_ns_to_tsc(ns);
+	while ((SRTIME)(xnarch_get_cpu_tsc() - etime) < 0) {
+		if (signal_pending(p) || xnthread_amok_p(thread))
+			return -EINTR;
+		cpu_relax();
+	}
 
 	return 0;
 }

Regarding testability of the second watchdog state: We could add a syscall to sigtest_module e.g which has the current rt_timer_spin semantics.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
author
#12 lore raw
Quoted 3 lines · Jan Kiszka

Regarding testability of the second watchdog state: We could add a syscall to sigtest_module e.g which has the current rt_timer_spin semantics.

Do you think this makes sense? Or some other testing driver? Then I would go that direction.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#13 lore raw
In reply to Jan Kiszka
Quoted 61 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

Quoted 8 lines

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

You can also call in an infinite loop, a xenomais syscall which causes a switch to primary mode, but fails.

Signature · 1 line
					    Gilles.
author
#14 lore raw
Quoted 69 lines · Gilles Chanteperdrix
Quoted 68 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Quoted 4 lines
Quoted 1 line

Thoughts?

You can also call in an infinite loop, a xenomais syscall which causes a switch to primary mode, but fails.

Nope, we would be migrated to secondary on xnthread_amok_p when returning to user mode. We need a true kernel loop.

Jan.

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#15 lore raw
Quoted 70 lines · Jan Kiszka
Quoted 69 lines · Gilles Chanteperdrix
Quoted 68 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Thoughts?

You can also call in an infinite loop, a xenomais syscall which causes a switch to primary mode, but fails.

Nope, we would be migrated to secondary on xnthread_amok_p when returning to user mode. We need a true kernel loop.

But the loop will continue, and the next call to the syscall will cause the thread to re-switch to primary mode.

Signature · 1 line
					    Gilles.
author
#16 lore raw
Quoted 73 lines · Gilles Chanteperdrix
Quoted 72 lines · Jan Kiszka
Quoted 71 lines · Gilles Chanteperdrix
Quoted 69 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

You can also call in an infinite loop, a xenomais syscall which causes a switch to primary mode, but fails.

Nope, we would be migrated to secondary on xnthread_amok_p when returning to user mode. We need a true kernel loop.

But the loop will continue, and the next call to the syscall will cause the thread to re-switch to primary mode.

But the "watchdog signal pending" flag will be cleared on each migration, thus the watchdog will never enter the second stage.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#17 lore raw
Quoted 77 lines · Jan Kiszka
Quoted 76 lines · Gilles Chanteperdrix
Quoted 75 lines · Jan Kiszka
Quoted 73 lines · Gilles Chanteperdrix
Quoted 61 lines · Jan Kiszka
Quoted 48 lines · Jan Kiszka
Quoted 47 lines · Gilles Chanteperdrix
Quoted 45 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible
SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The fallback case is in fact killing the RT task as before. But I'm unsure right now: will this leave the system always in a clean state behind?

Quoted 7 lines
Quoted 2 lines

The test case being a test case and doing nothing particular, I do not see what could go wrong. And if something goes wrong, then it needs fixing.

Well, if you kill a RT task while it's running in the kernel, you risk inconsistent system states (held mutexex etc.). In this case the task is supposed to spin in user space. If that is always safe, let's implement the test.

Had a closer look: These days the two-stage killing is only useful to catch endless loops in the kernel. User space tasks can't get around being migrated on watchdog events, even when SIGDEBUG is ignored.

Quoted 8 lines

To trigger the enforced task termination without leaving any broken states behind, there is one option: rt_task_spin. Surprisingly for me, it actually spins in the kernel, thus triggers the second level if waiting long enough. I wonder, though, if that behavior shouldn't be improved, ie. the spinning loop be closed in user space - which would take away that option again.

Thoughts?

You can also call in an infinite loop, a xenomais syscall which causes a switch to primary mode, but fails.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Nope, we would be migrated to secondary on xnthread_amok_p when returning to user mode. We need a true kernel loop.

But the loop will continue, and the next call to the syscall will cause the thread to re-switch to primary mode.

But the "watchdog signal pending" flag will be cleared on each migration, thus the watchdog will never enter the second stage.

Ok, let us forget about this test of the second stage. Please add the code which dumps the beginning of the mayday page to the test you propose, we will merge this and remove mayday.c.

Signature · 1 line
					    Gilles.
#18 lore raw
In reply to Jan Kiszka
Quoted 39 lines · Jan Kiszka
Quoted 31 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka
Quoted 5 lines · Jan Kiszka
Quoted 4 lines · Gilles Chanteperdrix
Quoted 2 lines · Jan Kiszka

We had two regressions in this code recently. So test all 6 possible SIGDEBUG reasons, or 5 if the watchdog is not available.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines

Ok for this test, with a few remarks:
- this is a regression test, so should go to src/testsuite/regression(/native), and should be added to the xeno-regression-test

What are unit test for (as they are defined here)? Looks a bit inconsistent.

I put under "regression" all the tests I have which corresponded to things that failed one time or another in xenomai past. Maybe we could move unit tests under regression.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

- we already have a regression test for the watchdog called mayday.c, which tests the second watchdog action, please merge mayday.c with sigdebug.c (mayday.c also allows checking the disassembly of the code in the mayday page, a nice feature)

Quoted 6 lines
Quoted 2 lines

It seems to have failed in that important last discipline. Need to check why.

Because it didn't check the page content for correctness. But that's now done via the new watchdog test. I can keep the debug output, but the watchdog test of mayday looks obsolete to me. Am I missing something?

Quoted 6 lines

The watchdog does two things: it first sends a SIGDEBUG, then if the application is still spinning, it sends a SIGSEGV. As far as I understood, you test tests the first case, and mayday tests the second case, so, I agree that mayday should be removed, but whatever it tests should be integrated in the sigdebug test.

Quoted 2 lines

Err... SIGSEGV is not a feature, it was the bug I fixed today. :) So the test case actually specified a bug as correct behavior.

The bug was reproducible on ARM, for an other reason, hence the mistake. Patch merged.

Signature · 1 line
                                                                Gilles.