[PATCH] dmatest: masking tests for channel capabilities

[PATCH] dmatest: masking tests for channel capabilities

[Jubin Mehta]

The current dmatest module tests all the hardware capabilities (MEMCPY, XOR
and PQ) supported by a particular DMA channel and these tests are performed
concurrently by default. This patch allows the user to enable or disable the
test performed for any particular capability. The mask bits for enabling the
tests are set using the debugfs.

Signed-off-by: Jubin Mehta <jubin.mehta@intel.com>
---
 Documentation/dmatest.txt |   32 ++++++++++++++++++++++++++++++++
 drivers/dma/dmatest.c     |   41 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/Documentation/dmatest.txt b/Documentation/dmatest.txt
index 132a094..e5fd29c 100644
--- a/Documentation/dmatest.txt
+++ b/Documentation/dmatest.txt
@@ -79,3 +79,35 @@ Comparison between buffers is stored to the dedicated structure.
 
 Note that the verify result is now accessible only via file 'results' in the
 debugfs.
+
+	Part 5 - Masking tests for channel capabilities
+
+By default, the dmatest module concurrently tests all the hardware capabilities
+(MEMCPY, XOR and PQ) supported by a particular DMA channel. In order to disable
+any capability/capabilities from being tested, the user needs to modify the
+file 'cap_mask' in debugfs.
+
+Method to enable the capabilities:
+
+PATH = /sys/kernel/debug/dmatest/cap_mask
+(DEFAULT) echo 0x07 > $PATH     // Set Bits 0,1,2 for MEMCPY, XOR and PQ
+          echo 0x01 > $PATH     // Set bit 0 to enable MEMCPY
+          echo 0x02 > $PATH     // Set bit 1 to enable XOR
+          echo 0x04 > $PATH     // Set bit 2 to enable PQ
+
+Example to perform only MEMCPY and PQ mode tests (0x01 | 0x04 = 0x05):
+
+        % modprobe dmatest
+        % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
+        % echo 5  > /sys/kernel/debug/dmatest/cap_mask
+        % echo 1 > /sys/kernel/debug/dmatest/iterations
+        % echo 1 > /sys/kernel/debug/dmatest/run
+
+	Output:
+	% cat /sys/kernel/debug/dmatest/results
+
+        dma0chan0-copy0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
+        dma0chan0-pq0: #1: No errors with src_off=0xXX dst_off=0xXX len=0xXX
+
+Note that the result is available only for the enabled capabilities of
+particular channel.
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index e88ded2..5420fc7 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -24,6 +24,21 @@
 #include <linux/uaccess.h>
 #include <linux/seq_file.h>
 
+/*
+ * Capability Mask Bits.The bits in the cap_mask denote the masking of
+ * the hardware capabilities of the dma channel.
+ *
+ * DMA_CAP_MEMCPY:  Bit 0 for enabling DMA_MEMCPY capability
+ * DMA_CAP_XOR:     Bit 1 for enabling DMA_XOR capabilit
+ * DMA_CAP_PQ:      Bit 2 for enabling DMA_PQ capability
+ * DMA_CAP_ALL:     Enable all the capabilities of the channel
+ */
+#define DMA_CAP_MEMCPY		(1 << 0)
+#define DMA_CAP_XOR		(1 << 1)
+#define DMA_CAP_PQ		(1 << 2)
+
+#define DMA_CAP_ALL		(DMA_CAP_MEMCPY | DMA_CAP_XOR | DMA_CAP_PQ)
+
 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");
@@ -66,6 +81,11 @@ module_param(timeout, uint, S_IRUGO);
 MODULE_PARM_DESC(timeout, "Transfer Timeout in msec (default: 3000), "
 		 "Pass -1 for infinite timeout");
 
+static unsigned int cap_mask = DMA_CAP_ALL;
+module_param(cap_mask, uint, S_IRUGO);
+MODULE_PARM_DESC(cap_mask,
+		"Mask bits for different capability test (default: 0x07)");
+
 /* Maximum amount of mismatched bytes in buffer to print */
 #define MAX_ERROR_COUNT		32
 
