diff mbox series

[7/9] genirq/timings: Add selftest for circular array

Message ID 20190513102953.16424-8-daniel.lezcano@linaro.org
State Superseded
Headers show
Series genirq/timings: Fixes and selftests | expand

Commit Message

Daniel Lezcano May 13, 2019, 10:29 a.m. UTC
Due to the complexity of the code and the difficulty to debug it,
let's add some selftests to the framework in order to spot issues or
regression at boot time when the runtime testing is enabled for this
subsystem.

This tests the circular buffer at the limits and validates:
 - the encoding / decoding of the values
 - the macro to browse the irq timings circular buffer
 - the function to push data in the circular buffer

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

---
 kernel/irq/Makefile  |   3 ++
 kernel/irq/timings.c | 119 +++++++++++++++++++++++++++++++++++++++++++
 lib/Kconfig.debug    |   9 ++++
 3 files changed, 131 insertions(+)

-- 
2.17.1

Comments

Andy Shevchenko May 13, 2019, 10:50 a.m. UTC | #1
On Mon, May 13, 2019 at 12:29:51PM +0200, Daniel Lezcano wrote:
> Due to the complexity of the code and the difficulty to debug it,

> let's add some selftests to the framework in order to spot issues or

> regression at boot time when the runtime testing is enabled for this

> subsystem.

> 

> This tests the circular buffer at the limits and validates:

>  - the encoding / decoding of the values

>  - the macro to browse the irq timings circular buffer

>  - the function to push data in the circular buffer


>  kernel/irq/timings.c | 119 +++++++++++++++++++++++++++++++++++++++++++


Is it possible to have it in a separate C-file?

> +config TEST_IRQ_TIMINGS

> +	bool "IRQ timings selftest"


> +	default n


This is already default.

> +	depends on IRQ_TIMINGS

> +	help

> +	  Enable this option to test the irq timings code on boot.

> +

> +	  If unsure, say N.


-- 
With Best Regards,
Andy Shevchenko
Daniel Lezcano May 13, 2019, 11:03 a.m. UTC | #2
On 13/05/2019 12:50, Andy Shevchenko wrote:
> On Mon, May 13, 2019 at 12:29:51PM +0200, Daniel Lezcano wrote:

>> Due to the complexity of the code and the difficulty to debug it,

>> let's add some selftests to the framework in order to spot issues or

>> regression at boot time when the runtime testing is enabled for this

>> subsystem.

>>

>> This tests the circular buffer at the limits and validates:

>>  - the encoding / decoding of the values

>>  - the macro to browse the irq timings circular buffer

>>  - the function to push data in the circular buffer

> 

>>  kernel/irq/timings.c | 119 +++++++++++++++++++++++++++++++++++++++++++

> 

> Is it possible to have it in a separate C-file?


It is possible but I would like to keep the selftest in the same file in
order to keep all the structures and functions self-contained.

>> +config TEST_IRQ_TIMINGS

>> +	bool "IRQ timings selftest"

> 

>> +	default n

> 

> This is already default.


Right, thanks for the review.

>> +	depends on IRQ_TIMINGS

>> +	help

>> +	  Enable this option to test the irq timings code on boot.

>> +

>> +	  If unsure, say N.

> 



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
diff mbox series

Patch

diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile
index ff6e352e3a6c..b4f53717d143 100644
--- a/kernel/irq/Makefile
+++ b/kernel/irq/Makefile
@@ -2,6 +2,9 @@ 
 
 obj-y := irqdesc.o handle.o manage.o spurious.o resend.o chip.o dummychip.o devres.o
 obj-$(CONFIG_IRQ_TIMINGS) += timings.o
+ifeq ($(CONFIG_TEST_IRQ_TIMINGS),y)
+	CFLAGS_timings.o += -DDEBUG
+endif
 obj-$(CONFIG_GENERIC_IRQ_CHIP) += generic-chip.o
 obj-$(CONFIG_GENERIC_IRQ_PROBE) += autoprobe.o
 obj-$(CONFIG_IRQ_DOMAIN) += irqdomain.o
diff --git a/kernel/irq/timings.c b/kernel/irq/timings.c
index 02689e6168c4..dae04117796c 100644
--- a/kernel/irq/timings.c
+++ b/kernel/irq/timings.c
@@ -1,10 +1,12 @@ 
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2016, Linaro Ltd - Daniel Lezcano <daniel.lezcano@linaro.org>
+#define pr_fmt(fmt) "irq_timings: " fmt
 
 #include <linux/kernel.h>
 #include <linux/percpu.h>
 #include <linux/slab.h>
 #include <linux/static_key.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/idr.h>
 #include <linux/irq.h>
