[PATCH v3 0/2] Add test to validate udelay

[PATCH v3 0/2] Add test to validate udelay

[David Riley]

This change adds a module and a script that makes use of it to
validate that udelay delays for at least as long as requested
(as compared to ktime).

Changes since v1:
- allow udelay() to be 0.5% faster than requested as per feedback

Changes since v2:
- fix permissions on udelay_test.sh script
- update commit message to indicate what this test targets
- fixed checkpatch whitespace error
- rebased

David Riley (2):
  kernel: time: Add udelay_test module to validate udelay
  tools: add script to test udelay

 kernel/time/Kconfig       |   7 ++
 kernel/time/Makefile      |   1 +
 kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/time/udelay_test.sh |  66 ++++++++++++++++++
 4 files changed, 244 insertions(+)
 create mode 100644 kernel/time/udelay_test.c
 create mode 100755 tools/time/udelay_test.sh

-- 
2.0.0.526.g5318336

[PATCH v3 1/2] kernel: time: Add udelay_test module to validate udelay

[David Riley]

Create a module that allows udelay() to be executed to ensure that
it is delaying at least as long as requested (with a little bit of
error allowed).

There are some configurations which don't have reliably udelay
due to using a loop delay with cpufreq changes which should use
a counter time based delay instead.  This test aims to identify
those configurations where timing is unreliable.

Signed-off-by: David Riley <davidriley@chromium.org>
---
 kernel/time/Kconfig       |   7 ++
 kernel/time/Makefile      |   1 +
 kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 kernel/time/udelay_test.c

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index f448513..c6af048 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -202,3 +202,10 @@ config HIGH_RES_TIMERS
 
 endmenu
 endif
+
+config UDELAY_TEST
+	tristate "udelay test driver"
+	default n
+	help
+	  This builds the "udelay_test" module that helps to make sure
+	  that udelay() is working properly.
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 57a413f..dca9175 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_TICK_ONESHOT)			+= tick-oneshot.o
 obj-$(CONFIG_TICK_ONESHOT)			+= tick-sched.o
 obj-$(CONFIG_TIMER_STATS)			+= timer_stats.o
 obj-$(CONFIG_DEBUG_FS)				+= timekeeping_debug.o
