On Mon, Jul 16, 2018 at 7:55 PM, Coly Li <colyli@xxxxxxx> wrote: > This patch adds a kernel module to test the consistency of multiple crc > calculation in Linux kernel. It is enabled with CONFIG_TEST_CRC enabled. > > The test results are printed into kernel message, which look like, > > test_crc: crc64_le: PASSED (0x4e6b> + 1ff972fa8c55, expval 0x4e6b1ff972fa8c55) > test_crc: crc64_le_bch: PASSED (0x0e4f1391d7a4a62e, expval 0x0e4f1391d7a4a62e) > test_crc: crc64_le_update: FAILED (0x03d4d0d85685d9a1, expval 0x3d4d0d85685d9a1f) > > kernel 0day system has framework to check kernel message, then the above > result can be handled by 0day system. If crc calculation inconsistency > happens, it can be detected quite soon. > > lib/test_crc.c can is a testing frame work for all crc consistency > testings. For now, there are only test caes for 3 crc routines, > - crc64_le() > - crc64_le_bch() > - crc64_le_update() > +config TEST_CRC > + tristate "CRC calculation test driver" > + default n Default default is n. > + depends on CRC64 > +#include <linux/init.h> > +#include <linux/list.h> > +#include <linux/module.h> > +#include <linux/printk.h> > +#include <linux/fs.h> > +#include <linux/miscdevice.h> > +#include <linux/slab.h> > +#include <linux/uaccess.h> > +#include <linux/async.h> > +#include <linux/delay.h> > +#include <linux/vmalloc.h> > +#include <linux/crc64.h> Perhaps in order? Moreover, either init.h or module.h depending on the Kconfig (here seems module.h is a right choice). > +struct crc_test_record { > + Redundant. > + char *name; > + __le64 data[4]; > + __le64 initval; > + __le64 expval; > + int (*handler)(struct crc_test_record *rec); > +}; > + > +static int chk_and_msg(const char *name, __le64 crc, __le64 expval) > +{ > + int ret = 0; > + > + if (crc == expval) { > + pr_info("test_crc: %s: PASSED:(0x%016llx, expval 0x%016llx)", > + name, crc, expval); > + } else { > + pr_err("test_crc: %s: FAILED:(0x%016llx, expval 0x%016llx)", > + name, crc, expval); > + ret = -EINVAL; > + } > + > + return ret; Perhaps collect statistics instead how it's done in many other tests? > +} > + > +/* Add your crc test caese here */ caese ? > +static int test_crc64_le(struct crc_test_record *rec) > +{ > + __le64 crc; > + > + crc = crc64_le(rec->data, sizeof(rec->data)); > + return chk_and_msg(rec->name, crc, rec->expval); > + Redundant. > +} > + { .name = NULL, } Simple {} would work. > +static int __init test_crc_init(void) > +{ > + int i; > + int v, ret = 0; > + > + pr_info("Kernel crc consitency testing:"); > + for (i = 0; test_data[i].name; i++) { > + v = test_data[i].handler(&test_data[i]); > + if (v < 0 && ret == 0) > + ret = -EINVAL; A bit strange. Anyway, better to collect statistics and print it at the end with corresponding return code. > + } > + > + return ret; > +} > +late_initcall(test_crc_init); Why? > +static void __exit test_crc_exit(void) { } > +module_exit(test_crc_exit); > +MODULE_LICENSE("GPL"); It's not the same as in SPDX. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-bcache" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html