@@ -164,6 +184,7 @@ struct dmatest_chan {
  * @xor_sources:	number of xor source buffers
  * @pq_sources:		number of p+q source buffers
  * @timeout:		transfer timeout in msec, -1 for infinite timeout
+ * @cap_mask:		mask for different hardware capabilities tests
  */
 struct dmatest_params {
 	unsigned int	buf_size;
@@ -175,6 +196,7 @@ struct dmatest_params {
 	unsigned int	xor_sources;
 	unsigned int	pq_sources;
 	int		timeout;
+	unsigned int	cap_mask;
 };
 
 /**
@@ -887,6 +909,7 @@ static int dmatest_add_channel(struct dmatest_info *info,
 {
 	struct dmatest_chan	*dtc;
 	struct dma_device	*dma_dev = chan->device;
+	struct dmatest_params	*params;
 	unsigned int		thread_count = 0;
 	int cnt;
 
@@ -898,16 +921,21 @@ static int dmatest_add_channel(struct dmatest_info *info,
 
 	dtc->chan = chan;
 	INIT_LIST_HEAD(&dtc->threads);
+	params = &info->params;
 
-	if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask)) {
+	/* Run dmatest threads for the supported and enabled capabilities */
+	if (dma_has_cap(DMA_MEMCPY, dma_dev->cap_mask) &&
+	    params->cap_mask & DMA_CAP_MEMCPY) {
 		cnt = dmatest_add_threads(info, dtc, DMA_MEMCPY);
 		thread_count += cnt > 0 ? cnt : 0;
 	}
-	if (dma_has_cap(DMA_XOR, dma_dev->cap_mask)) {
+	if (dma_has_cap(DMA_XOR, dma_dev->cap_mask) &&
+	    params->cap_mask & DMA_CAP_XOR) {
 		cnt = dmatest_add_threads(info, dtc, DMA_XOR);
 		thread_count += cnt > 0 ? cnt : 0;
 	}
-	if (dma_has_cap(DMA_PQ, dma_dev->cap_mask)) {
+	if (dma_has_cap(DMA_PQ, dma_dev->cap_mask) &&
+	    params->cap_mask & DMA_CAP_PQ) {
 		cnt = dmatest_add_threads(info, dtc, DMA_PQ);
 		thread_count += cnt > 0 ? cnt : 0;
 	}
@@ -1248,6 +1276,12 @@ static int dmatest_register_dbgfs(struct dmatest_info *info)
 	if (IS_ERR_OR_NULL(d))
 		goto err_node;
 
+	/* Mask bits for hardware capabilities tests */
+	d = debugfs_create_u32("cap_mask", S_IWUSR | S_IRUGO, info->root,
+			       (u32 *)&params->cap_mask);
+	if (IS_ERR_OR_NULL(d))
+		goto err_node;
+
 	/* Run or stop threaded test */
 	d = debugfs_create_file("run", S_IWUSR | S_IRUGO, info->root,
 				info, &dtf_run_fops);
@@ -1293,6 +1327,7 @@ static int __init dmatest_init(void)
 	params->xor_sources = xor_sources;
 	params->pq_sources = pq_sources;
 	params->timeout = timeout;
+	params->cap_mask = cap_mask;
 
 	ret = dmatest_register_dbgfs(info);
 	if (ret)
-- 
1.7.9.5

Re: [PATCH] dmatest: masking tests for channel capabilities

[Andy Shevchenko]

On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <jubin.mehta@intel.com> wrote:
> The current dmatest module tests all the hardware capabilities (MEMCPY, XOR
> and PQ) supported by a particular DMA channel and these tests are performed
> concurrently by default. This patch allows the user to enable or disable the
> test performed for any particular capability. The mask bits for enabling the
> tests are set using the debugfs.

This approach I like better.
Dan, Vinod, what is your opinion?

I have couple of comments below.

> +++ b/Documentation/dmatest.txt

> +Method to enable the capabilities:
> +
> +PATH = /sys/kernel/debug/dmatest/cap_mask
> +(DEFAULT) echo 0x07 > $PATH     // Set Bits 0,1,2 for MEMCPY, XOR and PQ
> +          echo 0x01 > $PATH     // Set bit 0 to enable MEMCPY
> +          echo 0x02 > $PATH     // Set bit 1 to enable XOR
> +          echo 0x04 > $PATH     // Set bit 2 to enable PQ

What if we make examples  followed by # and comments?
And move (DEFAULT) to the commentary.

> +++ b/drivers/dma/dmatest.c

> +/*
> + * Capability Mask Bits.The bits in the cap_mask denote the masking of
> + * the hardware capabilities of the dma channel.
> + *
> + * DMA_CAP_MEMCPY:  Bit 0 for enabling DMA_MEMCPY capability
> + * DMA_CAP_XOR:     Bit 1 for enabling DMA_XOR capabilit
> + * DMA_CAP_PQ:      Bit 2 for enabling DMA_PQ capability
> + * DMA_CAP_ALL:     Enable all the capabilities of the channel
> + */
> +#define DMA_CAP_MEMCPY         (1 << 0)
> +#define DMA_CAP_XOR            (1 << 1)
> +#define DMA_CAP_PQ             (1 << 2)

Can we reuse DMA_MEMCPY and so on from enum dma_transaction_type?

--
With Best Regards,
Andy Shevchenko

Re: [PATCH] dmatest: masking tests for channel capabilities

[Jon Mason]

On Mon, Jun 17, 2013 at 11:59:00AM +0300, Andy Shevchenko wrote:
> On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <jubin.mehta@intel.com> wrote:
> > The current dmatest module tests all the hardware capabilities (MEMCPY, XOR
> > and PQ) supported by a particular DMA channel and these tests are performed
> > concurrently by default. This patch allows the user to enable or disable the
> > test performed for any particular capability. The mask bits for enabling the
> > tests are set using the debugfs.
> 
> This approach I like better.
> Dan, Vinod, what is your opinion?

I withdraw my path in favor of this one.

Thanks,
Jon

> I have couple of comments below.
> 
> > +++ b/Documentation/dmatest.txt
> 
> > +Method to enable the capabilities:
> > +
> > +PATH = /sys/kernel/debug/dmatest/cap_mask
> > +(DEFAULT) echo 0x07 > $PATH     // Set Bits 0,1,2 for MEMCPY, XOR and PQ
> > +          echo 0x01 > $PATH     // Set bit 0 to enable MEMCPY
> > +          echo 0x02 > $PATH     // Set bit 1 to enable XOR
> > +          echo 0x04 > $PATH     // Set bit 2 to enable PQ
> 
> What if we make examples  followed by # and comments?
> And move (DEFAULT) to the commentary.
> 
> > +++ b/drivers/dma/dmatest.c
> 
> > +/*
> > + * Capability Mask Bits.The bits in the cap_mask denote the masking of
> > + * the hardware capabilities of the dma channel.
> > + *
> > + * DMA_CAP_MEMCPY:  Bit 0 for enabling DMA_MEMCPY capability
> > + * DMA_CAP_XOR:     Bit 1 for enabling DMA_XOR capabilit
> > + * DMA_CAP_PQ:      Bit 2 for enabling DMA_PQ capability
> > + * DMA_CAP_ALL:     Enable all the capabilities of the channel
> > + */
> > +#define DMA_CAP_MEMCPY         (1 << 0)
> > +#define DMA_CAP_XOR            (1 << 1)
> > +#define DMA_CAP_PQ             (1 << 2)
> 
> Can we reuse DMA_MEMCPY and so on from enum dma_transaction_type?
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] dmatest: masking tests for channel capabilities

[Dave Jiang]

On 06/17/2013 01:59 AM, Andy Shevchenko wrote:
> On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <jubin.mehta@intel.com> wrote:
>> +++ b/drivers/dma/dmatest.c
>> +/*
>> + * Capability Mask Bits.The bits in the cap_mask denote the masking of
>> + * the hardware capabilities of the dma channel.
>> + *
>> + * DMA_CAP_MEMCPY:  Bit 0 for enabling DMA_MEMCPY capability
>> + * DMA_CAP_XOR:     Bit 1 for enabling DMA_XOR capabilit
>> + * DMA_CAP_PQ:      Bit 2 for enabling DMA_PQ capability
>> + * DMA_CAP_ALL:     Enable all the capabilities of the channel
>> + */
>> +#define DMA_CAP_MEMCPY         (1 << 0)
>> +#define DMA_CAP_XOR            (1 << 1)
>> +#define DMA_CAP_PQ             (1 << 2)
> Can we reuse DMA_MEMCPY and so on from enum dma_transaction_type?

The issue is that the originals are enum types. We can't pass bitmap 
type (dma_cap_mask_t) through sysfs. So to use those enums for what we 
want to do it would have to be someting like:
(1 << DMA_MEMCPY)

And also in that case for user scripts you have bits that are here and 
there for the bitmask instead of something simple and sequential.

-- 

Dave Jiang
Application Engineer, Storage Divsion
Intel Corp.
dave.jiang@intel.com

Re: [PATCH] dmatest: masking tests for channel capabilities

[Jubin Mehta]

On Mon, Jun 17, 2013 at 11:59:00AM +0300, Andy Shevchenko wrote:
> > +PATH = /sys/kernel/debug/dmatest/cap_mask
> > +(DEFAULT) echo 0x07 > $PATH     // Set Bits 0,1,2 for MEMCPY, XOR and PQ
> > +          echo 0x01 > $PATH     // Set bit 0 to enable MEMCPY
> > +          echo 0x02 > $PATH     // Set bit 1 to enable XOR
> > +          echo 0x04 > $PATH     // Set bit 2 to enable PQ
> 
> What if we make examples  followed by # and comments?
> And move (DEFAULT) to the commentary.
>
Yes, I shall fix the issue in the next patch.
 
Regards,
Jubin 

Re: [PATCH] dmatest: masking tests for channel capabilities

[Dan Williams]

[ apologies for the resend, gmail defaulted to html ]

On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <djbw@fb.com> wrote:
>> +Example to perform only MEMCPY and PQ mode tests (0x01 | 0x04 = 0x05):
>> +
>> +        % modprobe dmatest
>> +        % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
>> +        % echo 5  > /sys/kernel/debug/dmatest/cap_mask
>> +        % echo 1 > /sys/kernel/debug/dmatest/iterations
>> +        % echo 1 > /sys/kernel/debug/dmatest/run
>
>
> Hmmm, I should have paid more attention when the debugfs support was
> initially proposed for dmatest.  As it is I see duplication and
> configuration parameters getting out of sync with their module parameter
> equivalents versus just having all configuration go through module
> parameters.  module_param_call() can be used for the more complex options.
> Debugfs at this point looks like overkill for what amounts to some simple
> configuration variables and a result line.
>
> --
> Dan
>

Re: [PATCH] dmatest: masking tests for channel capabilities

[Andy Shevchenko]

On Tue, Jun 18, 2013 at 12:12 AM, Dan Williams <djbw@fb.com> wrote:
> On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <djbw@fb.com> wrote:
>>> +Example to perform only MEMCPY and PQ mode tests (0x01 | 0x04 = 0x05):
>>> +
>>> +        % modprobe dmatest
>>> +        % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
>>> +        % echo 5  > /sys/kernel/debug/dmatest/cap_mask
>>> +        % echo 1 > /sys/kernel/debug/dmatest/iterations
>>> +        % echo 1 > /sys/kernel/debug/dmatest/run
>>
>>
>> Hmmm, I should have paid more attention when the debugfs support was
>> initially proposed for dmatest.  As it is I see duplication and
>> configuration parameters getting out of sync with their module parameter
>> equivalents versus just having all configuration go through module
>> parameters.  module_param_call() can be used for the more complex options.
>> Debugfs at this point looks like overkill for what amounts to some simple
>> configuration variables and a result line.

There two main issues we fight against:
 - test automation, where we can collect results and use them later
 - annoying modprobe / modprobe -r for each test case

The module parameters were left to support old behaviour of the module.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH] dmatest: masking tests for channel capabilities

[Jubin Mehta]

On Mon, Jun 17, 2013 at 02:12:51PM -0700, Dan Williams wrote:
> [ apologies for the resend, gmail defaulted to html ]
> 
> On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <djbw@fb.com> wrote:
> >> +Example to perform only MEMCPY and PQ mode tests (0x01 | 0x04 = 0x05):
> >> +
> >> +        % modprobe dmatest
> >> +        % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
> >> +        % echo 5  > /sys/kernel/debug/dmatest/cap_mask
> >> +        % echo 1 > /sys/kernel/debug/dmatest/iterations
> >> +        % echo 1 > /sys/kernel/debug/dmatest/run
> >
> >
> > Hmmm, I should have paid more attention when the debugfs support was
> > initially proposed for dmatest.  As it is I see duplication and
> > configuration parameters getting out of sync with their module parameter
> > equivalents versus just having all configuration go through module
> > parameters.  module_param_call() can be used for the more complex options.
> > Debugfs at this point looks like overkill for what amounts to some simple
> > configuration variables and a result line.
> >
> > --
> > Dan
> 
Would you like to have some changes regarding the configuration
process for the module parameters of the dmatest?

Any other comments with respect to the patch?

Re: [PATCH] dmatest: masking tests for channel capabilities

[Dan Williams]

On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <jubin.mehta@intel.com> wrote:
> On Mon, Jun 17, 2013 at 02:12:51PM -0700, Dan Williams wrote:
>> [ apologies for the resend, gmail defaulted to html ]
>>
>> On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <djbw@fb.com> wrote:
>> >> +Example to perform only MEMCPY and PQ mode tests (0x01 | 0x04 = 0x05):
>> >> +
>> >> +        % modprobe dmatest
>> >> +        % echo dma0chan0 > /sys/kernel/debug/dmatest/channel
>> >> +        % echo 5  > /sys/kernel/debug/dmatest/cap_mask
>> >> +        % echo 1 > /sys/kernel/debug/dmatest/iterations
>> >> +        % echo 1 > /sys/kernel/debug/dmatest/run
>> >
>> >
>> > Hmmm, I should have paid more attention when the debugfs support was
>> > initially proposed for dmatest.  As it is I see duplication and
>> > configuration parameters getting out of sync with their module parameter
>> > equivalents versus just having all configuration go through module
>> > parameters.  module_param_call() can be used for the more complex options.
>> > Debugfs at this point looks like overkill for what amounts to some simple
>> > configuration variables and a result line.
>> >
>> > --
>> > Dan
>>
> Would you like to have some changes regarding the configuration
> process for the module parameters of the dmatest?

Yes, as a first step I would like to see a clean up of the the
configuration parameters to be available via
/sys/module/dmatest/parameters rather than /sys/kernel/debug/dmatest

As for "run" and "results" I see Andy's point that those are a bit
awkward as parameters.  However, we do have trace points as a more
general mechanism for dumping events and data to userspace.  If we had
/sys/module/dmatest/parameters/run with a tracepoint for the result
line does that get us everything we need for automation?  I can see
more tracepoints beng added to get some perf metrics out of the tests.
 Thoughts?

> Any other comments with respect to the patch?

How about a comma separated list of named capabilities (copy, xor, pq,
xor_val, sg... etc) rather than a mask?  If we are already not using
the dma_transaction_type values might as well use something human
readable.

--
Dan

Re: [PATCH] dmatest: masking tests for channel capabilities

[Andy Shevchenko]

On Tue, Jun 18, 2013 at 10:16 PM, Dan Williams <djbw@fb.com> wrote:
> On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <jubin.mehta@intel.com> wrote:

>> Would you like to have some changes regarding the configuration
>> process for the module parameters of the dmatest?
>
> Yes, as a first step I would like to see a clean up of the the
> configuration parameters to be available via
> /sys/module/dmatest/parameters rather than /sys/kernel/debug/dmatest

Do  you mean to enable write access to them?

> As for "run" and "results" I see Andy's point that those are a bit
> awkward as parameters.  However, we do have trace points as a more
> general mechanism for dumping events and data to userspace.  If we had
> /sys/module/dmatest/parameters/run with a tracepoint for the result
> line does that get us everything we need for automation?  I can see
> more tracepoints beng added to get some perf metrics out of the tests.

I'm not familiar neither with perf, nor with tracepoints. Could you
elaborate and show here the scheme how it should work? How we will get
results? In what form?


--
With Best Regards,
Andy Shevchenko

Re: [PATCH] dmatest: masking tests for channel capabilities

[Dan Williams]

On Tue, Jun 18, 2013 at 1:01 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jun 18, 2013 at 10:16 PM, Dan Williams <djbw@fb.com> wrote:
>> On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <jubin.mehta@intel.com> wrote:
>
>>> Would you like to have some changes regarding the configuration
>>> process for the module parameters of the dmatest?
>>
>> Yes, as a first step I would like to see a clean up of the the
>> configuration parameters to be available via
>> /sys/module/dmatest/parameters rather than /sys/kernel/debug/dmatest
>
> Do  you mean to enable write access to them?

Yes.

>
>> As for "run" and "results" I see Andy's point that those are a bit
>> awkward as parameters.  However, we do have trace points as a more
>> general mechanism for dumping events and data to userspace.  If we had
>> /sys/module/dmatest/parameters/run with a tracepoint for the result
>> line does that get us everything we need for automation?  I can see
>> more tracepoints beng added to get some perf metrics out of the tests.
>
> I'm not familiar neither with perf, nor with tracepoints. Could you
> elaborate and show here the scheme how it should work? How we will get
> results? In what form?
>

It would be very similar to what it does now except using generic
functionality.  At a high level you would have a tracepoint that emits
something like:

dma0chan0-copy0: #1: No errors with src_off=0x7bf dst_off=0x8ad len=0x3fea (0)

...and that message shows up in the ftrace buffer when it fires.

These articles are a good starting point.
https://lwn.net/Articles/379903/
https://lwn.net/Articles/383362/