+obj-$(CONFIG_UDELAY_TEST)			+= udelay_test.o
diff --git a/kernel/time/udelay_test.c b/kernel/time/udelay_test.c
new file mode 100644
index 0000000..b89115f
--- /dev/null
+++ b/kernel/time/udelay_test.c
@@ -0,0 +1,170 @@
+/*
+ * udelay() test kernel module
+ *
+ * Test is executed by writing and reading to /sys/kernel/debug/udelay_test
+ * Tests are configured by writing: USECS ITERATIONS
+ * Tests are executed by reading from the same file.
+ * Specifying usecs of 0 or negative values will run multiples tests.
+ *
+ * Copyright (C) 2014 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+
+#define DEFAULT_ITERATIONS 100
+
+#define DEBUGFS_FILENAME "udelay_test"
+
+static DEFINE_MUTEX(udelay_test_lock);
+static struct dentry *udelay_test_debugfs_file;
+static int udelay_test_usecs;
+static int udelay_test_iterations = DEFAULT_ITERATIONS;
+
+static int udelay_test_single(struct seq_file *s, int usecs, int iters)
+{
+	int min = 0, max = 0, fail_count = 0;
+	long long sum = 0;
+	long long avg;
+	int i;
+	/* Allow udelay to be up to 0.5% fast */
+	int allowed_error_ns = usecs * 5;
+
+	if (iters <= 0)
+		return 0;
+
+	for (i = 0; i < iters; ++i) {
+		struct timespec ts1, ts2;
+		int time_passed;
+
+		ktime_get_ts(&ts1);
+		udelay(usecs);
+		ktime_get_ts(&ts2);
+		time_passed = timespec_to_ns(&ts2) - timespec_to_ns(&ts1);
+
+		if (i == 0 || time_passed < min)
+			min = time_passed;
+		if (i == 0 || time_passed > max)
+			max = time_passed;
+		if ((time_passed + allowed_error_ns) / 1000 < usecs)
+			++fail_count;
+		sum += time_passed;
+	}
+
+	avg = sum;
+	do_div(avg, iters);
+	seq_printf(s, "%d usecs x %d: exp=%d allowed=%d min=%d avg=%lld max=%d",
+			usecs, iters, usecs * 1000,
+			(usecs * 1000) - allowed_error_ns, min, avg, max);
+	if (fail_count)
+		seq_printf(s, " FAIL=%d", fail_count);
+	seq_puts(s, "\n");
+
+	return 0;
+}
+
+static int udelay_test_show(struct seq_file *s, void *v)
+{
+	int usecs;
+	int iters;
+	int ret = 0;
+
+	mutex_lock(&udelay_test_lock);
+	usecs = udelay_test_usecs;
+	iters = udelay_test_iterations;
+	mutex_unlock(&udelay_test_lock);
+
+	if (usecs > 0) {
+		return udelay_test_single(s, usecs, iters);
+	} else if (usecs == 0) {
+		struct timespec ts;
+
+		ktime_get_ts(&ts);
+		seq_printf(s, "udelay() test (lpj=%ld kt=%ld.%09ld)\n",
+				loops_per_jiffy, ts.tv_sec, ts.tv_nsec);
+		seq_puts(s, "usage:\n");
+		seq_puts(s, "echo USECS [ITERS] > " DEBUGFS_FILENAME "\n");
+		seq_puts(s, "cat " DEBUGFS_FILENAME "\n");
+	}
+
+	return ret;
+}
+
+static int udelay_test_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, udelay_test_show, inode->i_private);
+}
+
+static ssize_t udelay_test_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *pos)
+{
+	char lbuf[32];
+	int ret;
+	int usecs;
+	int iters;
+
+	if (count >= sizeof(lbuf))
+		return -EINVAL;
+
+	if (copy_from_user(lbuf, buf, count))
+		return -EFAULT;
+	lbuf[count] = '\0';
+
+	ret = sscanf(lbuf, "%d %d", &usecs, &iters);
+	if (ret < 1)
+		return -EINVAL;
+	else if (ret < 2)
+		iters = DEFAULT_ITERATIONS;
+
+	mutex_lock(&udelay_test_lock);
+	udelay_test_usecs = usecs;
+	udelay_test_iterations = iters;
+	mutex_unlock(&udelay_test_lock);
+
+	return count;
+}
+
+static const struct file_operations udelay_test_debugfs_ops = {
+	.owner = THIS_MODULE,
+	.open = udelay_test_open,
+	.read = seq_read,
+	.write = udelay_test_write,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int __init udelay_test_init(void)
+{
+	mutex_lock(&udelay_test_lock);
+	udelay_test_debugfs_file = debugfs_create_file(DEBUGFS_FILENAME,
+			S_IRUSR, NULL, NULL, &udelay_test_debugfs_ops);
+	mutex_unlock(&udelay_test_lock);
+
+	return 0;
+}
+
+module_init(udelay_test_init);
+
+static void __exit udelay_test_exit(void)
+{
+	mutex_lock(&udelay_test_lock);
+	debugfs_remove(udelay_test_debugfs_file);
+	mutex_unlock(&udelay_test_lock);
+}
+
+module_exit(udelay_test_exit);
+
+MODULE_AUTHOR("David Riley <davidriley@chromium.org>");
+MODULE_LICENSE("GPL");
-- 
2.0.0.526.g5318336

Re: [PATCH v3 1/2] kernel: time: Add udelay_test module to validate udelay

[Doug Anderson]

David,

On Thu, Jun 12, 2014 at 1:13 PM, David Riley <davidriley@chromium.org> wrote:
> Create a module that allows udelay() to be executed to ensure that
> it is delaying at least as long as requested (with a little bit of
> error allowed).
>
> There are some configurations which don't have reliably udelay
> due to using a loop delay with cpufreq changes which should use
> a counter time based delay instead.  This test aims to identify
> those configurations where timing is unreliable.
>
> Signed-off-by: David Riley <davidriley@chromium.org>
> ---
>  kernel/time/Kconfig       |   7 ++
>  kernel/time/Makefile      |   1 +
>  kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+)

Looks good to me.  Also properly demonstrates that utter brokenness
that is udelay on exynos5420 and exynos5800 right now upstream, like:

171 usecs x 100: exp=171000 allowed=170145 min=57791 avg=61586
max=435750 FAIL=99

...the same kernel on exynos5250 shows all passes (though I didn't
stress the low speeds where there are known problems due to the
intermediate freq transition).

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH v3 1/2] kernel: time: Add udelay_test module to validate udelay

[John Stultz]

