[PATCH v2] dmaengine: Simple DMA memcpy test client

[PATCH v2] dmaengine: Simple DMA memcpy test client

[Haavard Skinnemoen]

This client tests DMA memcpy using various lengths and various offsets
into the source and destination buffers. It will initialize both
buffers with a know pattern and verify that the DMA engine copies the
requested region and nothing more.

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
Changes since v1:
  * Remove extra dashes around "help" in Kconfig
  * Remove "default n" from Kconfig
  * Turn TEST_BUF_SIZE into a module parameter
  * Return DMA_NAK instead of DMA_DUP
  * Print unhandled events
  * Support testing specific channels and devices
  * Move to the end of the Makefile

Big thanks to Sam Ravnborg and Shannon Nelson for feedback.

I haven't added support for testing multiple channels at once, or
for testing a single channel using multiple threads. It will be a
rather big change, but I think it might be worth it since it will make
the module much better at catching nasty race conditions in the DMA
Engine driver.

 drivers/dma/Kconfig   |    7 +
 drivers/dma/Makefile  |    1 +
 drivers/dma/dmatest.c |  304 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+), 0 deletions(-)
 create mode 100644 drivers/dma/dmatest.c

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6a7d25f..3bb9f2c 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -49,4 +49,11 @@ config NET_DMA
 	  Since this is the main user of the DMA engine, it should be enabled;
 	  say Y here.
 
+config DMATEST
+	tristate "DMA Test client"
+	depends on DMA_ENGINE
+	help
+	  Simple DMA test client. Say N unless you're debugging a
+	  DMA Device driver.
+
 endif
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index b152cd8..cecfb60 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_NET_DMA) += iovlock.o
 obj-$(CONFIG_INTEL_IOATDMA) += ioatdma.o
 ioatdma-objs := ioat.o ioat_dma.o ioat_dca.o
 obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o
