| Message ID | 20131107021829.15120.26174.stgit@viggo.jf.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Dan Williams |
| Headers | show |
On Wed, 2013-11-06 at 18:18 -0800, Dan Williams wrote: > Allows for scripting test runs by module load / unload. Prevent module > load from returning until 'iterations' (finite) tests have completed, or > cause reads of the 'wait' parameter in sysfs to pause until the tests > are done. > > Also killed the local waitqueue since we can just let the thread exit > naturally as long as we hold a reference. > > Cc: Nicolas Ferre <nicolas.ferre@atmel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Documentation/dmatest.txt | 29 ++++++++++++------ > drivers/dma/dmatest.c | 72 +++++++++++++++++++++++++++++---------------- > 2 files changed, 66 insertions(+), 35 deletions(-) > > diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt > index d89628331c2a..aa5412c254e3 100644 > --- a/Documentation/dmatest.txt > +++ b/Documentation/dmatest.txt > @@ -35,17 +35,26 @@ stops. > > Note that running a new test will not stop any in progress test. > > -The following command should return actual state of the test. > - % cat /sys/kernel/debug/dmatest/run > +The following command returns the state of the test. > + % cat /sys/module/dmatest/parameters/run > > -To wait for test done the user may perform a busy loop that checks the state. > +To wait for test completion userpace can poll 'run' until it is false, or use > +the wait parameter. Specifying 'wait=1' when loading the module causes module > +initialization to pause until a test run has completed, while reading > +/sys/module/dmatest/parameters/wait waits for any running test to complete > +before returning. For example, the following scripts wait for 42 tests > +to complete before exiting. Note that if 'iterations' is set to 'infinite' then > +waiting is disabled. > > - % while [ $(cat /sys/module/dmatest/parameters/run) = "Y" ] > - > do > - > echo -n "." > - > sleep 1 > - > done > - > echo Is the abiove still valid? We could leave this example as well. > +Example: > + #!/bin/sh > + modprobe dmatest run=1 iterations=42 wait=1 > + modprobe -r dmatest > +...or: > + #!/bin/sh > + modprobe dmatest run=1 iterations=42 > + cat /sys/module/dmatest/parameters/wait > + modprobe -r dmatest What about to keep style and show interactive commands for the user like the others? And may be we would remove modprobe -r. For example, modprobe -r is not case I'm using this module with. Another idea, shoudn't modprobe -r imply wait? > > Part 3 - When built-in in the kernel... > > @@ -75,7 +84,7 @@ number of tests executed, number that failed, and a result code. > > Example: > % dmesg | tail -n 1 > - dmatest: dma3chan0-copy0: summary 400000 tests, 0 failures iops: 61524 KB/s 246098 (0) > + dmatest: dma0chan0-copy0: summary 1 test, 0 failures 1000 iops 100000 KB/s (0) > > The details of a data miscompare error are also emitted, but do not follow the > above format. > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index 26b502069638..dd4d84d556d5 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -161,6 +161,43 @@ struct dmatest_chan { > struct list_head threads; > }; > > +static DECLARE_WAIT_QUEUE_HEAD(thread_wait); > +static bool wait; > + > +static bool is_threaded_test_run(struct dmatest_info *info) > +{ > + struct dmatest_chan *dtc; > + > + list_for_each_entry(dtc, &info->channels, node) { > + struct dmatest_thread *thread; > + > + list_for_each_entry(thread, &dtc->threads, node) { > + if (!thread->done) > + return true; > + } > + } > + > + return false; > +} > + > +static int dmatest_wait_get(char *val, const struct kernel_param *kp) > +{ > + struct dmatest_info *info = &test_info; > + struct dmatest_params *params = &info->params; > + > + if (params->iterations) > + wait_event(thread_wait, !is_threaded_test_run(info)); > + wait = true; > + return param_get_bool(val, kp); > +} > + > +static struct kernel_param_ops wait_ops = { > + .get = dmatest_wait_get, > + .set = param_set_bool, > +}; > +module_param_cb(wait, &wait_ops, &wait, S_IRUGO); > +MODULE_PARM_DESC(wait, "Wait for tests to complete (default: false)"); > + > static bool dmatest_match_channel(struct dmatest_params *params, > struct dma_chan *chan) > { > @@ -660,12 +697,7 @@ err_thread_type: > dmaengine_terminate_all(chan); > > thread->done = true; > - > - if (params->iterations > 0) > - while (!kthread_should_stop()) { > - DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wait_dmatest_exit); > - interruptible_sleep_on(&wait_dmatest_exit); > - } > + wake_up(&thread_wait); > > return ret; > } > @@ -681,6 +713,7 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc) > pr_debug("thread %s exited with status %d\n", > thread->task->comm, ret); > list_del(&thread->node); > + put_task_struct(thread->task); > kfree(thread); > } > > @@ -719,18 +752,19 @@ static int dmatest_add_threads(struct dmatest_info *info, > thread->chan = dtc->chan; > thread->type = type; > smp_wmb(); > - thread->task = kthread_run(dmatest_func, thread, "%s-%s%u", > + thread->task = kthread_create(dmatest_func, thread, "%s-%s%u", > dma_chan_name(chan), op, i); > if (IS_ERR(thread->task)) { > - pr_warn("Failed to run thread %s-%s%u\n", > + pr_warn("Failed to create thread %s-%s%u\n", > dma_chan_name(chan), op, i); > kfree(thread); > break; > } > > /* srcbuf and dstbuf are allocated by the thread itself */ > - > + get_task_struct(thread->task); > list_add_tail(&thread->node, &dtc->threads); > + wake_up_process(thread->task); > } > > return i; > @@ -863,22 +897,6 @@ static void restart_threaded_test(struct dmatest_info *info, bool run) > run_threaded_test(info); > } > > -static bool is_threaded_test_run(struct dmatest_info *info) > -{ > - struct dmatest_chan *dtc; > - > - list_for_each_entry(dtc, &info->channels, node) { > - struct dmatest_thread *thread; > - > - list_for_each_entry(thread, &dtc->threads, node) { > - if (!thread->done) > - return true; > - } > - } > - > - return false; > -} > - > static int dmatest_run_get(char *val, const struct kernel_param *kp) > { > struct dmatest_info *info = &test_info; > @@ -920,6 +938,7 @@ static int dmatest_run_set(const char *val, const struct kernel_param *kp) > static int __init dmatest_init(void) > { > struct dmatest_info *info = &test_info; > + struct dmatest_params *params = &info->params; > > if (dmatest_run) { > mutex_lock(&info->lock); > @@ -927,6 +946,9 @@ static int __init dmatest_init(void) > mutex_unlock(&info->lock); > } > > + if (params->iterations && wait) > + wait_event(thread_wait, !is_threaded_test_run(info)); > + > /* module parameters are stable, inittime tests are started, > * let userspace take over 'run' control > */ >
On Tue, Nov 12, 2013 at 4:14 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, 2013-11-06 at 18:18 -0800, Dan Williams wrote: >> Allows for scripting test runs by module load / unload. Prevent module >> load from returning until 'iterations' (finite) tests have completed, or >> cause reads of the 'wait' parameter in sysfs to pause until the tests >> are done. >> >> Also killed the local waitqueue since we can just let the thread exit >> naturally as long as we hold a reference. >> >> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> Documentation/dmatest.txt | 29 ++++++++++++------ >> drivers/dma/dmatest.c | 72 +++++++++++++++++++++++++++++---------------- >> 2 files changed, 66 insertions(+), 35 deletions(-) >> >> diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt >> index d89628331c2a..aa5412c254e3 100644 >> --- a/Documentation/dmatest.txt >> +++ b/Documentation/dmatest.txt >> @@ -35,17 +35,26 @@ stops. >> >> Note that running a new test will not stop any in progress test. >> >> -The following command should return actual state of the test. >> - % cat /sys/kernel/debug/dmatest/run >> +The following command returns the state of the test. >> + % cat /sys/module/dmatest/parameters/run >> >> -To wait for test done the user may perform a busy loop that checks the state. >> +To wait for test completion userpace can poll 'run' until it is false, or use >> +the wait parameter. Specifying 'wait=1' when loading the module causes module >> +initialization to pause until a test run has completed, while reading >> +/sys/module/dmatest/parameters/wait waits for any running test to complete >> +before returning. For example, the following scripts wait for 42 tests >> +to complete before exiting. Note that if 'iterations' is set to 'infinite' then >> +waiting is disabled. >> >> - % while [ $(cat /sys/module/dmatest/parameters/run) = "Y" ] >> - > do >> - > echo -n "." >> - > sleep 1 >> - > done >> - > echo > > Is the abiove still valid? We could leave this example as well. > It is still valid. I mention above that userspace can poll 'run'. I think it is ok to leave how to poll as an exercise for the reader now that the 'wait' parameter gives a more succinct one liner method to wait for completion. >> +Example: >> + #!/bin/sh >> + modprobe dmatest run=1 iterations=42 wait=1 >> + modprobe -r dmatest >> +...or: >> + #!/bin/sh >> + modprobe dmatest run=1 iterations=42 >> + cat /sys/module/dmatest/parameters/wait >> + modprobe -r dmatest > > > What about to keep style and show interactive commands for the user like > the others? That's the "cat /sys/module/dmatest/parameters/wait" line. Not much of a style anymore since we have: Example of usage: % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1 ...or: % modprobe dmatest % echo dma0chan0 > /sys/module/dmatest/parameters/channel % echo 2000 > /sys/module/dmatest/parameters/timeout % echo 1 > /sys/module/dmatest/parameters/iterations % echo 1 > /sys/module/dmatest/parameters/run ...or on the kernel command line: dmatest.channel=dma0chan0 dmatest.timeout=2000 dmatest.iterations=1 dmatest.run=1 ...at the top of the file. I think it's already clear that there are multiple ways to specify a test. > And may be we would remove modprobe -r. For example, modprobe -r is not > case I'm using this module with. I do because I prefer to specify all test parameters on one line, just a preference. > > Another idea, shouldn't modprobe -r imply wait? > No, it's always been the case that modprobe -r interrupts the running test, it allows for time based runs. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2013-11-12 at 15:06 -0800, Dan Williams wrote: > On Tue, Nov 12, 2013 at 4:14 AM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, 2013-11-06 at 18:18 -0800, Dan Williams wrote: > >> Allows for scripting test runs by module load / unload. Prevent module > >> load from returning until 'iterations' (finite) tests have completed, or > >> cause reads of the 'wait' parameter in sysfs to pause until the tests > >> are done. > >> > >> Also killed the local waitqueue since we can just let the thread exit > >> naturally as long as we hold a reference. [] > >> +Example: > >> + #!/bin/sh > >> + modprobe dmatest run=1 iterations=42 wait=1 > >> + modprobe -r dmatest > >> +...or: > >> + #!/bin/sh > >> + modprobe dmatest run=1 iterations=42 > >> + cat /sys/module/dmatest/parameters/wait > >> + modprobe -r dmatest > > > > > > What about to keep style and show interactive commands for the user like > > the others? > > That's the "cat /sys/module/dmatest/parameters/wait" line. > > Not much of a style anymore since we have: > > Example of usage: > % modprobe dmatest channel=dma0chan0 timeout=2000 iterations=1 run=1 > > ...or: > % modprobe dmatest > % echo dma0chan0 > /sys/module/dmatest/parameters/channel > % echo 2000 > /sys/module/dmatest/parameters/timeout > % echo 1 > /sys/module/dmatest/parameters/iterations > % echo 1 > /sys/module/dmatest/parameters/run > > ...or on the kernel command line: > > dmatest.channel=dma0chan0 dmatest.timeout=2000 > dmatest.iterations=1 dmatest.run=1 > > ...at the top of the file. I think it's already clear that there are > multiple ways to specify a test. Yes, but I'm talking simple about syntax: instead of scripts / snippets from my point of view better to provide just a set of command lines that user have to execute, like in excerpt you cited above. [] > > Another idea, shouldn't modprobe -r imply wait? > No, it's always been the case that modprobe -r interrupts the running > test, it allows for time based runs. By the way, have you tried to interrupt an ongoing test? Last time I tried to do such I got a lot of errors.
diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt index d89628331c2a..aa5412c254e3 100644 --- a/Documentation/dmatest.txt +++ b/Documentation/dmatest.txt @@ -35,17 +35,26 @@ stops. Note that running a new test will not stop any in progress test. -The following command should return actual state of the test. - % cat /sys/kernel/debug/dmatest/run +The following command returns the state of the test. + % cat /sys/module/dmatest/parameters/run -To wait for test done the user may perform a busy loop that checks the state. +To wait for test completion userpace can poll 'run' until it is false, or use +the wait parameter. Specifying 'wait=1' when loading the module causes module +initialization to pause until a test run has completed, while reading +/sys/module/dmatest/parameters/wait waits for any running test to complete +before returning. For example, the following scripts wait for 42 tests +to complete before exiting. Note that if 'iterations' is set to 'infinite' then +waiting is disabled. - % while [ $(cat /sys/module/dmatest/parameters/run) = "Y" ] - > do - > echo -n "." - > sleep 1 - > done - > echo +Example: + #!/bin/sh + modprobe dmatest run=1 iterations=42 wait=1 + modprobe -r dmatest +...or: + #!/bin/sh + modprobe dmatest run=1 iterations=42 + cat /sys/module/dmatest/parameters/wait + modprobe -r dmatest Part 3 - When built-in in the kernel... @@ -75,7 +84,7 @@ number of tests executed, number that failed, and a result code. Example: % dmesg | tail -n 1 - dmatest: dma3chan0-copy0: summary 400000 tests, 0 failures iops: 61524 KB/s 246098 (0) + dmatest: dma0chan0-copy0: summary 1 test, 0 failures 1000 iops 100000 KB/s (0) The details of a data miscompare error are also emitted, but do not follow the above format. diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c index 26b502069638..dd4d84d556d5 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -161,6 +161,43 @@ struct dmatest_chan { struct list_head threads; }; +static DECLARE_WAIT_QUEUE_HEAD(thread_wait); +static bool wait; + +static bool is_threaded_test_run(struct dmatest_info *info) +{ + struct dmatest_chan *dtc; + + list_for_each_entry(dtc, &info->channels, node) { + struct dmatest_thread *thread; + + list_for_each_entry(thread, &dtc->threads, node) { + if (!thread->done) + return true; + } + } + + return false; +} + +static int dmatest_wait_get(char *val, const struct kernel_param *kp) +{ + struct dmatest_info *info = &test_info; + struct dmatest_params *params = &info->params; + + if (params->iterations) + wait_event(thread_wait, !is_threaded_test_run(info)); + wait = true; + return param_get_bool(val, kp); +} + +static struct kernel_param_ops wait_ops = { + .get = dmatest_wait_get, + .set = param_set_bool, +}; +module_param_cb(wait, &wait_ops, &wait, S_IRUGO); +MODULE_PARM_DESC(wait, "Wait for tests to complete (default: false)"); + static bool dmatest_match_channel(struct dmatest_params *params, struct dma_chan *chan) { @@ -660,12 +697,7 @@ err_thread_type: dmaengine_terminate_all(chan); thread->done = true; - - if (params->iterations > 0) - while (!kthread_should_stop()) { - DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wait_dmatest_exit); - interruptible_sleep_on(&wait_dmatest_exit); - } + wake_up(&thread_wait); return ret; } @@ -681,6 +713,7 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc) pr_debug("thread %s exited with status %d\n", thread->task->comm, ret); list_del(&thread->node); + put_task_struct(thread->task); kfree(thread); } @@ -719,18 +752,19 @@ static int dmatest_add_threads(struct dmatest_info *info, thread->chan = dtc->chan; thread->type = type; smp_wmb(); - thread->task = kthread_run(dmatest_func, thread, "%s-%s%u", + thread->task = kthread_create(dmatest_func, thread, "%s-%s%u", dma_chan_name(chan), op, i); if (IS_ERR(thread->task)) { - pr_warn("Failed to run thread %s-%s%u\n", + pr_warn("Failed to create thread %s-%s%u\n", dma_chan_name(chan), op, i); kfree(thread); break; } /* srcbuf and dstbuf are allocated by the thread itself */ - + get_task_struct(thread->task); list_add_tail(&thread->node, &dtc->threads); + wake_up_process(thread->task); } return i; @@ -863,22 +897,6 @@ static void restart_threaded_test(struct dmatest_info *info, bool run) run_threaded_test(info); } -static bool is_threaded_test_run(struct dmatest_info *info) -{ - struct dmatest_chan *dtc; - - list_for_each_entry(dtc, &info->channels, node) { - struct dmatest_thread *thread; - - list_for_each_entry(thread, &dtc->threads, node) { - if (!thread->done) - return true; - } - } - - return false; -} - static int dmatest_run_get(char *val, const struct kernel_param *kp) { struct dmatest_info *info = &test_info; @@ -920,6 +938,7 @@ static int dmatest_run_set(const char *val, const struct kernel_param *kp) static int __init dmatest_init(void) { struct dmatest_info *info = &test_info; + struct dmatest_params *params = &info->params; if (dmatest_run) { mutex_lock(&info->lock); @@ -927,6 +946,9 @@ static int __init dmatest_init(void) mutex_unlock(&info->lock); } + if (params->iterations && wait) + wait_event(thread_wait, !is_threaded_test_run(info)); + /* module parameters are stable, inittime tests are started, * let userspace take over 'run' control */
Allows for scripting test runs by module load / unload. Prevent module load from returning until 'iterations' (finite) tests have completed, or cause reads of the 'wait' parameter in sysfs to pause until the tests are done. Also killed the local waitqueue since we can just let the thread exit naturally as long as we hold a reference. Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/dmatest.txt | 29 ++++++++++++------ drivers/dma/dmatest.c | 72 +++++++++++++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 35 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html