On Thu, Jun 12, 2014 at 1:13 PM, David Riley <davidriley@chromium.org> wrote:
> Create a module that allows udelay() to be executed to ensure that
> it is delaying at least as long as requested (with a little bit of
> error allowed).
>
> There are some configurations which don't have reliably udelay
> due to using a loop delay with cpufreq changes which should use
> a counter time based delay instead.  This test aims to identify
> those configurations where timing is unreliable.
>
> Signed-off-by: David Riley <davidriley@chromium.org>
> ---
>  kernel/time/Kconfig       |   7 ++
>  kernel/time/Makefile      |   1 +
>  kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+)
>  create mode 100644 kernel/time/udelay_test.c
>
> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
> index f448513..c6af048 100644
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -202,3 +202,10 @@ config HIGH_RES_TIMERS
>
>  endmenu
>  endif
> +
> +config UDELAY_TEST
> +       tristate "udelay test driver"
> +       default n
> +       help
> +         This builds the "udelay_test" module that helps to make sure
> +         that udelay() is working properly.


Thanks for resubmitting! So I've queued this for my testing.

My only thoughts playing with it now, is that the Kconfig entry is
just in an awkward spot. There isn't really a udelay, or really
general timekeeping specific area in the menus, so it just shows up in
"General Setup" between the "Timer Subsystem" and "Cpu/task time.."
submenus.

I suspect this would be better added in lib/Kconfig.debug near
TEST_MODULE.  Any objections to me changing that?

Also I'd probably rename the config option to TEST_UDELAY, as well as
tweak the option string to be more consistent with those similar test
driver options.

thanks
-john

Re: [PATCH v3 1/2] kernel: time: Add udelay_test module to validate udelay

[David Riley]

On Fri, Jun 13, 2014 at 9:06 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Jun 12, 2014 at 1:13 PM, David Riley <davidriley@chromium.org> wrote:
>> Create a module that allows udelay() to be executed to ensure that
>> it is delaying at least as long as requested (with a little bit of
>> error allowed).
>>
>> There are some configurations which don't have reliably udelay
>> due to using a loop delay with cpufreq changes which should use
>> a counter time based delay instead.  This test aims to identify
>> those configurations where timing is unreliable.
>>
>> Signed-off-by: David Riley <davidriley@chromium.org>
>> ---
>>  kernel/time/Kconfig       |   7 ++
>>  kernel/time/Makefile      |   1 +
>>  kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 178 insertions(+)
>>  create mode 100644 kernel/time/udelay_test.c
>>
>> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
>> index f448513..c6af048 100644
>> --- a/kernel/time/Kconfig
>> +++ b/kernel/time/Kconfig
>> @@ -202,3 +202,10 @@ config HIGH_RES_TIMERS
>>
>>  endmenu
>>  endif
>> +
>> +config UDELAY_TEST
>> +       tristate "udelay test driver"
>> +       default n
>> +       help
>> +         This builds the "udelay_test" module that helps to make sure
>> +         that udelay() is working properly.
>
>
> Thanks for resubmitting! So I've queued this for my testing.
>
> My only thoughts playing with it now, is that the Kconfig entry is
> just in an awkward spot. There isn't really a udelay, or really
> general timekeeping specific area in the menus, so it just shows up in
> "General Setup" between the "Timer Subsystem" and "Cpu/task time.."
> submenus.
>
> I suspect this would be better added in lib/Kconfig.debug near
> TEST_MODULE.  Any objections to me changing that?
>
> Also I'd probably rename the config option to TEST_UDELAY, as well as
> tweak the option string to be more consistent with those similar test
> driver options.

Hi John,

I'm okay with those changes.  Do you want me to resubmit or will you
just make the changes locally?

Thanks
Dave

>
> thanks
> -john

Re: [PATCH v3 1/2] kernel: time: Add udelay_test module to validate udelay

[John Stultz]

