Re: [PATCH 1/4] dma-debug: Use pr_fmt()

2018-12-04 Thread Joe Perches 
On Mon, 2018-12-03 at 17:28 +, Robin Murphy wrote:
> Use pr_fmt() to generate the "DMA-API: " prefix consistently. This
> results in it being added to a couple of pr_*() messages which were
> missing it before, and for the err_printk() calls moves it to the actual
> start of the message instead of somewhere in the middle.
> 
> Signed-off-by: Robin Murphy 
> ---
> 
> I chose not to refactor the existing split strings for minimal churn here.
> 
>  kernel/dma/debug.c | 74 --
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 231ca4628062..91b84140e4a5 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -17,6 +17,8 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
>   */
>  
> +#define pr_fmt(fmt)  "DMA-API: " fmt
> +
>  #include 
>  #include 
>  #include 
> @@ -234,7 +236,7 @@ static bool driver_filter(struct device *dev)
>   error_count += 1;   \
>   if (driver_filter(dev) &&   \
>   (show_all_errors || show_num_errors > 0)) { \
> - WARN(1, "%s %s: " format,   \
> + WARN(1, pr_fmt("%s %s: ") format,   \
>dev ? dev_driver_string(dev) : "NULL", \
>dev ? dev_name(dev) : "NULL", ## arg); \
>   dump_entry_trace(entry);\

I think converting this WARN to

dev_err(dev, format, ##__VA_ARGS__);
dump_stack();

would look better and be more intelligible.

Perhaps add a #define for dev_fmt if really necessary.


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/4] dma-debug: Use pr_fmt()

2018-12-04 Thread Christoph Hellwig 
Looks good,

Reviewed-by: Christoph Hellwig 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/4] dma-debug: Use pr_fmt()

2018-12-03 Thread Robin Murphy 
Use pr_fmt() to generate the "DMA-API: " prefix consistently. This
results in it being added to a couple of pr_*() messages which were
missing it before, and for the err_printk() calls moves it to the actual
start of the message instead of somewhere in the middle.

Signed-off-by: Robin Murphy 
---

I chose not to refactor the existing split strings for minimal churn here.

 kernel/dma/debug.c | 74 --
 1 file changed, 38 insertions(+), 36 deletions(-)

diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 231ca4628062..91b84140e4a5 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -17,6 +17,8 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#define pr_fmt(fmt)"DMA-API: " fmt
+
 #include 
 #include 
 #include 
@@ -234,7 +236,7 @@ static bool driver_filter(struct device *dev)
error_count += 1;   \
if (driver_filter(dev) &&   \
(show_all_errors || show_num_errors > 0)) { \
-   WARN(1, "%s %s: " format,   \
+   WARN(1, pr_fmt("%s %s: ") format,   \
 dev ? dev_driver_string(dev) : "NULL", \
 dev ? dev_name(dev) : "NULL", ## arg); \
dump_entry_trace(entry);\
@@ -519,7 +521,7 @@ static void active_cacheline_inc_overlap(phys_addr_t cln)
 * prematurely.
 */
WARN_ONCE(overlap > ACTIVE_CACHELINE_MAX_OVERLAP,
- "DMA-API: exceeded %d overlapping mappings of cacheline 
%pa\n",
+ pr_fmt("exceeded %d overlapping mappings of cacheline %pa\n"),
  ACTIVE_CACHELINE_MAX_OVERLAP, );
 }
 
@@ -614,7 +616,7 @@ void debug_dma_assert_idle(struct page *page)
 
cln = to_cacheline_number(entry);
err_printk(entry->dev, entry,
-  "DMA-API: cpu touching an active dma mapped cacheline 
[cln=%pa]\n",
+  "cpu touching an active dma mapped cacheline [cln=%pa]\n",
   );
 }
 
@@ -634,7 +636,7 @@ static void add_dma_entry(struct dma_debug_entry *entry)
 
rc = active_cacheline_insert(entry);
if (rc == -ENOMEM) {
-   pr_err("DMA-API: cacheline tracking ENOMEM, dma-debug 
disabled\n");
+   pr_err("cacheline tracking ENOMEM, dma-debug disabled\n");
global_disable = true;
}
 
@@ -673,7 +675,7 @@ static struct dma_debug_entry *dma_entry_alloc(void)
if (list_empty(_entries)) {
global_disable = true;
spin_unlock_irqrestore(_entries_lock, flags);
-   pr_err("DMA-API: debugging out of memory - disabling\n");
+   pr_err("debugging out of memory - disabling\n");
return NULL;
}
 
@@ -777,7 +779,7 @@ static int prealloc_memory(u32 num_entries)
num_free_entries = num_entries;
min_free_entries = num_entries;
 
-   pr_info("DMA-API: preallocated %d debug entries\n", num_entries);
+   pr_info("preallocated %d debug entries\n", num_entries);
 
return 0;
 
@@ -850,7 +852,7 @@ static ssize_t filter_write(struct file *file, const char 
__user *userbuf,
 * switched off.
 */
if (current_driver_name[0])
-   pr_info("DMA-API: switching off dma-debug driver 
filter\n");
+   pr_info("switching off dma-debug driver filter\n");
current_driver_name[0] = 0;
current_driver = NULL;
goto out_unlock;
@@ -868,7 +870,7 @@ static ssize_t filter_write(struct file *file, const char 
__user *userbuf,
current_driver_name[i] = 0;
current_driver = NULL;
 
-   pr_info("DMA-API: enable driver filter for driver [%s]\n",
+   pr_info("enable driver filter for driver [%s]\n",
current_driver_name);
 
 out_unlock:
@@ -887,7 +889,7 @@ static int dma_debug_fs_init(void)
 {
dma_debug_dent = debugfs_create_dir("dma-api", NULL);
if (!dma_debug_dent) {
-   pr_err("DMA-API: can not create debugfs directory\n");
+   pr_err("can not create debugfs directory\n");
return -ENOMEM;
}
 
@@ -973,7 +975,7 @@ static int dma_debug_device_change(struct notifier_block 
*nb, unsigned long acti
count = device_dma_allocations(dev, );
if (count == 0)
break;
-   err_printk(dev, entry, "DMA-API: device driver has pending "
+   err_printk(dev, entry, "device driver has pending "
"DMA allocations while released from device "
"[count=%d]\n"
"One of leaked entries details: "
@@ -1023,14 +1025,14 @@