On Sat, May 19, 2018 at 12:09:46AM +0300, Andy Shevchenko wrote: > On Fri, May 18, 2018 at 4:59 AM, Joel Fernandes <joe...@google.com> wrote: > > From: "Joel Fernandes (Google)" <j...@joelfernandes.org> > > > > In this patch we introduce a test module for simulating a long atomic > > section in the kernel which the preemptoff or irqsoff tracers can > > detect. This module is to be used only for test purposes and is default > > disabled. > > > > Following is the expected output (only briefly shown) that can be parsed > > to verify that the tracers are working correctly. We will use this from > > the kselftests in future patches. > > > +config TEST_ATOMIC_SECTIONS > > + tristate "Simulate atomic sections for tracers to detect" > > > + default n > > n _is_ default default.
I would rather be explicit, several other TEST configs also mention it. Is it a strong desire to drop off default n? > > +/* > > SPDX? Ok, will add. > > > + */ > > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/ktime.h> > > +#include <linux/irq.h> > > +#include <linux/printk.h> > > +#include <linux/interrupt.h> > > +#include <linux/delay.h> > > +#include <linux/string.h> > > +#include <linux/kthread.h> > > Perhaps keep in order? Sure. > > + > > +static int atomic_time = 100; > > +static char atomic_mode[10] = "irq"; > > + > > +module_param_named(atomic_time, atomic_time, int, S_IRUGO); > > +module_param_string(atomic_mode, atomic_mode, 10, S_IRUGO); > > +MODULE_PARM_DESC(atomic_time, "Period in microseconds (100 uS default)"); > > +MODULE_PARM_DESC(atomic_mode, "Mode of the test such as preempt or irq > > (default irq)"); > > > + > > + > > Extra blank line. Fixed > > +static int __init atomic_sect_init(void) > > +{ > > + char task_name[50]; > > + struct task_struct *test_task; > > + > > > + sprintf(task_name, "%s dis test", atomic_mode); > > Just to be protective from dumb user. > > snprintf(); > Done. > > + > > + test_task = kthread_run((void*)atomic_sect_run, NULL, task_name); > > + if (IS_ERR(test_task)) { > > > + return -1; > > return PTR_ERR() ? Sure, will do. Thanks for the review! - Joel