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 <[email protected]>
---
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 *)¶ms->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
On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <[email protected]> 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
On Mon, Jun 17, 2013 at 11:59:00AM +0300, Andy Shevchenko wrote:
> On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <[email protected]> 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
On 06/17/2013 01:59 AM, Andy Shevchenko wrote:
> On Thu, Jun 13, 2013 at 8:24 PM, Jubin Mehta <[email protected]> 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.
[email protected]
[ apologies for the resend, gmail defaulted to html ]
On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <[email protected]> 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
>
On Tue, Jun 18, 2013 at 12:12 AM, Dan Williams <[email protected]> wrote:
> On Mon, Jun 17, 2013 at 2:10 PM, Dan Williams <[email protected]> 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
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 <[email protected]> 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?
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
On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <[email protected]> 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 <[email protected]> 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
On Tue, Jun 18, 2013 at 10:16 PM, Dan Williams <[email protected]> wrote:
> On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <[email protected]> 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
On Tue, Jun 18, 2013 at 1:01 PM, Andy Shevchenko
<[email protected]> wrote:
> On Tue, Jun 18, 2013 at 10:16 PM, Dan Williams <[email protected]> wrote:
>> On Tue, Jun 18, 2013 at 10:34 AM, Jubin Mehta <[email protected]> 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/