diff mbox

dmatest: verbose mode

Message ID 20131108211040.27762.45313.stgit@viggo.jf.intel.com (mailing list archive)
State Superseded
Delegated to: Dan Williams
Headers show

Commit Message

Dan Williams Nov. 8, 2013, 9:11 p.m. UTC
Verbose mode turns on test success messages, by default we only output
test summaries and failure results.

Also cleaned up some stray quotes, leftover from putting the result
message format string all on one line.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/dma/dmatest.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 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

Linus Walleij Nov. 11, 2013, 9:27 a.m. UTC | #1
On Fri, Nov 8, 2013 at 10:11 PM, Dan Williams <dan.j.williams@intel.com> wrote:

> Verbose mode turns on test success messages, by default we only output
> test summaries and failure results.
>
> Also cleaned up some stray quotes, leftover from putting the result
> message format string all on one line.
>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
(...)
> +#define pr_verbose(fmt, ...) ({ \
> +       if (verbose) \
> +               pr_info(fmt, ##__VA_ARGS__); \
> +       else \
> +               pr_debug(fmt, ##__VA_ARGS__); \
> +})

Hm hm. The kernel already has things like this:

<linux/device.h>:

#ifdef VERBOSE_DEBUG
#define dev_vdbg        dev_dbg
#else
#define dev_vdbg(dev, format, arg...)                           \
({                                                              \
        if (0)                                                  \
                dev_printk(KERN_DEBUG, dev, format, ##arg);     \
        0;                                                      \
})
#endif

This is a bit half-done as pr_vdbg() etc does not exist but
there is some intent to generalize verbose debugging atleast.

Now that is definately a compile-time thing, not runtime like
this, but to avoid confusion, maybe a dma-unique name like
pr_dmavdbg() or something could be used?

Yours,
Linus Walleij
--
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
Dan Williams Nov. 12, 2013, 12:52 a.m. UTC | #2
On Mon, Nov 11, 2013 at 1:27 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Nov 8, 2013 at 10:11 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>
>> Verbose mode turns on test success messages, by default we only output
>> test summaries and failure results.
>>
>> Also cleaned up some stray quotes, leftover from putting the result
>> message format string all on one line.
>>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> (...)
>> +#define pr_verbose(fmt, ...) ({ \
>> +       if (verbose) \
>> +               pr_info(fmt, ##__VA_ARGS__); \
>> +       else \
>> +               pr_debug(fmt, ##__VA_ARGS__); \
>> +})
>
> Hm hm. The kernel already has things like this:
>
> <linux/device.h>:
>
> #ifdef VERBOSE_DEBUG
> #define dev_vdbg        dev_dbg
> #else
> #define dev_vdbg(dev, format, arg...)                           \
> ({                                                              \
>         if (0)                                                  \
>                 dev_printk(KERN_DEBUG, dev, format, ##arg);     \
>         0;                                                      \
> })
> #endif
>
> This is a bit half-done as pr_vdbg() etc does not exist but
> there is some intent to generalize verbose debugging atleast.
>
> Now that is definately a compile-time thing, not runtime like
> this, but to avoid confusion, maybe a dma-unique name like
> pr_dmavdbg() or something could be used?

Fair enough.  I'll make it specifically about the result verbosity.
--
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. 12, 2013, 12:34 p.m. UTC | #3
On Fri, 2013-11-08 at 13:11 -0800, Dan Williams wrote:
> Verbose mode turns on test success messages, by default we only output
> test summaries and failure results.
> 
> Also cleaned up some stray quotes, leftover from putting the result
> message format string all on one line.
> 

As I mentioned in previous mail this patch is not needed.


> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/dma/dmatest.c |   17 ++++++++++++++---
>  1 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index 0d050d2324e3..b33aa4c55047 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -70,6 +70,17 @@ static bool noverify;
>  module_param(noverify, bool, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(noverify, "Disable random data setup and verification");
>  
> +static bool verbose;
> +module_param(verbose, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(verbose, "Enable test \"success\" messages (default: off)");
> +
> +#define pr_verbose(fmt, ...) ({ \
> +	if (verbose) \
> +		pr_info(fmt, ##__VA_ARGS__); \
> +	else \
> +		pr_debug(fmt, ##__VA_ARGS__); \
> +})
> +
>  /**
>   * struct dmatest_params - test parameters.
>   * @buf_size:		size of the memcpy test buffer
> @@ -336,7 +347,7 @@ static unsigned int min_odd(unsigned int x, unsigned int y)
>  static void result(const char *err, unsigned int n, unsigned int src_off,
>  		   unsigned int dst_off, unsigned int len, unsigned long data)
>  {
> -	pr_info("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)",
> +	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);
>  }
>  
> @@ -344,8 +355,8 @@ static void dbg_result(const char *err, unsigned int n, unsigned int src_off,
>  		       unsigned int dst_off, unsigned int len,
>  		       unsigned long data)
>  {
> -	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);
> +	pr_verbose("%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 unsigned long long dmatest_persec(s64 runtime, unsigned int val)
>
Dan Williams Nov. 12, 2013, 11:43 p.m. UTC | #4
On Tue, Nov 12, 2013 at 4:34 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2013-11-08 at 13:11 -0800, Dan Williams wrote:
>> Verbose mode turns on test success messages, by default we only output
>> test summaries and failure results.
>>
>> Also cleaned up some stray quotes, leftover from putting the result
>> message format string all on one line.
>>
>
> As I mentioned in previous mail this patch is not needed.
>
>

When addressing Linus' comment I made sure to clarify that verbose
mode is about test results and not debug output.

See: https://patchwork.kernel.org/patch/3170391/
--
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
diff mbox

Patch

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 0d050d2324e3..b33aa4c55047 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -70,6 +70,17 @@  static bool noverify;
 module_param(noverify, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(noverify, "Disable random data setup and verification");
 
+static bool verbose;
+module_param(verbose, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(verbose, "Enable test \"success\" messages (default: off)");
+
+#define pr_verbose(fmt, ...) ({ \
+	if (verbose) \
+		pr_info(fmt, ##__VA_ARGS__); \
+	else \
+		pr_debug(fmt, ##__VA_ARGS__); \
+})
+
 /**
  * struct dmatest_params - test parameters.
  * @buf_size:		size of the memcpy test buffer
@@ -336,7 +347,7 @@  static unsigned int min_odd(unsigned int x, unsigned int y)
 static void result(const char *err, unsigned int n, unsigned int src_off,
 		   unsigned int dst_off, unsigned int len, unsigned long data)
 {
-	pr_info("%s: result #%u: '%s' with src_off=0x%x ""dst_off=0x%x len=0x%x (%lu)",
+	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);
 }
 
@@ -344,8 +355,8 @@  static void dbg_result(const char *err, unsigned int n, unsigned int src_off,
 		       unsigned int dst_off, unsigned int len,
 		       unsigned long data)
 {
-	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);
+	pr_verbose("%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 unsigned long long dmatest_persec(s64 runtime, unsigned int val)