On Thu, Oct 10, 2019 at 10:11 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 10/9/19 8:39 PM, Iurii Zaikin wrote:
KUnit tests for decoding extended 64 bit timestamps.
"Added the link to the ext4 docs from which the tests were derived."
Document reference is great. I would still like to see summary in the commit log.
As you said below:
"This builds the ext4 inode sysctl unit test, which runs on boot."
Also include what should user expect to see when one of these fails.
Will do.
Signed-off-by: Iurii Zaikin yzaikin@google.com
fs/ext4/Kconfig | 12 +++ fs/ext4/Makefile | 1 + fs/ext4/inode-test.c | 221 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 fs/ext4/inode-test.c
diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig index cbb5ca830e57..cb0b52753674 100644 --- a/fs/ext4/Kconfig +++ b/fs/ext4/Kconfig @@ -106,3 +106,15 @@ config EXT4_DEBUG If you select Y here, then you will be able to turn on debugging with a command such as: echo 1 > /sys/module/ext4/parameters/mballoc_debug
+config EXT4_KUNIT_TESTS
bool "KUnit test for ext4 inode"depends on EXT4_FSdepends on KUNIThelpThis builds the ext4 inode sysctl unit test, which runs on boot.Tests the encoding correctness of ext4 inode.For more information on KUnit and unit tests in general please referto the KUnit documentation in Documentation/dev-tools/kunit/.Please add Documentation/filesystems/ext4/inodes.rst Inode Timestamps here as well. Yeah. Especially after looking at the document, summary of what these test(s) is definitely helpful. You can't expect users to read the document before enabling it. Please write a summary of tests and what they do and add it here and then in the commit log. Also include what user should expect when they pass and when one of them fails.
I'm not sure this is compatible with Theodore's preference for having a single config option for all ext4 tests. If anything, I should be removing all inode-specific stuff from the description. I think it makes sense to add wording that this option is only useful for devs running a kernel test harness and should not be enabled in production.
If unsure, say N.diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile index b17ddc229ac5..a0588fd2eea6 100644 --- a/fs/ext4/Makefile +++ b/fs/ext4/Makefile @@ -13,4 +13,5 @@ ext4-y := balloc.o bitmap.o block_validity.o dir.o ext4_jbd2.o extents.o \
ext4-$(CONFIG_EXT4_FS_POSIX_ACL) += acl.o ext4-$(CONFIG_EXT4_FS_SECURITY) += xattr_security.o +ext4-$(CONFIG_EXT4_KUNIT_TESTS) += inode-test.o ext4-$(CONFIG_FS_VERITY) += verity.o diff --git a/fs/ext4/inode-test.c b/fs/ext4/inode-test.c new file mode 100644 index 000000000000..43bc6cb547cd --- /dev/null +++ b/fs/ext4/inode-test.c @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- KUnit test of ext4 inode that verify the seconds part of [a/c/m]
- timestamps in ext4 inode structs are decoded correctly.
- These tests are derived from the table under
- Documentation/filesystems/ext4/inodes.rst Inode Timestamps
Yeah. Especially after looking at the document, summary of what these test(s) is definitely helpful. You can't expect users to read the document before enabling the tests.
- */
+#include <kunit/test.h> +#include <linux/kernel.h> +#include <linux/time64.h>
+#include "ext4.h"
+/* binary: 00000000 00000000 00000000 00000000 */ +#define LOWER_MSB_0 0L +/* binary: 01111111 11111111 11111111 11111111 */ +#define UPPER_MSB_0 0x7fffffffL +/* binary: 10000000 00000000 00000000 00000000 */ +#define LOWER_MSB_1 (-0x80000000L) +/* binary: 11111111 11111111 11111111 11111111 */ +#define UPPER_MSB_1 (-1L) +/* binary: 00111111 11111111 11111111 11111111 */ +#define MAX_NANOSECONDS ((1L << 30) - 1)
+#define CASE_NAME_FORMAT "%s: msb:%x lower_bound:%x extra_bits: %x"
+struct timestamp_expectation {
const char *test_case_name;struct timespec64 expected;u32 extra_bits;bool msb_set;bool lower_bound;+};
+static time64_t get_32bit_time(const struct timestamp_expectation * const test) +{
if (test->msb_set) {if (test->lower_bound)return LOWER_MSB_1;return UPPER_MSB_1;}if (test->lower_bound)return LOWER_MSB_0;return UPPER_MSB_0;+}
+static void inode_test_xtimestamp_decoding(struct kunit *test) +{
const struct timestamp_expectation test_data[] = {{.test_case_name = "1901-12-13",.msb_set = true,.lower_bound = true,.extra_bits = 0,.expected = {.tv_sec = -0x80000000LL, .tv_nsec = 0L},},{.test_case_name = "1969-12-31",.msb_set = true,.lower_bound = false,.extra_bits = 0,.expected = {.tv_sec = -1LL, .tv_nsec = 0L},},{.test_case_name = "1970-01-01",.msb_set = false,.lower_bound = true,.extra_bits = 0,.expected = {0LL, 0L},},{.test_case_name = "2038-01-19",.msb_set = false,.lower_bound = false,.extra_bits = 0,.expected = {.tv_sec = 0x7fffffffLL, .tv_nsec = 0L},},{.test_case_name = "2038-01-19",.msb_set = true,.lower_bound = true,.extra_bits = 1,.expected = {.tv_sec = 0x80000000LL, .tv_nsec = 0L},},{.test_case_name = "2106-02-07",.msb_set = true,.lower_bound = false,.extra_bits = 1,.expected = {.tv_sec = 0xffffffffLL, .tv_nsec = 0L},},{.test_case_name = "2106-02-07",.msb_set = false,.lower_bound = true,.extra_bits = 1,.expected = {.tv_sec = 0x100000000LL, .tv_nsec = 0L},},{.test_case_name = "2174-02-25",.msb_set = false,.lower_bound = false,.extra_bits = 1,.expected = {.tv_sec = 0x17fffffffLL, .tv_nsec = 0L},},{.test_case_name = "2174-02-25",.msb_set = true,.lower_bound = true,.extra_bits = 2,.expected = {.tv_sec = 0x180000000LL, .tv_nsec = 0L},},{.test_case_name = "2242-03-16",.msb_set = true,.lower_bound = false,.extra_bits = 2,.expected = {.tv_sec = 0x1ffffffffLL, .tv_nsec = 0L},},{.test_case_name = "2242-03-16",.msb_set = false,.lower_bound = true,.extra_bits = 2,.expected = {.tv_sec = 0x200000000LL, .tv_nsec = 0L},},{.test_case_name = " 2310-04-04",.msb_set = false,.lower_bound = false,.extra_bits = 2,.expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 0L},},{.test_case_name = " 2310-04-04 00:00:00.1",.msb_set = false,.lower_bound = false,.extra_bits = 6,.expected = {.tv_sec = 0x27fffffffLL, .tv_nsec = 1L},},{.test_case_name = "2378-04-22 00:00:00.MAX_NSEC",.msb_set = false,.lower_bound = true,.extra_bits = 0xFFFFFFFF,.expected = {.tv_sec = 0x300000000LL,.tv_nsec = MAX_NANOSECONDS},},{.test_case_name = "2378-04-22",.msb_set = false,.lower_bound = true,.extra_bits = 3,.expected = {.tv_sec = 0x300000000LL, .tv_nsec = 0L},},{.test_case_name = "2446-05-10",.msb_set = false,.lower_bound = false,.extra_bits = 3,.expected = {.tv_sec = 0x37fffffffLL, .tv_nsec = 0L},}};Is there a way to make the test data dynamic. Can you read from a data file? It will be easier to if the data
Maybe this is question to Brendan?
From the general unit test philosophy, unit tests must be 100% deterministic,
repeatable and simple enough to be correct by visual inspection, dynamically generated test data runs contrary to these goals IMHO. As for reading from a data file, not sure what exactly you mean here: - Having a running kernel read a file in the filesystem, especially as early in the initialization process as KUnit currently runs is something I'm not sure how to implement reliably. Also, doing I/O in the tests will make them slower and require more setup from test running environment. - Having reading a file in the build stage and linking it as a data blob into the kernel image. This approach looks better to me since it avoids the I/O and has no noticeable speed penalty or test harness requirements. It would be up to Brendan whether he wants such capability in KUnit and based on the user-space test code I've seen so far, the number of test data points in this test doesn't warrant reading from files even in userspace which has far fewer constraints.
struct timespec64 timestamp;int i;for (i = 0; i < ARRAY_SIZE(test_data); ++i) {timestamp.tv_sec = get_32bit_time(&test_data[i]);ext4_decode_extra_time(×tamp,cpu_to_le32(test_data[i].extra_bits));KUNIT_EXPECT_EQ_MSG(test,test_data[i].expected.tv_sec,timestamp.tv_sec,CASE_NAME_FORMAT,test_data[i].test_case_name,test_data[i].msb_set,test_data[i].lower_bound,test_data[i].extra_bits);KUNIT_EXPECT_EQ_MSG(test,test_data[i].expected.tv_nsec,timestamp.tv_nsec,CASE_NAME_FORMAT,test_data[i].test_case_name,test_data[i].msb_set,test_data[i].lower_bound,test_data[i].extra_bits);}+}
+static struct kunit_case ext4_inode_test_cases[] = {
KUNIT_CASE(inode_test_xtimestamp_decoding),{}+};
+static struct kunit_suite ext4_inode_test_suite = {
.name = "ext4_inode_test",.test_cases = ext4_inode_test_cases,+};
+kunit_test_suite(ext4_inode_test_suite);
2.23.0.700.g56cf767bdb-goog
thanks, -- Shuah