+obj-$(CONFIG_DMATEST) += dmatest.o
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
new file mode 100644
index 0000000..523ee93
--- /dev/null
+++ b/drivers/dma/dmatest.c
@@ -0,0 +1,304 @@
+/*
+ * DMA Engine test module
+ *
+ * Copyright (C) 2007 Atmel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/init.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/random.h>
+#include <linux/wait.h>
+
+static unsigned int test_buf_size = 16384;
+module_param(test_buf_size, uint, S_IRUGO);
+MODULE_PARM_DESC(test_buf_size, "Size of the memcpy test buffer");
+
+static char test_channel[BUS_ID_SIZE];
+module_param_string(channel, test_channel, sizeof(test_channel), S_IRUGO);
+MODULE_PARM_DESC(channel, "Bus ID of the channel to test (default: any)");
+
+static char test_device[BUS_ID_SIZE];
+module_param_string(device, test_device, sizeof(test_device), S_IRUGO);
+MODULE_PARM_DESC(device, "Bus ID of the DMA Engine to test (default: any)");
+
+#define SRC_PATTERN		0x7c
+#define SRC_PATTERN_OUTSIDE	0X8d
+#define POISON_UNINIT		0x49
+#define POISON_OUTSIDE		0x37
+
+struct dmatest {
+	spinlock_t		lock;
+	struct dma_client	client;
+	struct task_struct	*thread;
+	struct dma_chan		*chan;
+	wait_queue_head_t	wq;
+	u8			*srcbuf;
+	u8			*dstbuf;
+};
+
+static inline struct dmatest *to_dmatest(struct dma_client *client)
+{
+	return container_of(client, struct dmatest, client);
+}
+
+static bool dmatest_match_channel(struct dma_chan *chan)
+{
+	if (test_channel[0] == '\0')
+		return true;
+	return strcmp(chan->dev.bus_id, test_channel) == 0;
+}
+
+static bool dmatest_match_device(struct dma_device *device)
+{
+	if (test_device[0] == '\0')
+		return true;
+	return strcmp(device->dev->bus_id, test_device) == 0;
+}
+
+static enum dma_state_client
+dmatest_event(struct dma_client *client, struct dma_chan *chan,
+		enum dma_state state)
+{
+	struct dmatest		*test = to_dmatest(client);
+	enum dma_state_client	ack = DMA_NAK;
+
+	spin_lock(&test->lock);
+	switch (state) {
+	case DMA_RESOURCE_AVAILABLE:
+		if (test->chan) {
+			ack = DMA_NAK;
+		} else if (dmatest_match_channel(chan)
+				&& dmatest_match_device(chan->device)) {
+			printk(KERN_INFO "dmatest: Got channel %s\n",
+				chan->dev.bus_id);
+			test->chan = chan;
+			wake_up_interruptible(&test->wq);
+			ack = DMA_ACK;
+		} else {
+			ack = DMA_DUP;
+		}
+		break;
+
+	case DMA_RESOURCE_REMOVED:
+		if (test->chan == chan) {
+			printk(KERN_INFO "dmatest: Lost channel %s\n",
+				chan->dev.bus_id);
+			test->chan = NULL;
+			ack = DMA_ACK;
+		}
+		break;
+
+	default:
+		printk(KERN_INFO "dmatest: Unhandled event %u (%s)\n",
+				state, chan->dev.bus_id);
+		break;
+	}
+	spin_unlock(&test->lock);
+
+	return ack;
+}
+
+static unsigned long dmatest_random(void)
+{
+	unsigned long buf;
+
+	get_random_bytes(&buf, sizeof(buf));
+	return buf;
+}
+
+static unsigned int dmatest_verify(u8 *buf, unsigned int start,
+		unsigned int end, u8 expected)
+{
+	unsigned int i;
+	unsigned int error_count = 0;
+
+	for (i = start; i < end; i++) {
+		if (buf[i] != expected) {
+			if (error_count < 32)
+				printk(KERN_ERR "dmatest: buf[0x%x] = %02x "
+					"(expected %02x)\n",
+					i, buf[i], expected);
+			error_count++;
+		}
+	}
+
+	if (error_count > 32)
+		printk(KERN_ERR "dmatest: %u errors suppressed\n",
+			error_count - 32);
+
+	return error_count;
+}
+
+static int dmatest_func(void *data)
+{
+	struct dmatest	*test = data;
+	struct dma_chan	*chan;
+	bool		should_stop = false;
+	unsigned int	src_off, dst_off, len;
+	unsigned int	error_count;
+	dma_cookie_t	cookie;
+	enum dma_status	status;
+
+	dma_cap_set(DMA_MEMCPY, test->client.cap_mask);
+	dma_async_client_register(&test->client);
+	dma_async_client_chan_request(&test->client);
+
+	for (;;) {
+		DEFINE_WAIT(chan_wait);
+
+		pr_debug("dmatest: Waiting for a channel...\n");
+		for (;;) {
+			spin_lock(&test->lock);
+			prepare_to_wait(&test->wq, &chan_wait,
+					TASK_UNINTERRUPTIBLE);
+			if (kthread_should_stop()) {
+				should_stop = true;
+				break;
+			}
+
+			if (test->chan) {
+				chan = test->chan;
+				dma_chan_get(chan);
+				break;
+			}
+			spin_unlock(&test->lock);
+
+			schedule();
+		}
+		finish_wait(&test->wq, &chan_wait);
+		spin_unlock(&test->lock);
+
+		if (should_stop)
+			break;
+
+		pr_debug("dmatest: Got it!\n");
+
+		len = dmatest_random() % test_buf_size;
+		src_off = dmatest_random() % (test_buf_size - len);
+		dst_off = dmatest_random() % (test_buf_size - len);
+
+		memset(test->srcbuf, SRC_PATTERN_OUTSIDE, src_off);
+		memset(test->srcbuf + src_off, SRC_PATTERN, len);
+		memset(test->srcbuf + src_off + len, SRC_PATTERN_OUTSIDE,
+				test_buf_size - (src_off + len));
+		memset(test->dstbuf, POISON_OUTSIDE, dst_off);
+		memset(test->dstbuf + dst_off, POISON_UNINIT, len);
+		memset(test->dstbuf + dst_off + len, POISON_OUTSIDE,
+				test_buf_size - (dst_off + len));
+
+		cookie = dma_async_memcpy_buf_to_buf(chan,
+				test->dstbuf + dst_off,
+				test->srcbuf + src_off,
+				len);
+		if (dma_submit_error(cookie)) {
+			printk("dmatest: submit error: %d\n", cookie);
+			dma_chan_put(chan);
+			msleep(100);
+			continue;
+		}
+		dma_async_memcpy_issue_pending(chan);
+
+		do {
+			msleep(1);
+			status = dma_async_memcpy_complete(
+					chan, cookie, NULL, NULL);
+		} while (status == DMA_IN_PROGRESS);
+
+		dma_chan_put(chan);
+
+		if (status == DMA_ERROR) {
+			printk("dmatest: error during copy\n");
+			continue;
+		}
+
+		error_count = 0;
+
+		printk(KERN_INFO "dmatest: verifying source buffer...\n");
+		error_count += dmatest_verify(test->srcbuf, 0, src_off,
+				SRC_PATTERN_OUTSIDE);
+		error_count += dmatest_verify(test->srcbuf, src_off,
+				src_off + len, SRC_PATTERN);
+		error_count += dmatest_verify(test->srcbuf, src_off + len,
+				test_buf_size, SRC_PATTERN_OUTSIDE);
+
+		printk(KERN_INFO "dmatest: verifying dest buffer...\n");
+		error_count += dmatest_verify(test->dstbuf, 0, dst_off,
+				POISON_OUTSIDE);
+		error_count += dmatest_verify(test->dstbuf, dst_off,
+				dst_off + len, SRC_PATTERN);
+		error_count += dmatest_verify(test->dstbuf, dst_off + len,
+				test_buf_size, POISON_OUTSIDE);
+
+		if (error_count)
+			printk(KERN_ERR "dmatest: %u errors with "
+				"src_off=0x%x dst_off=0x%x len=0x%x\n",
+				error_count, src_off, dst_off, len);
+		else
+			printk(KERN_INFO "dmatest: No errors with "
+				"src_off=0x%x dst_off=0x%x len=0x%x\n",
+				src_off, dst_off, len);
+	}
+
+	dma_async_client_unregister(&test->client);
+
+	return 0;
+}
+
+static struct dmatest dmatest_data = {
+	.client		= {
+		.event_callback	= dmatest_event,
+	},
+	.wq		= __WAIT_QUEUE_HEAD_INITIALIZER(dmatest_data.wq),
+};
+
+static int __init dmatest_init(void)
+{
+	struct dmatest	*test = &dmatest_data;
+	int		ret = -ENOMEM;
+
+	spin_lock_init(&test->lock);
+
+	test->srcbuf = kmalloc(test_buf_size, GFP_KERNEL);
+	if (!test->srcbuf)
+		goto err_srcbuf;
+	test->dstbuf = kmalloc(test_buf_size, GFP_KERNEL);
+	if (!test->dstbuf)
+		goto err_dstbuf;
+
+	test->thread = kthread_run(dmatest_func, test, "kdmatestd");
+	if (IS_ERR(test->thread)) {
+		ret = PTR_ERR(test->thread);
+		goto err_kthread;
+	}
+
+	return 0;
+
+err_kthread:
+	kfree(test->dstbuf);
+err_dstbuf:
+	kfree(test->srcbuf);
+err_srcbuf:
+	return ret;
+}
+module_init(dmatest_init);
+
+static void __exit dmatest_exit(void)
+{
+	int ret;
+
+	ret = kthread_stop(dmatest_data.thread);
+	printk("dmatest: Thread exited with status %d\n", ret);
+	kfree(dmatest_data.srcbuf);
+	kfree(dmatest_data.dstbuf);
+}
+module_exit(dmatest_exit);
+
+MODULE_AUTHOR("Haavard Skinnemoen <hskinnemoen@atmel.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.5.3.4

Re: [PATCH v2] dmaengine: Simple DMA memcpy test client

[Olof Johansson]

On Fri, Nov 23, 2007 at 04:34:36PM +0100, Haavard Skinnemoen wrote:
> This client tests DMA memcpy using various lengths and various offsets
> into the source and destination buffers. It will initialize both
> buffers with a know pattern and verify that the DMA engine copies the
> requested region and nothing more.
> 
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>

Hi,

First of all: Thanks for sharing this, it's quite useful! I've been
playing around with this a bit today, and I've been seeing some odd
behaviour.

It seems that once a channel is allocated to dmatest, it will never be
freed, i.e. the drivers device_free_chan_resources will never be called
on it. Looking at the dma stack, I'm not sure just where it's expected
to happen. Once the channel is allocated, the dma_chan_get/put calls all
just modify the per-cpu variables, and nothing will ever boil down to a
call to kref_put() of the refcount until the _driver_ is deregistered.
Not even deregistering the client seems to do it.

Or am I missing something here? Shannon? Dan?

I happened to catch it due to a BUG_ON() in my device_alloc_chan_resources
that checked to make sure it wasn't allocated twice without a free
inbetween. It hit on the second load of the dmatest module, since they
were never freed on unload.


-Olof

RE: [PATCH v2] dmaengine: Simple DMA memcpy test client

[Nelson, Shannon]

>From: Olof Johansson [mailto:olof@lixom.net] 
>Sent: Thursday, December 13, 2007 7:32 PM
>To: Haavard Skinnemoen
>Cc: Nelson, Shannon; Williams, Dan J; 
>linux-kernel@vger.kernel.org; Sam Ravnborg
>Subject: Re: [PATCH v2] dmaengine: Simple DMA memcpy test client
>
>On Fri, Nov 23, 2007 at 04:34:36PM +0100, Haavard Skinnemoen wrote:
>> This client tests DMA memcpy using various lengths and 
>various offsets
>> into the source and destination buffers. It will initialize both
>> buffers with a know pattern and verify that the DMA engine copies the
>> requested region and nothing more.
>> 
>> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
>
>Hi,
>
>First of all: Thanks for sharing this, it's quite useful! I've been
>playing around with this a bit today, and I've been seeing some odd
>behaviour.
>
>It seems that once a channel is allocated to dmatest, it will never be
>freed, i.e. the drivers device_free_chan_resources will never be called
>on it. Looking at the dma stack, I'm not sure just where it's expected
>to happen. Once the channel is allocated, the dma_chan_get/put 
>calls all
>just modify the per-cpu variables, and nothing will ever boil down to a
>call to kref_put() of the refcount until the _driver_ is deregistered.
>Not even deregistering the client seems to do it.
>
>Or am I missing something here? Shannon? Dan?
>
>I happened to catch it due to a BUG_ON() in my 
>device_alloc_chan_resources
>that checked to make sure it wasn't allocated twice without a free
>inbetween. It hit on the second load of the dmatest module, since they
>were never freed on unload.
>
>
>-Olof
>

No, you're not missing anything, you've read it right.  Notice at the
top of the ioatdma's ioat_dma_alloc_chan_resources() we check to see if
we've already been here.

I suspect this is a hangover from the earlier dmaengine design where TCP
was the only client and it never released things, so the only time we
needed to free the resources was when ioatdma was unloaded.

sln
--
======================================================================
Mr. Shannon Nelson                 LAN Access Division, Intel Corp.
Shannon.Nelson@intel.com                I don't speak for Intel
(503) 712-7659                    Parents can't afford to be squeamish.