On Fri, Jun 13, 2014 at 9:13 AM, David Riley <davidriley@google.com> wrote:
> On Fri, Jun 13, 2014 at 9:06 AM, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Jun 12, 2014 at 1:13 PM, David Riley <davidriley@chromium.org> wrote:
>>> Create a module that allows udelay() to be executed to ensure that
>>> it is delaying at least as long as requested (with a little bit of
>>> error allowed).
>>>
>>> There are some configurations which don't have reliably udelay
>>> due to using a loop delay with cpufreq changes which should use
>>> a counter time based delay instead.  This test aims to identify
>>> those configurations where timing is unreliable.
>>>
>>> Signed-off-by: David Riley <davidriley@chromium.org>
>>> ---
>>>  kernel/time/Kconfig       |   7 ++
>>>  kernel/time/Makefile      |   1 +
>>>  kernel/time/udelay_test.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 178 insertions(+)
>>>  create mode 100644 kernel/time/udelay_test.c
>>>
>>> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
>>> index f448513..c6af048 100644
>>> --- a/kernel/time/Kconfig
>>> +++ b/kernel/time/Kconfig
>>> @@ -202,3 +202,10 @@ config HIGH_RES_TIMERS
>>>
>>>  endmenu
>>>  endif
>>> +
>>> +config UDELAY_TEST
>>> +       tristate "udelay test driver"
>>> +       default n
>>> +       help
>>> +         This builds the "udelay_test" module that helps to make sure
>>> +         that udelay() is working properly.
>>
>>
>> Thanks for resubmitting! So I've queued this for my testing.
>>
>> My only thoughts playing with it now, is that the Kconfig entry is
>> just in an awkward spot. There isn't really a udelay, or really
>> general timekeeping specific area in the menus, so it just shows up in
>> "General Setup" between the "Timer Subsystem" and "Cpu/task time.."
>> submenus.
>>
>> I suspect this would be better added in lib/Kconfig.debug near
>> TEST_MODULE.  Any objections to me changing that?
>>
>> Also I'd probably rename the config option to TEST_UDELAY, as well as
>> tweak the option string to be more consistent with those similar test
>> driver options.
>
> Hi John,
>
> I'm okay with those changes.  Do you want me to resubmit or will you
> just make the changes locally?

Don't bother, already made them locally..

thanks
-john

[PATCH v3 2/2] tools: add script to test udelay

[David Riley]

This script makes use of the udelay_test module to exercise udelay()
and ensure that it is delaying long enough (as compared to ktime).

Signed-off-by: David Riley <davidriley@chromium.org>
---
 tools/time/udelay_test.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100755 tools/time/udelay_test.sh

diff --git a/tools/time/udelay_test.sh b/tools/time/udelay_test.sh
new file mode 100755
index 0000000..12d46b9
--- /dev/null
+++ b/tools/time/udelay_test.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+
+# udelay() test script
+#
+# Test is executed by writing and reading to /sys/kernel/debug/udelay_test
+# and exercises a variety of delays to ensure that udelay() is delaying
+# at least as long as requested (as compared to ktime).
+#
+# Copyright (C) 2014 Google, Inc.
+#
+# This software is licensed under the terms of the GNU General Public
+# License version 2, as published by the Free Software Foundation, and
+# may be copied, distributed, and modified under those terms.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+MODULE_NAME=udelay_test
+UDELAY_PATH=/sys/kernel/debug/udelay_test
+
+setup()
+{
+	/sbin/modprobe -q $MODULE_NAME
+	tmp_file=`mktemp`
+}
+
+test_one()
+{
+	delay=$1
+	echo $delay > $UDELAY_PATH
+	tee -a $tmp_file < $UDELAY_PATH
+}
+
+cleanup()
+{
+	if [ -f $tmp_file ]; then
+		rm $tmp_file
+	fi
+	/sbin/modprobe -q -r $MODULE_NAME
+}
+
+trap cleanup EXIT
+setup
+
+# Delay for a variety of times.
+# 1..200, 200..500 (by 10), 500..2000 (by 100)
+for (( delay = 1; delay < 200; delay += 1 )); do
+	test_one $delay
+done
+for (( delay = 200; delay < 500; delay += 10 )); do
+	test_one $delay
+done
+for (( delay = 500; delay <= 2000; delay += 100 )); do
+	test_one $delay
+done
+
+# Search for failures
+count=`grep -c FAIL $tmp_file`
+if [ $? -eq "0" ]; then
+	echo "ERROR: $count delays failed to delay long enough"
+	retcode=1
+fi
+
+exit $retcode
-- 
2.0.0.526.g5318336

Re: [PATCH v3 2/2] tools: add script to test udelay

[Doug Anderson]

David,

On Thu, Jun 12, 2014 at 1:13 PM, David Riley <davidriley@chromium.org> wrote:
> This script makes use of the udelay_test module to exercise udelay()
> and ensure that it is delaying long enough (as compared to ktime).
>
> Signed-off-by: David Riley <davidriley@chromium.org>
> ---
>  tools/time/udelay_test.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>