diff mbox

[02/10] dmatest: replace stored results mechanism, with uniform messages

Message ID 20131107021749.15120.57874.stgit@viggo.jf.intel.com (mailing list archive)
State Accepted
Commit 872f05c6e9a37e9358fd58eb54deee7337863496
Delegated to: Dan Williams
Headers show

Commit Message

Dan Williams Nov. 7, 2013, 2:17 a.m. UTC
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

Comments

Andy Shevchenko Nov. 12, 2013, 12:29 p.m. UTC | #1
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);
>  
>
Dan Williams Nov. 12, 2013, 11:30 p.m. UTC | #2
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
Andy Shevchenko Nov. 13, 2013, 2:30 p.m. UTC | #3
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 mbox

Patch

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);