@@ -626,3 +628,120 @@  int irq_timings_alloc(int irq)
 
 	return 0;
 }
+
+#ifdef CONFIG_TEST_IRQ_TIMINGS
+static int __init irq_timings_test_irqts(struct irq_timings *irqts,
+					 unsigned count)
+{
+	int start = count >= IRQ_TIMINGS_SIZE ? count - IRQ_TIMINGS_SIZE : 0;
+	int i, irq, oirq = 0xBEEF;
+	u64 ots = 0xDEAD, ts;
+
+	/*
+	 * Fill the circular buffer by using the dedicated function.
+	 */
+	for (i = 0; i < count; i++) {
+		pr_debug("%d: index=%d, ts=%llX irq=%X\n",
+			 i, i & IRQ_TIMINGS_MASK, ots + i, oirq + i);
+
+		irq_timings_push(ots + i, oirq + i);
+	}
+
+	/*
+	 * Compute the first elements values after the index wrapped
+	 * up or not.
+	 */
+	ots += start;
+	oirq += start;
+
+	/*
+	 * Test the circular buffer count is correct.
+	 */
+	pr_debug("---> Checking timings array count (%d) is right\n", count);
+	if (WARN_ON(irqts->count != count))
+		return -EINVAL;
+
+	/*
+	 * Test the macro allowing to browse all the irqts.
+	 */
+	pr_debug("---> Checking the for_each_irqts() macro\n");
+	for_each_irqts(i, irqts) {
+
+		irq = irq_timing_decode(irqts->values[i], &ts);
+
+		pr_debug("index=%d, ts=%llX / %llX, irq=%X / %X\n",
+			 i, ts, ots, irq, oirq);
+
+		if (WARN_ON(ts != ots || irq != oirq))
+			return -EINVAL;
+
+		ots++; oirq++;
+	}
+
+	/*
+	 * The circular buffer should have be flushed when browsed
+	 * with for_each_irqts
+	 */
+	pr_debug("---> Checking timings array is empty after browsing it\n");
+	if (WARN_ON(irqts->count))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int __init irq_timings_irqts_selftest(void)
+{
+	struct irq_timings *irqts = this_cpu_ptr(&irq_timings);
+	int i, ret;
+
+	/*
+	 * Test the circular buffer with different number of
+	 * elements. The purpose is to test at the limits (empty, half
+	 * full, full, wrapped with the cursor at the boundaries,
+	 * wrapped several times, etc ...
+	 */
+	int count[] = { 0,
+			IRQ_TIMINGS_SIZE >> 1,
+			IRQ_TIMINGS_SIZE,
+			IRQ_TIMINGS_SIZE + (IRQ_TIMINGS_SIZE >> 1),
+			2 * IRQ_TIMINGS_SIZE,
+			(2 * IRQ_TIMINGS_SIZE) + 3,
+	};
+
+	for (i = 0; i < ARRAY_SIZE(count); i++) {
+
+		pr_info("---> Checking the timings with %d/%d values\n",
+			count[i], IRQ_TIMINGS_SIZE);
+
+		ret = irq_timings_test_irqts(irqts, count[i]);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int __init irq_timings_selftest(void)
+{
+	int ret;
+
+	pr_info("------------------- selftest start -----------------\n");
+
+	/*
+	 * At this point, we don't except any subsystem to use the irq
+	 * timings but us, so it should not be enabled.
+	 */
+	if (static_branch_unlikely(&irq_timing_enabled)) {
+		pr_warn("irq timings already initialized, skipping selftest\n");
+		return 0;
+	}
+
+	ret = irq_timings_irqts_selftest();
+
+	pr_info("---------- selftest end with %s -----------\n",
+		ret ? "failure" : "success");
+
+	return ret;
+}
+early_initcall(irq_timings_selftest);
+#endif
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..a05c6695f6f9 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1827,6 +1827,15 @@  config TEST_PARMAN
 
 	  If unsure, say N.
 
+config TEST_IRQ_TIMINGS
+	bool "IRQ timings selftest"
+	default n
+	depends on IRQ_TIMINGS
+	help
+	  Enable this option to test the irq timings code on boot.
+
+	  If unsure, say N.
+
 config TEST_LKM
 	tristate "Test module loading with 'hello world' module"
 	depends on m