Message ID | 20131107021749.15120.57874.stgit@viggo.jf.intel.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 872f05c6e9a37e9358fd58eb54deee7337863496 |
Delegated to: | Dan Williams |
Headers | show |
On Wed, 2013-11-06 at 18:17 -0800, Dan Williams wrote: > For long running tests the tracking results in a memory leak for the "ok" > results, and for the failures the kernel log should be sufficient. Provide a > uniform format for error messages so they can be easily parsed and remove the > debugfs file. > It seems for me as another big revert. Could you do this as a revert then? > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > Documentation/dmatest.txt | 27 ++--- > drivers/dma/dmatest.c | 240 ++++++--------------------------------------- > 2 files changed, 46 insertions(+), 221 deletions(-) > > diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt > index 8b7a5c879df9..45b8c95f1a21 100644 > --- a/Documentation/dmatest.txt > +++ b/Documentation/dmatest.txt > @@ -16,9 +16,8 @@ be built as module or inside kernel. Let's consider those cases. > Part 2 - When dmatest is built as a module... > > After mounting debugfs and loading the module, the /sys/kernel/debug/dmatest > -folder with nodes will be created. There are two important files located. First > -is the 'run' node that controls run and stop phases of the test, and the second > -one, 'results', is used to get the test case results. > +folder with a file named 'run' nodes will be created. 'run' controls run and > +stop phases of the test. > > Note that in this case test will not run on load automatically. > > @@ -32,8 +31,9 @@ Hint: available channel list could be extracted by running the following > command: > % ls -1 /sys/class/dma/ > > -After a while you will start to get messages about current status or error like > -in the original code. > +Once started a message like "dmatest: Started 1 threads using dma0chan0" is > +emitted. After that only test failure messages are reported until the test > +stops. > > Note that running a new test will not stop any in progress test. > > @@ -62,19 +62,18 @@ case. You always could check them at run-time by running > > Part 4 - Gathering the test results > > -The module provides a storage for the test results in the memory. The gathered > -data could be used after test is done. > +Test results are printed to the kernel log buffer with the format: > > -The special file 'results' in the debugfs represents gathered data of the in > -progress test. The messages collected are printed to the kernel log as well. > +"dmatest: result <channel>: <test id>: '<error msg>' with src_off=<val> dst_off=<val> len=<val> (<err code>)" > > Example of output: > - % cat /sys/kernel/debug/dmatest/results > - dma0chan0-copy0: #1: No errors with src_off=0x7bf dst_off=0x8ad len=0x3fea (0) > + % dmesg | tail -n 1 > + dmatest: result dma0chan0-copy0: #1: No errors with src_off=0x7bf dst_off=0x8ad len=0x3fea (0) > > The message format is unified across the different types of errors. A number in > the parens represents additional information, e.g. error code, error counter, > -or status. > +or status. A test thread also emits a summary line at completion listing the > +number of tests executed, number that failed, and a result code. > > -Note that the buffer comparison is done in the old way, i.e. data is not > -collected and just printed out. > +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 dcb38d86550e..58b195f9d03c 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -8,6 +8,8 @@ > * it under the terms of the GNU General Public License version 2 as > * published by the Free Software Foundation. > */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > #include <linux/delay.h> > #include <linux/dma-mapping.h> > #include <linux/dmaengine.h> > @@ -88,39 +90,6 @@ MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), " > #define PATTERN_OVERWRITE 0x20 > #define PATTERN_COUNT_MASK 0x1f > > -enum dmatest_error_type { > - DMATEST_ET_OK, > - DMATEST_ET_MAP_SRC, > - DMATEST_ET_MAP_DST, > - DMATEST_ET_PREP, > - DMATEST_ET_SUBMIT, > - DMATEST_ET_TIMEOUT, > - DMATEST_ET_DMA_ERROR, > - DMATEST_ET_DMA_IN_PROGRESS, > - DMATEST_ET_VERIFY, > -}; > - > -struct dmatest_thread_result { > - struct list_head node; > - unsigned int n; > - unsigned int src_off; > - unsigned int dst_off; > - unsigned int len; > - enum dmatest_error_type type; > - union { > - unsigned long data; > - dma_cookie_t cookie; > - enum dma_status status; > - int error; > - }; > -}; > - > -struct dmatest_result { > - struct list_head node; > - char *name; > - struct list_head results; > -}; > - > struct dmatest_info; > > struct dmatest_thread { > @@ -180,10 +149,6 @@ struct dmatest_info { > > /* debugfs related stuff */ > struct dentry *root; > - > - /* Test results */ > - struct list_head results; > - struct mutex results_lock; > }; > > static struct dmatest_info test_info; > @@ -337,100 +302,19 @@ static unsigned int min_odd(unsigned int x, unsigned int y) > return val % 2 ? val : val - 1; > } > > -static char *thread_result_get(const char *name, > - struct dmatest_thread_result *tr) > +static void result(const char *err, unsigned int n, unsigned int src_off, > + unsigned int dst_off, unsigned int len, unsigned long data) > { > - static const char * const messages[] = { > - [DMATEST_ET_OK] = "No errors", > - [DMATEST_ET_MAP_SRC] = "src mapping error", > - [DMATEST_ET_MAP_DST] = "dst mapping error", > - [DMATEST_ET_PREP] = "prep error", > - [DMATEST_ET_SUBMIT] = "submit error", > - [DMATEST_ET_TIMEOUT] = "test timed out", > - [DMATEST_ET_DMA_ERROR] = > - "got completion callback (DMA_ERROR)", > - [DMATEST_ET_DMA_IN_PROGRESS] = > - "got completion callback (DMA_IN_PROGRESS)", > - [DMATEST_ET_VERIFY] = "errors", > - }; > - static char buf[512]; > - > - snprintf(buf, sizeof(buf) - 1, > - "%s: #%u: %s with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", > - name, tr->n, messages[tr->type], tr->src_off, tr->dst_off, > - tr->len, tr->data); > - > - return buf; > + pr_info("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", > + current->comm, n, err, src_off, dst_off, len, data); > } > > -static int thread_result_add(struct dmatest_info *info, > - struct dmatest_result *r, enum dmatest_error_type type, > - unsigned int n, unsigned int src_off, unsigned int dst_off, > - unsigned int len, unsigned long data) > +static void dbg_result(const char *err, unsigned int n, unsigned int src_off, > + unsigned int dst_off, unsigned int len, > + unsigned long data) > { > - struct dmatest_thread_result *tr; > - > - tr = kzalloc(sizeof(*tr), GFP_KERNEL); > - if (!tr) > - return -ENOMEM; > - > - tr->type = type; > - tr->n = n; > - tr->src_off = src_off; > - tr->dst_off = dst_off; > - tr->len = len; > - tr->data = data; > - > - mutex_lock(&info->results_lock); > - list_add_tail(&tr->node, &r->results); > - mutex_unlock(&info->results_lock); > - > - if (tr->type == DMATEST_ET_OK) > - pr_debug("%s\n", thread_result_get(r->name, tr)); > - else > - pr_warn("%s\n", thread_result_get(r->name, tr)); > - > - return 0; > -} > - > -static void result_free(struct dmatest_info *info, const char *name) > -{ > - struct dmatest_result *r, *_r; > - > - mutex_lock(&info->results_lock); > - list_for_each_entry_safe(r, _r, &info->results, node) { > - struct dmatest_thread_result *tr, *_tr; > - > - if (name && strcmp(r->name, name)) > - continue; > - > - list_for_each_entry_safe(tr, _tr, &r->results, node) { > - list_del(&tr->node); > - kfree(tr); > - } > - > - kfree(r->name); > - list_del(&r->node); > - kfree(r); > - } > - > - mutex_unlock(&info->results_lock); > -} > - > -static struct dmatest_result *result_init(struct dmatest_info *info, > - const char *name) > -{ > - struct dmatest_result *r; > - > - r = kzalloc(sizeof(*r), GFP_KERNEL); > - if (r) { > - r->name = kstrdup(name, GFP_KERNEL); > - INIT_LIST_HEAD(&r->results); > - mutex_lock(&info->results_lock); > - list_add_tail(&r->node, &info->results); > - mutex_unlock(&info->results_lock); > - } > - return r; > + pr_debug("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", > + current->comm, n, err, src_off, dst_off, len, data); > } > > /* > @@ -456,7 +340,6 @@ static int dmatest_func(void *data) > struct dmatest_params *params; > struct dma_chan *chan; > struct dma_device *dev; > - const char *thread_name; > unsigned int src_off, dst_off, len; > unsigned int error_count; > unsigned int failed_tests = 0; > @@ -469,9 +352,7 @@ static int dmatest_func(void *data) > int src_cnt; > int dst_cnt; > int i; > - struct dmatest_result *result; > > - thread_name = current->comm; > set_freezable(); > > ret = -ENOMEM; > @@ -501,10 +382,6 @@ static int dmatest_func(void *data) > } else > goto err_thread_type; > > - result = result_init(info, thread_name); > - if (!result) > - goto err_srcs; > - > thread->srcs = kcalloc(src_cnt+1, sizeof(u8 *), GFP_KERNEL); > if (!thread->srcs) > goto err_srcs; > @@ -576,10 +453,8 @@ static int dmatest_func(void *data) > ret = dma_mapping_error(dev->dev, dma_srcs[i]); > if (ret) { > unmap_src(dev->dev, dma_srcs, len, i); > - thread_result_add(info, result, > - DMATEST_ET_MAP_SRC, > - total_tests, src_off, dst_off, > - len, ret); > + result("src mapping error", total_tests, > + src_off, dst_off, len, ret); > failed_tests++; > continue; > } > @@ -594,10 +469,8 @@ static int dmatest_func(void *data) > unmap_src(dev->dev, dma_srcs, len, src_cnt); > unmap_dst(dev->dev, dma_dsts, params->buf_size, > i); > - thread_result_add(info, result, > - DMATEST_ET_MAP_DST, > - total_tests, src_off, dst_off, > - len, ret); > + result("dst mapping error", total_tests, > + src_off, dst_off, len, ret); > failed_tests++; > continue; > } > @@ -627,9 +500,8 @@ static int dmatest_func(void *data) > unmap_src(dev->dev, dma_srcs, len, src_cnt); > unmap_dst(dev->dev, dma_dsts, params->buf_size, > dst_cnt); > - thread_result_add(info, result, DMATEST_ET_PREP, > - total_tests, src_off, dst_off, > - len, 0); > + result("prep error", total_tests, src_off, > + dst_off, len, ret); > msleep(100); > failed_tests++; > continue; > @@ -641,9 +513,8 @@ static int dmatest_func(void *data) > cookie = tx->tx_submit(tx); > > if (dma_submit_error(cookie)) { > - thread_result_add(info, result, DMATEST_ET_SUBMIT, > - total_tests, src_off, dst_off, > - len, cookie); > + result("submit error", total_tests, src_off, > + dst_off, len, ret); > msleep(100); > failed_tests++; > continue; > @@ -664,17 +535,15 @@ static int dmatest_func(void *data) > * free it this time?" dancing. For now, just > * leave it dangling. > */ > - thread_result_add(info, result, DMATEST_ET_TIMEOUT, > - total_tests, src_off, dst_off, > - len, 0); > + result("test timed out", total_tests, src_off, dst_off, > + len, 0); > failed_tests++; > continue; > } else if (status != DMA_SUCCESS) { > - enum dmatest_error_type type = (status == DMA_ERROR) ? > - DMATEST_ET_DMA_ERROR : DMATEST_ET_DMA_IN_PROGRESS; > - thread_result_add(info, result, type, > - total_tests, src_off, dst_off, > - len, status); > + result(status == DMA_ERROR ? > + "completion error status" : > + "completion busy status", total_tests, src_off, > + dst_off, len, ret); > failed_tests++; > continue; > } > @@ -685,7 +554,7 @@ static int dmatest_func(void *data) > > error_count = 0; > > - pr_debug("%s: verifying source buffer...\n", thread_name); > + pr_debug("%s: verifying source buffer...\n", current->comm); > error_count += dmatest_verify(thread->srcs, 0, src_off, > 0, PATTERN_SRC, true); > error_count += dmatest_verify(thread->srcs, src_off, > @@ -695,8 +564,7 @@ static int dmatest_func(void *data) > params->buf_size, src_off + len, > PATTERN_SRC, true); > > - pr_debug("%s: verifying dest buffer...\n", > - thread->task->comm); > + pr_debug("%s: verifying dest buffer...\n", current->comm); > error_count += dmatest_verify(thread->dsts, 0, dst_off, > 0, PATTERN_DST, false); > error_count += dmatest_verify(thread->dsts, dst_off, > @@ -707,14 +575,12 @@ static int dmatest_func(void *data) > PATTERN_DST, false); > > if (error_count) { > - thread_result_add(info, result, DMATEST_ET_VERIFY, > - total_tests, src_off, dst_off, > - len, error_count); > + result("data error", total_tests, src_off, dst_off, > + len, error_count); > failed_tests++; > } else { > - thread_result_add(info, result, DMATEST_ET_OK, > - total_tests, src_off, dst_off, > - len, 0); > + dbg_result("test passed", total_tests, src_off, dst_off, > + len, 0); > } > } > > @@ -731,8 +597,8 @@ err_srcbuf: > err_srcs: > kfree(pq_coefs); > err_thread_type: > - pr_notice("%s: terminating after %u tests, %u failures (status %d)\n", > - thread_name, total_tests, failed_tests, ret); > + pr_info("%s: terminating after %u tests, %u failures (status %d)\n", > + current->comm, total_tests, failed_tests, ret); > > /* terminate all transfers on specified channels */ > if (ret) > @@ -937,9 +803,6 @@ static int __restart_threaded_test(struct dmatest_info *info, bool run) > if (run == false) > return 0; > > - /* Clear results from previous run */ > - result_free(info, NULL); > - > /* Copy test parameters */ > params->buf_size = test_buf_size; > strlcpy(params->channel, strim(test_channel), sizeof(params->channel)); > @@ -1024,35 +887,6 @@ static const struct file_operations dtf_run_fops = { > .llseek = default_llseek, > }; > > -static int dtf_results_show(struct seq_file *sf, void *data) > -{ > - struct dmatest_info *info = sf->private; > - struct dmatest_result *result; > - struct dmatest_thread_result *tr; > - > - mutex_lock(&info->results_lock); > - list_for_each_entry(result, &info->results, node) { > - list_for_each_entry(tr, &result->results, node) > - seq_printf(sf, "%s\n", > - thread_result_get(result->name, tr)); > - } > - > - mutex_unlock(&info->results_lock); > - return 0; > -} > - > -static int dtf_results_open(struct inode *inode, struct file *file) > -{ > - return single_open(file, dtf_results_show, inode->i_private); > -} > - > -static const struct file_operations dtf_results_fops = { > - .open = dtf_results_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = single_release, > -}; > - > static int dmatest_register_dbgfs(struct dmatest_info *info) > { > struct dentry *d; > @@ -1069,10 +903,6 @@ static int dmatest_register_dbgfs(struct dmatest_info *info) > debugfs_create_file("run", S_IWUSR | S_IRUGO, info->root, info, > &dtf_run_fops); > > - /* Results of test in progress */ > - debugfs_create_file("results", S_IRUGO, info->root, info, > - &dtf_results_fops); > - > return 0; > > err_root: > @@ -1090,9 +920,6 @@ static int __init dmatest_init(void) > mutex_init(&info->lock); > INIT_LIST_HEAD(&info->channels); > > - mutex_init(&info->results_lock); > - INIT_LIST_HEAD(&info->results); > - > ret = dmatest_register_dbgfs(info); > if (ret) > return ret; > @@ -1112,7 +939,6 @@ static void __exit dmatest_exit(void) > > debugfs_remove_recursive(info->root); > stop_threaded_test(info); > - result_free(info, NULL); > } > module_exit(dmatest_exit); > >
On Tue, Nov 12, 2013 at 4:29 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, 2013-11-06 at 18:17 -0800, Dan Williams wrote: >> For long running tests the tracking results in a memory leak for the "ok" >> results, and for the failures the kernel log should be sufficient. Provide a >> uniform format for error messages so they can be easily parsed and remove the >> debugfs file. >> > > It seems for me as another big revert. > Could you do this as a revert then? > Two notes: 1/ The revert had conflicts 2/ I reuse some of the implementation like thread_result_get Pure reverts have their place, but any other concerns with the patch? -- 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:30 -0800, Dan Williams wrote: > On Tue, Nov 12, 2013 at 4:29 AM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, 2013-11-06 at 18:17 -0800, Dan Williams wrote: > >> For long running tests the tracking results in a memory leak for the "ok" > >> results, and for the failures the kernel log should be sufficient. Provide a > >> uniform format for error messages so they can be easily parsed and remove the > >> debugfs file. > >> > > > > It seems for me as another big revert. > > Could you do this as a revert then? > > > > Two notes: > > 1/ The revert had conflicts Yeah, it rather set of reverts that might remove functionality you are still using. > 2/ I reuse some of the implementation like thread_result_get > > Pure reverts have their place, but any other concerns with the patch? No so far. If you think that better to keep it as a single patch I'm okay with that.
diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt index 8b7a5c879df9..45b8c95f1a21 100644 --- a/Documentation/dmatest.txt +++ b/Documentation/dmatest.txt @@ -16,9 +16,8 @@ be built as module or inside kernel. Let's consider those cases. Part 2 - When dmatest is built as a module... After mounting debugfs and loading the module, the /sys/kernel/debug/dmatest -folder with nodes will be created. There are two important files located. First -is the 'run' node that controls run and stop phases of the test, and the second -one, 'results', is used to get the test case results. +folder with a file named 'run' nodes will be created. 'run' controls run and +stop phases of the test. Note that in this case test will not run on load automatically. @@ -32,8 +31,9 @@ Hint: available channel list could be extracted by running the following command: % ls -1 /sys/class/dma/ -After a while you will start to get messages about current status or error like -in the original code. +Once started a message like "dmatest: Started 1 threads using dma0chan0" is +emitted. After that only test failure messages are reported until the test +stops. Note that running a new test will not stop any in progress test. @@ -62,19 +62,18 @@ case. You always could check them at run-time by running Part 4 - Gathering the test results -The module provides a storage for the test results in the memory. The gathered -data could be used after test is done. +Test results are printed to the kernel log buffer with the format: -The special file 'results' in the debugfs represents gathered data of the in -progress test. The messages collected are printed to the kernel log as well. +"dmatest: result <channel>: <test id>: '<error msg>' with src_off=<val> dst_off=<val> len=<val> (<err code>)" Example of output: - % cat /sys/kernel/debug/dmatest/results - dma0chan0-copy0: #1: No errors with src_off=0x7bf dst_off=0x8ad len=0x3fea (0) + % dmesg | tail -n 1 + dmatest: result dma0chan0-copy0: #1: No errors with src_off=0x7bf dst_off=0x8ad len=0x3fea (0) The message format is unified across the different types of errors. A number in the parens represents additional information, e.g. error code, error counter, -or status. +or status. A test thread also emits a summary line at completion listing the +number of tests executed, number that failed, and a result code. -Note that the buffer comparison is done in the old way, i.e. data is not -collected and just printed out. +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 dcb38d86550e..58b195f9d03c 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -8,6 +8,8 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/dmaengine.h> @@ -88,39 +90,6 @@ MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), " #define PATTERN_OVERWRITE 0x20 #define PATTERN_COUNT_MASK 0x1f -enum dmatest_error_type { - DMATEST_ET_OK, - DMATEST_ET_MAP_SRC, - DMATEST_ET_MAP_DST, - DMATEST_ET_PREP, - DMATEST_ET_SUBMIT, - DMATEST_ET_TIMEOUT, - DMATEST_ET_DMA_ERROR, - DMATEST_ET_DMA_IN_PROGRESS, - DMATEST_ET_VERIFY, -}; - -struct dmatest_thread_result { - struct list_head node; - unsigned int n; - unsigned int src_off; - unsigned int dst_off; - unsigned int len; - enum dmatest_error_type type; - union { - unsigned long data; - dma_cookie_t cookie; - enum dma_status status; - int error; - }; -}; - -struct dmatest_result { - struct list_head node; - char *name; - struct list_head results; -}; - struct dmatest_info; struct dmatest_thread { @@ -180,10 +149,6 @@ struct dmatest_info { /* debugfs related stuff */ struct dentry *root; - - /* Test results */ - struct list_head results; - struct mutex results_lock; }; static struct dmatest_info test_info; @@ -337,100 +302,19 @@ static unsigned int min_odd(unsigned int x, unsigned int y) return val % 2 ? val : val - 1; } -static char *thread_result_get(const char *name, - struct dmatest_thread_result *tr) +static void result(const char *err, unsigned int n, unsigned int src_off, + unsigned int dst_off, unsigned int len, unsigned long data) { - static const char * const messages[] = { - [DMATEST_ET_OK] = "No errors", - [DMATEST_ET_MAP_SRC] = "src mapping error", - [DMATEST_ET_MAP_DST] = "dst mapping error", - [DMATEST_ET_PREP] = "prep error", - [DMATEST_ET_SUBMIT] = "submit error", - [DMATEST_ET_TIMEOUT] = "test timed out", - [DMATEST_ET_DMA_ERROR] = - "got completion callback (DMA_ERROR)", - [DMATEST_ET_DMA_IN_PROGRESS] = - "got completion callback (DMA_IN_PROGRESS)", - [DMATEST_ET_VERIFY] = "errors", - }; - static char buf[512]; - - snprintf(buf, sizeof(buf) - 1, - "%s: #%u: %s with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", - name, tr->n, messages[tr->type], tr->src_off, tr->dst_off, - tr->len, tr->data); - - return buf; + pr_info("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", + current->comm, n, err, src_off, dst_off, len, data); } -static int thread_result_add(struct dmatest_info *info, - struct dmatest_result *r, enum dmatest_error_type type, - unsigned int n, unsigned int src_off, unsigned int dst_off, - unsigned int len, unsigned long data) +static void dbg_result(const char *err, unsigned int n, unsigned int src_off, + unsigned int dst_off, unsigned int len, + unsigned long data) { - struct dmatest_thread_result *tr; - - tr = kzalloc(sizeof(*tr), GFP_KERNEL); - if (!tr) - return -ENOMEM; - - tr->type = type; - tr->n = n; - tr->src_off = src_off; - tr->dst_off = dst_off; - tr->len = len; - tr->data = data; - - mutex_lock(&info->results_lock); - list_add_tail(&tr->node, &r->results); - mutex_unlock(&info->results_lock); - - if (tr->type == DMATEST_ET_OK) - pr_debug("%s\n", thread_result_get(r->name, tr)); - else - pr_warn("%s\n", thread_result_get(r->name, tr)); - - return 0; -} - -static void result_free(struct dmatest_info *info, const char *name) -{ - struct dmatest_result *r, *_r; - - mutex_lock(&info->results_lock); - list_for_each_entry_safe(r, _r, &info->results, node) { - struct dmatest_thread_result *tr, *_tr; - - if (name && strcmp(r->name, name)) - continue; - - list_for_each_entry_safe(tr, _tr, &r->results, node) { - list_del(&tr->node); - kfree(tr); - } - - kfree(r->name); - list_del(&r->node); - kfree(r); - } - - mutex_unlock(&info->results_lock); -} - -static struct dmatest_result *result_init(struct dmatest_info *info, - const char *name) -{ - struct dmatest_result *r; - - r = kzalloc(sizeof(*r), GFP_KERNEL); - if (r) { - r->name = kstrdup(name, GFP_KERNEL); - INIT_LIST_HEAD(&r->results); - mutex_lock(&info->results_lock); - list_add_tail(&r->node, &info->results); - mutex_unlock(&info->results_lock); - } - return r; + pr_debug("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)", + current->comm, n, err, src_off, dst_off, len, data); } /* @@ -456,7 +340,6 @@ static int dmatest_func(void *data) struct dmatest_params *params; struct dma_chan *chan; struct dma_device *dev; - const char *thread_name; unsigned int src_off, dst_off, len; unsigned int error_count; unsigned int failed_tests = 0; @@ -469,9 +352,7 @@ static int dmatest_func(void *data) int src_cnt; int dst_cnt; int i; - struct dmatest_result *result; - thread_name = current->comm; set_freezable(); ret = -ENOMEM; @@ -501,10 +382,6 @@ static int dmatest_func(void *data) } else goto err_thread_type; - result = result_init(info, thread_name); - if (!result) - goto err_srcs; - thread->srcs = kcalloc(src_cnt+1, sizeof(u8 *), GFP_KERNEL); if (!thread->srcs) goto err_srcs; @@ -576,10 +453,8 @@ static int dmatest_func(void *data) ret = dma_mapping_error(dev->dev, dma_srcs[i]); if (ret) { unmap_src(dev->dev, dma_srcs, len, i); - thread_result_add(info, result, - DMATEST_ET_MAP_SRC, - total_tests, src_off, dst_off, - len, ret); + result("src mapping error", total_tests, + src_off, dst_off, len, ret); failed_tests++; continue; } @@ -594,10 +469,8 @@ static int dmatest_func(void *data) unmap_src(dev->dev, dma_srcs, len, src_cnt); unmap_dst(dev->dev, dma_dsts, params->buf_size, i); - thread_result_add(info, result, - DMATEST_ET_MAP_DST, - total_tests, src_off, dst_off, - len, ret); + result("dst mapping error", total_tests, + src_off, dst_off, len, ret); failed_tests++; continue; } @@ -627,9 +500,8 @@ static int dmatest_func(void *data) unmap_src(dev->dev, dma_srcs, len, src_cnt); unmap_dst(dev->dev, dma_dsts, params->buf_size, dst_cnt); - thread_result_add(info, result, DMATEST_ET_PREP, - total_tests, src_off, dst_off, - len, 0); + result("prep error", total_tests, src_off, + dst_off, len, ret); msleep(100); failed_tests++; continue; @@ -641,9 +513,8 @@ static int dmatest_func(void *data) cookie = tx->tx_submit(tx); if (dma_submit_error(cookie)) { - thread_result_add(info, result, DMATEST_ET_SUBMIT, - total_tests, src_off, dst_off, - len, cookie); + result("submit error", total_tests, src_off, + dst_off, len, ret); msleep(100); failed_tests++; continue; @@ -664,17 +535,15 @@ static int dmatest_func(void *data) * free it this time?" dancing. For now, just * leave it dangling. */ - thread_result_add(info, result, DMATEST_ET_TIMEOUT, - total_tests, src_off, dst_off, - len, 0); + result("test timed out", total_tests, src_off, dst_off, + len, 0); failed_tests++; continue; } else if (status != DMA_SUCCESS) { - enum dmatest_error_type type = (status == DMA_ERROR) ? - DMATEST_ET_DMA_ERROR : DMATEST_ET_DMA_IN_PROGRESS; - thread_result_add(info, result, type, - total_tests, src_off, dst_off, - len, status); + result(status == DMA_ERROR ? + "completion error status" : + "completion busy status", total_tests, src_off, + dst_off, len, ret); failed_tests++; continue; } @@ -685,7 +554,7 @@ static int dmatest_func(void *data) error_count = 0; - pr_debug("%s: verifying source buffer...\n", thread_name); + pr_debug("%s: verifying source buffer...\n", current->comm); error_count += dmatest_verify(thread->srcs, 0, src_off, 0, PATTERN_SRC, true); error_count += dmatest_verify(thread->srcs, src_off, @@ -695,8 +564,7 @@ static int dmatest_func(void *data) params->buf_size, src_off + len, PATTERN_SRC, true); - pr_debug("%s: verifying dest buffer...\n", - thread->task->comm); + pr_debug("%s: verifying dest buffer...\n", current->comm); error_count += dmatest_verify(thread->dsts, 0, dst_off, 0, PATTERN_DST, false); error_count += dmatest_verify(thread->dsts, dst_off, @@ -707,14 +575,12 @@ static int dmatest_func(void *data) PATTERN_DST, false); if (error_count) { - thread_result_add(info, result, DMATEST_ET_VERIFY, - total_tests, src_off, dst_off, - len, error_count); + result("data error", total_tests, src_off, dst_off, + len, error_count); failed_tests++; } else { - thread_result_add(info, result, DMATEST_ET_OK, - total_tests, src_off, dst_off, - len, 0); + dbg_result("test passed", total_tests, src_off, dst_off, + len, 0); } } @@ -731,8 +597,8 @@ err_srcbuf: err_srcs: kfree(pq_coefs); err_thread_type: - pr_notice("%s: terminating after %u tests, %u failures (status %d)\n", - thread_name, total_tests, failed_tests, ret); + pr_info("%s: terminating after %u tests, %u failures (status %d)\n", + current->comm, total_tests, failed_tests, ret); /* terminate all transfers on specified channels */ if (ret) @@ -937,9 +803,6 @@ static int __restart_threaded_test(struct dmatest_info *info, bool run) if (run == false) return 0; - /* Clear results from previous run */ - result_free(info, NULL); - /* Copy test parameters */ params->buf_size = test_buf_size; strlcpy(params->channel, strim(test_channel), sizeof(params->channel)); @@ -1024,35 +887,6 @@ static const struct file_operations dtf_run_fops = { .llseek = default_llseek, }; -static int dtf_results_show(struct seq_file *sf, void *data) -{ - struct dmatest_info *info = sf->private; - struct dmatest_result *result; - struct dmatest_thread_result *tr; - - mutex_lock(&info->results_lock); - list_for_each_entry(result, &info->results, node) { - list_for_each_entry(tr, &result->results, node) - seq_printf(sf, "%s\n", - thread_result_get(result->name, tr)); - } - - mutex_unlock(&info->results_lock); - return 0; -} - -static int dtf_results_open(struct inode *inode, struct file *file) -{ - return single_open(file, dtf_results_show, inode->i_private); -} - -static const struct file_operations dtf_results_fops = { - .open = dtf_results_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - static int dmatest_register_dbgfs(struct dmatest_info *info) { struct dentry *d; @@ -1069,10 +903,6 @@ static int dmatest_register_dbgfs(struct dmatest_info *info) debugfs_create_file("run", S_IWUSR | S_IRUGO, info->root, info, &dtf_run_fops); - /* Results of test in progress */ - debugfs_create_file("results", S_IRUGO, info->root, info, - &dtf_results_fops); - return 0; err_root: @@ -1090,9 +920,6 @@ static int __init dmatest_init(void) mutex_init(&info->lock); INIT_LIST_HEAD(&info->channels); - mutex_init(&info->results_lock); - INIT_LIST_HEAD(&info->results); - ret = dmatest_register_dbgfs(info); if (ret) return ret; @@ -1112,7 +939,6 @@ static void __exit dmatest_exit(void) debugfs_remove_recursive(info->root); stop_threaded_test(info); - result_free(info, NULL); } module_exit(dmatest_exit);
For long running tests the tracking results in a memory leak for the "ok" results, and for the failures the kernel log should be sufficient. Provide a uniform format for error messages so they can be easily parsed and remove the debugfs file. Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/dmatest.txt | 27 ++--- drivers/dma/dmatest.c | 240 ++++++--------------------------------------- 2 files changed, 46 insertions(+), 221 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