[PATCH 0/18] DMA-API debugging facility v4

[PATCH 0/18] DMA-API debugging facility v4

[Joerg Roedel]

Hi,

this is version 4 of the patchset which introduces code to debug drivers
usage of the DMA-API. Many thanks to all the reviewers and the useful
comments on the previous versions of this patchset. Appended is a
changelog, the shortlog and the diff-stat.

Changes from v3 -> v4:

- a patch from David Woodhouse adds printing of the mapping path
  stacktrace on an unmap error
- another patch from David added a function which drivers can use to
  dump their DMA mappings
- two new checks in the sync-path check if the dma memory was mapped for
  the requested sync direction
- a check for mapping requests of memory on kernel stacks was added
  (thanks to Arndt Bergmann)
- A bug in the handling of dma_map_sg/dma_unmap_sg pointed out by
  FUJITA Tomonori was fixed
- As a result of the previous fix a check was added to find if a driver
  unmaps different count of sg entries than it mapped
- Various changes to the hash (larger hash size, hash function uses
  lower bits than before)
- Some minor fixes pointed out by reviewers

Changes from v2 -> v3:

- rebased patches against tip/core/iommu branch
- changed storage of virtual address to physical address in
  struct dma_debug_entry (thanks Fujita)
- removed usage of x86 specific bad_dma_address (thanks Fujita)
- changed a error log message to be more clear (thanks Roel)
- fixed a bug with wrong handling of map_page/unmap_page requests
  (thanks Michael)
- various improvements and fixes suggested by Ingo, thanks
- added more comments

Changes from v1 -> v2:

- moved code to lib/ and include/linux to make it usable for all
  architectures
- more fine grained hash locking (locking is now per hash
  bucket, no global lock anymore)
- dma_debug_entries are preallocated
- per default the code will only print one warning and is
  silent then
- added a debugfs interface to see some statistics and to
  enable more verbose error reporting in the kernel log
- added command line parameter to disable debugging code
- allocation errors are now handled correctly
- added documentation about this facility for driver developers


	Joerg

Shortlog:

David Woodhouse (2):
      dma-debug: add function to dump dma mappings
      dma-debug: print stacktrace of mapping path on unmap error

Joerg Roedel (16):
      dma-debug: add Kconfig entry
      dma-debug: add header file and core data structures
      dma-debug: add hash functions for dma_debug_entries
      dma-debug: add allocator code
      dma-debug: add initialization code
      dma-debug: add kernel command line parameters
      dma-debug: add debugfs interface
      dma-debug: add core checking functions
      dma-debug: add checking for map/unmap_page/single
      dma-debug: add add checking for map/unmap_sg
      dma-debug: add checking for [alloc|free]_coherent
      dma-debug: add checks for sync_single_*
      dma-debug: add checks for sync_single_range_*
      dma-debug: add checks for sync_single_sg_*
      dma-debug: x86 architecture bindings
      dma-debug: Documentation update

Diffstat:

 Documentation/DMA-API.txt           |  106 +++++
 Documentation/kernel-parameters.txt |   10 +
 arch/Kconfig                        |    2 +
 arch/x86/Kconfig                    |    1 +
 arch/x86/include/asm/dma-mapping.h  |   45 ++-
 arch/x86/kernel/pci-dma.c           |    6 +
 include/linux/dma-debug.h           |  167 +++++++
 lib/Kconfig.debug                   |   11 +
 lib/Makefile                        |    2 +
 lib/dma-debug.c                     |  870 +++++++++++++++++++++++++++++++++++
 10 files changed, 1214 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/dma-debug.h
 create mode 100644 lib/dma-debug.c

[PATCH 01/18] dma-debug: add Kconfig entry

[Joerg Roedel]

Impact: add a Kconfig entry for DMA-API debugging

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/Kconfig      |    2 ++
 lib/Kconfig.debug |   11 +++++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 550dab2..830c16a 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -106,3 +106,5 @@ config HAVE_CLK
 	  The <linux/clk.h> calls support software clock gating and
 	  thus are a key power management tool on many systems.
 
+config HAVE_DMA_API_DEBUG
+	bool
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1bcf9cd..d9cbada 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -902,6 +902,17 @@ config DYNAMIC_PRINTK_DEBUG
 	  debugging for all modules. This mode can be turned off via the above
 	  disable command.
 
+config DMA_API_DEBUG
+	bool "Enable debugging of DMA-API usage"
+	depends on HAVE_DMA_API_DEBUG
+	help
+	  Enable this option to debug the use of the DMA API by device drivers.
+	  With this option you will be able to detect common bugs in device
+	  drivers like double-freeing of DMA mappings or freeing mappings that
+	  were never allocated.
+	  This option causes a performance degredation.  Use only if you want
+	  to debug device drivers. If unsure, say N.
+
 source "samples/Kconfig"
 
 source "lib/Kconfig.kgdb"
-- 
1.5.6.4

[PATCH 02/18] dma-debug: add header file and core data structures

[Joerg Roedel]

Impact: add groundwork for DMA-API debugging

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   25 +++++++++++++++++++++++++
 lib/Makefile              |    2 ++
 lib/dma-debug.c           |   42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/dma-debug.h
 create mode 100644 lib/dma-debug.c

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
new file mode 100644
index 0000000..ce4ace7
--- /dev/null
+++ b/include/linux/dma-debug.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
+ *
+ * Author: Joerg Roedel <joerg.roedel@amd.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#ifndef __DMA_DEBUG_H
+#define __DMA_DEBUG_H
+
+struct device;
+
+#endif /* __DMA_DEBUG_H */
diff --git a/lib/Makefile b/lib/Makefile
index 32b0e64..50b48cf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,6 +84,8 @@ obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
 
 obj-$(CONFIG_DYNAMIC_PRINTK_DEBUG) += dynamic_printk.o
 
+obj-$(CONFIG_DMA_API_DEBUG) += dma-debug.o
+
 hostprogs-y	:= gen_crc32table
 clean-files	:= crc32table.h
 
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
new file mode 100644
index 0000000..3109971
--- /dev/null
+++ b/lib/dma-debug.c
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2008 Advanced Micro Devices, Inc.
+ *
+ * Author: Joerg Roedel <joerg.roedel@amd.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#include <linux/dma-debug.h>
+#include <linux/types.h>
+#include <linux/list.h>
+
+enum {
+	dma_debug_single,
+	dma_debug_page,
+	dma_debug_sg,
+	dma_debug_coherent,
+};
+
+struct dma_debug_entry {
+	struct list_head list;
+	struct device    *dev;
+	int              type;
+	phys_addr_t      paddr;
+	u64              dev_addr;
+	u64              size;
+	int              direction;
+	int		 sg_call_ents;
+	int		 sg_mapped_ents;
+};
+
-- 
1.5.6.4

[PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Joerg Roedel]

Impact: implement necessary functions for the core hash

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 lib/dma-debug.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 3109971..5ff7d2e 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -18,9 +18,14 @@
  */
 
 #include <linux/dma-debug.h>
+#include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/list.h>
 
+#define HASH_SIZE       1024ULL
+#define HASH_FN_SHIFT   13
+#define HASH_FN_MASK    (HASH_SIZE - 1)
+
 enum {
 	dma_debug_single,
 	dma_debug_page,
@@ -40,3 +45,99 @@ struct dma_debug_entry {
 	int		 sg_mapped_ents;
 };
 
+struct hash_bucket {
+	struct list_head list;
+	spinlock_t lock;
+} __cacheline_aligned_in_smp;
+
+/* Hash list to save the allocated dma addresses */
+static struct hash_bucket dma_entry_hash[HASH_SIZE];
+
+/*
+ * Hash related functions
+ *
+ * Every DMA-API request is saved into a struct dma_debug_entry. To
+ * have quick access to these structs they are stored into a hash.
+ */
+static int hash_fn(struct dma_debug_entry *entry)
+{
+	/*
+	 * Hash function is based on the dma address.
+	 * We use bits 20-27 here as the index into the hash
+	 */
+	return (entry->dev_addr >> HASH_FN_SHIFT) & HASH_FN_MASK;
+}
+
+/*
+ * Request exclusive access to a hash bucket for a given dma_debug_entry.
+ */
+static struct hash_bucket *get_hash_bucket(struct dma_debug_entry *entry,
+					   unsigned long *flags)
+{
+	int idx = hash_fn(entry);
+	unsigned long __flags;
+
+	spin_lock_irqsave(&dma_entry_hash[idx].lock, __flags);
+	*flags = __flags;
+	return &dma_entry_hash[idx];
+}
+
+/*
+ * Give up exclusive access to the hash bucket
+ */
+static void put_hash_bucket(struct hash_bucket *bucket,
+			    unsigned long *flags)
+{
+	unsigned long __flags = *flags;
+
+	spin_unlock_irqrestore(&bucket->lock, __flags);
+}
+
+/*
+ * Search a given entry in the hash bucket list
+ */
+static struct dma_debug_entry *hash_bucket_find(struct hash_bucket *bucket,
+						struct dma_debug_entry *ref)
+{
+	struct dma_debug_entry *entry;
+
+	list_for_each_entry(entry, &bucket->list, list) {
+		if ((entry->dev_addr == ref->dev_addr) &&
+		    (entry->dev == ref->dev))
+			return entry;
+	}
+
+	return NULL;
+}
+
+/*
+ * Add an entry to a hash bucket
+ */
+static void hash_bucket_add(struct hash_bucket *bucket,
+			    struct dma_debug_entry *entry)
+{
+	list_add_tail(&entry->list, &bucket->list);
+}
+
+/*
+ * Remove entry from a hash bucket list
+ */
+static void hash_bucket_del(struct dma_debug_entry *entry)
+{
+	list_del(&entry->list);
+}
+
+/*
+ * Wrapper function for adding an entry to the hash.
+ * This function takes care of locking itself.
+ */
+static void add_dma_entry(struct dma_debug_entry *entry)
+{
+	struct hash_bucket *bucket;
+	unsigned long flags;
+
+	bucket = get_hash_bucket(entry, &flags);
+	hash_bucket_add(bucket, entry);
+	put_hash_bucket(bucket, &flags);
+}
+
-- 
1.5.6.4

[PATCH 04/18] dma-debug: add allocator code

[Joerg Roedel]

Impact: add allocator code for struct dma_debug_entry

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 lib/dma-debug.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 5ff7d2e..b609146 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -52,6 +52,16 @@ struct hash_bucket {
 
 /* Hash list to save the allocated dma addresses */
 static struct hash_bucket dma_entry_hash[HASH_SIZE];
+/* List of pre-allocated dma_debug_entry's */
+static LIST_HEAD(free_entries);
+/* Lock for the list above */
+static DEFINE_SPINLOCK(free_entries_lock);
+
+/* Global disable flag - will be set in case of an error */
+static bool global_disable __read_mostly;
+
+static u32 num_free_entries;
+static u32 min_free_entries;
 
 /*
  * Hash related functions
@@ -141,3 +151,50 @@ static void add_dma_entry(struct dma_debug_entry *entry)
 	put_hash_bucket(bucket, &flags);
 }
 
+/* struct dma_entry allocator
+ *
+ * The next two functions implement the allocator for
+ * struct dma_debug_entries.
+ */
+static struct dma_debug_entry *dma_entry_alloc(void)
+{
+	struct dma_debug_entry *entry = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&free_entries_lock, flags);
+
+	if (list_empty(&free_entries)) {
+		printk(KERN_ERR "DMA-API: debugging out of memory "
+				"- disabling\n");
+		global_disable = true;
+		goto out;
+	}
+
+	entry = list_entry(free_entries.next, struct dma_debug_entry, list);
+	list_del(&entry->list);
+	memset(entry, 0, sizeof(*entry));
+
+	num_free_entries -= 1;
+	if (num_free_entries < min_free_entries)
+		min_free_entries = num_free_entries;
+
+out:
+	spin_unlock_irqrestore(&free_entries_lock, flags);
+
+	return entry;
+}
+
+static void dma_entry_free(struct dma_debug_entry *entry)
+{
+	unsigned long flags;
+
+	/*
+	 * add to beginning of the list - this way the entries are
+	 * more likely cache hot when they are reallocated.
+	 */
+	spin_lock_irqsave(&free_entries_lock, flags);
+	list_add(&entry->list, &free_entries);
+	num_free_entries += 1;
+	spin_unlock_irqrestore(&free_entries_lock, flags);
+}
+
-- 
1.5.6.4

[PATCH 05/18] dma-debug: add initialization code

[Joerg Roedel]

Impact: add code to initialize dma-debug core data structures

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   14 +++++++++
 lib/dma-debug.c           |   66 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index ce4ace7..345d538 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -20,6 +20,20 @@
 #ifndef __DMA_DEBUG_H
 #define __DMA_DEBUG_H
 
+#include <linux/types.h>
+
 struct device;
 
+#ifdef CONFIG_DMA_API_DEBUG
+
+extern void dma_debug_init(u32 num_entries);
+
+#else /* CONFIG_DMA_API_DEBUG */
+
+static inline void dma_debug_init(u32 num_entries)
+{
+}
+
+#endif /* CONFIG_DMA_API_DEBUG */
+
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index b609146..5b50bb3 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -21,6 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/list.h>
+#include <linux/slab.h>
 
 #define HASH_SIZE       1024ULL
 #define HASH_FN_SHIFT   13
@@ -198,3 +199,68 @@ static void dma_entry_free(struct dma_debug_entry *entry)
 	spin_unlock_irqrestore(&free_entries_lock, flags);
 }
 
+/*
+ * DMA-API debugging init code
+ *
+ * The init code does two things:
+ *   1. Initialize core data structures
+ *   2. Preallocate a given number of dma_debug_entry structs
+ */
+
+static int prealloc_memory(u32 num_entries)
+{
+	struct dma_debug_entry *entry, *next_entry;
+	int i;
+
+	for (i = 0; i < num_entries; ++i) {
+		entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+		if (!entry)
+			goto out_err;
+
+		list_add_tail(&entry->list, &free_entries);
+	}
+
+	num_free_entries = num_entries;
+	min_free_entries = num_entries;
+
+	printk(KERN_INFO "DMA-API: preallocated %d debug entries\n",
+			num_entries);
+
+	return 0;
+
+out_err:
+
+	list_for_each_entry_safe(entry, next_entry, &free_entries, list) {
+		list_del(&entry->list);
+		kfree(entry);
+	}
+
+	return -ENOMEM;
+}
+
+/*
+ * Let the architectures decide how many entries should be preallocated.
+ */
+void dma_debug_init(u32 num_entries)
+{
+	int i;
+
+	if (global_disable)
+		return;
+
+	for (i = 0; i < HASH_SIZE; ++i) {
+		INIT_LIST_HEAD(&dma_entry_hash[i].list);
+		dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
+	}
+
+	if (prealloc_memory(num_entries) != 0) {
+		printk(KERN_ERR "DMA-API: debugging out of memory error "
+				"- disabled\n");
+		global_disable = true;
+
+		return;
+	}
+
+	printk(KERN_INFO "DMA-API: debugging enabled by kernel config\n");
+}
+
-- 
1.5.6.4

[PATCH 06/18] dma-debug: add kernel command line parameters

[Joerg Roedel]

Impact: add dma_debug= and dma_debug_entries= kernel parameters

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 Documentation/kernel-parameters.txt |   10 +++++++++
 lib/dma-debug.c                     |   38 +++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 54f21a5..0fa3c05 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -491,6 +491,16 @@ and is between 256 and 4096 characters. It is defined in the file
 			Range: 0 - 8192
 			Default: 64
 
+	dma_debug=off	If the kernel is compiled with DMA_API_DEBUG support
+			this option disables the debugging code at boot.
+
+	dma_debug_entries=<number>
+			This option allows to tune the number of preallocated
+			entries for DMA-API debugging code. One entry is
+			required per DMA-API allocation. Use this if the
+			DMA-API debugging code disables itself because the
+			architectural default is too low.
+
 	hpet=		[X86-32,HPET] option to control HPET usage
 			Format: { enable (default) | disable | force }
 			disable: disable HPET and use PIT instead
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 5b50bb3..2ede463 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -64,6 +64,9 @@ static bool global_disable __read_mostly;
 static u32 num_free_entries;
 static u32 min_free_entries;
 
+/* number of preallocated entries requested by kernel cmdline */
+static u32 req_entries;
+
 /*
  * Hash related functions
  *
@@ -253,6 +256,9 @@ void dma_debug_init(u32 num_entries)
 		dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
 	}
 
+	if (req_entries)
+		num_entries = req_entries;
+
 	if (prealloc_memory(num_entries) != 0) {
 		printk(KERN_ERR "DMA-API: debugging out of memory error "
 				"- disabled\n");
@@ -264,3 +270,35 @@ void dma_debug_init(u32 num_entries)
 	printk(KERN_INFO "DMA-API: debugging enabled by kernel config\n");
 }
 
+static __init int dma_debug_cmdline(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (strncmp(str, "off", 3) == 0) {
+		printk(KERN_INFO "DMA-API: debugging disabled on kernel "
+				 "command line\n");
+		global_disable = true;
+	}
+
+	return 0;
+}
+
+static __init int dma_debug_entries_cmdline(char *str)
+{
+	int res;
+
+	if (!str)
+		return -EINVAL;
+
+	res = get_option(&str, &req_entries);
+
+	if (!res)
+		req_entries = 0;
+
+	return 0;
+}
+
+__setup("dma_debug=", dma_debug_cmdline);
+__setup("dma_debug_entries=", dma_debug_entries_cmdline);
+
-- 
1.5.6.4

[PATCH 07/18] dma-debug: add debugfs interface

[Joerg Roedel]

Impact: add debugfs interface for configuring DMA-API debugging

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 lib/dma-debug.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 2ede463..20d6cdb 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -19,6 +19,7 @@
 
 #include <linux/dma-debug.h>
 #include <linux/spinlock.h>
+#include <linux/debugfs.h>
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/slab.h>
@@ -61,12 +62,29 @@ static DEFINE_SPINLOCK(free_entries_lock);
 /* Global disable flag - will be set in case of an error */
 static bool global_disable __read_mostly;
 
+/* Global error count */
+static u32 error_count;
+
+/* Global error show enable*/
+static u32 show_all_errors __read_mostly;
+/* Number of errors to show */
+static u32 show_num_errors = 1;
+
 static u32 num_free_entries;
 static u32 min_free_entries;
 
 /* number of preallocated entries requested by kernel cmdline */
 static u32 req_entries;
 
+/* debugfs dentry's for the stuff above */
+static struct dentry *dma_debug_dent        __read_mostly;
+static struct dentry *global_disable_dent   __read_mostly;
+static struct dentry *error_count_dent      __read_mostly;
+static struct dentry *show_all_errors_dent  __read_mostly;
+static struct dentry *show_num_errors_dent  __read_mostly;
+static struct dentry *num_free_entries_dent __read_mostly;
+static struct dentry *min_free_entries_dent __read_mostly;
+
 /*
  * Hash related functions
  *
@@ -241,6 +259,58 @@ out_err:
 	return -ENOMEM;
 }
 
+static int dma_debug_fs_init(void)
+{
+	dma_debug_dent = debugfs_create_dir("dma-api", NULL);
+	if (!dma_debug_dent) {
+		printk(KERN_ERR "DMA-API: can not create debugfs directory\n");
+		return -ENOMEM;
+	}
+
+	global_disable_dent = debugfs_create_bool("disabled", 0444,
+			dma_debug_dent,
+			(u32 *)&global_disable);
+	if (!global_disable_dent)
+		goto out_err;
+
+	error_count_dent = debugfs_create_u32("error_count", 0444,
+			dma_debug_dent, &error_count);
+	if (!error_count_dent)
+		goto out_err;
+
+	show_all_errors_dent = debugfs_create_u32("all_errors", 0644,
+			dma_debug_dent,
+			&show_all_errors);
+	if (!show_all_errors_dent)
+		goto out_err;
+
+	show_num_errors_dent = debugfs_create_u32("num_errors", 0644,
+			dma_debug_dent,
+			&show_num_errors);
+	if (!show_num_errors_dent)
+		goto out_err;
+
+	num_free_entries_dent = debugfs_create_u32("num_free_entries", 0444,
+			dma_debug_dent,
+			&num_free_entries);
+	if (!num_free_entries_dent)
+		goto out_err;
+
+	min_free_entries_dent = debugfs_create_u32("min_free_entries", 0444,
+			dma_debug_dent,
+			&min_free_entries);
+	if (!min_free_entries_dent)
+		goto out_err;
+
+	return 0;
+
+out_err:
+	debugfs_remove_recursive(dma_debug_dent);
+
+	return -ENOMEM;
+}
+
+
 /*
  * Let the architectures decide how many entries should be preallocated.
  */
@@ -256,6 +326,14 @@ void dma_debug_init(u32 num_entries)
 		dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
 	}
 
+	if (dma_debug_fs_init() != 0) {
+		printk(KERN_ERR "DMA-API: error creating debugfs entries "
+				"- disabling\n");
+		global_disable = true;
+
+		return;
+	}
+
 	if (req_entries)
 		num_entries = req_entries;
 
-- 
1.5.6.4

[PATCH 08/18] dma-debug: add core checking functions

[Joerg Roedel]

Impact: add functions to check on dma unmap and sync

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 lib/dma-debug.c |  188 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 187 insertions(+), 1 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 20d6cdb..d0cb47a 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -17,10 +17,13 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <linux/dma-mapping.h>
 #include <linux/dma-debug.h>
 #include <linux/spinlock.h>
 #include <linux/debugfs.h>
+#include <linux/device.h>
 #include <linux/types.h>
+#include <linux/sched.h>
 #include <linux/list.h>
 #include <linux/slab.h>
 
@@ -50,7 +53,7 @@ struct dma_debug_entry {
 struct hash_bucket {
 	struct list_head list;
 	spinlock_t lock;
-} __cacheline_aligned_in_smp;
+} ____cacheline_aligned_in_smp;
 
 /* Hash list to save the allocated dma addresses */
 static struct hash_bucket dma_entry_hash[HASH_SIZE];
@@ -85,6 +88,36 @@ static struct dentry *show_num_errors_dent  __read_mostly;
 static struct dentry *num_free_entries_dent __read_mostly;
 static struct dentry *min_free_entries_dent __read_mostly;
 
+static const char *type2name[4] = { "single", "page",
+				    "scather-gather", "coherent" };
+
+static const char *dir2name[4] = { "DMA_BIDIRECTIONAL", "DMA_TO_DEVICE",
+				   "DMA_FROM_DEVICE", "DMA_NONE" };
+
+/*
+ * The access to some variables in this macro is racy. We can't use atomic_t
+ * here because all these variables are exported to debugfs. Some of them even
+ * writeable. This is also the reason why a lock won't help much. But anyway,
+ * the races are no big deal. Here is why:
+ *
+ *   error_count: the addition is racy, but the worst thing that can happen is
+ *                that we don't count some errors
+ *   show_num_errors: the subtraction is racy. Also no big deal because in
+ *                    worst case this will result in one warning more in the
+ *                    system log than the user configured. This variable is
+ *                    writeable via debugfs.
+ */
+#define err_printk(dev, format, arg...) do {			\
+		error_count += 1;				\
+		if (show_all_errors || show_num_errors > 0) {	\
+			WARN(1, "%s %s: " format,		\
+			     dev_driver_string(dev),		\
+			     dev_name(dev) , ## arg);		\
+		}						\
+		if (!show_all_errors && show_num_errors > 0)	\
+			show_num_errors -= 1;			\
+	} while (0);
+
 /*
  * Hash related functions
  *
@@ -380,3 +413,156 @@ static __init int dma_debug_entries_cmdline(char *str)
 __setup("dma_debug=", dma_debug_cmdline);
 __setup("dma_debug_entries=", dma_debug_entries_cmdline);
 
+static void check_unmap(struct dma_debug_entry *ref)
+{
+	struct dma_debug_entry *entry;
+	struct hash_bucket *bucket;
+	unsigned long flags;
+
+	if (dma_mapping_error(ref->dev, ref->dev_addr))
+		return;
+
+	bucket = get_hash_bucket(ref, &flags);
+	entry = hash_bucket_find(bucket, ref);
+
+	if (!entry) {
+		err_printk(ref->dev, "DMA-API: device driver tries "
+			   "to free DMA memory it has not allocated "
+			   "[device address=0x%016llx] [size=%llu bytes]\n",
+			   ref->dev_addr, ref->size);
+		goto out;
+	}
+
+	if (ref->size != entry->size) {
+		err_printk(ref->dev, "DMA-API: device driver frees "
+			   "DMA memory with different size "
+			   "[device address=0x%016llx] [map size=%llu bytes] "
+			   "[unmap size=%llu bytes]\n",
+			   ref->dev_addr, entry->size, ref->size);
+	}
+
+	if (ref->type != entry->type) {
+		err_printk(ref->dev, "DMA-API: device driver frees "
+			   "DMA memory with wrong function "
+			   "[device address=0x%016llx] [size=%llu bytes] "
+			   "[mapped as %s] [unmapped as %s]\n",
+			   ref->dev_addr, ref->size,
+			   type2name[entry->type], type2name[ref->type]);
+	} else if ((entry->type == dma_debug_coherent) &&
+		   (ref->paddr != entry->paddr)) {
+		err_printk(ref->dev, "DMA-API: device driver frees "
+			   "DMA memory with different CPU address "
+			   "[device address=0x%016llx] [size=%llu bytes] "
+			   "[cpu alloc address=%p] [cpu free address=%p]",
+			   ref->dev_addr, ref->size,
+			   (void *)entry->paddr, (void *)ref->paddr);
+	}
+
+	if (ref->sg_call_ents && ref->type == dma_debug_sg &&
+	    ref->sg_call_ents != entry->sg_call_ents) {
+		err_printk(ref->dev, "DMA-API: device driver frees "
+			   "DMA sg list with different entry count "
+			   "[map count=%d] [unmap count=%d]\n",
+			   entry->sg_call_ents, ref->sg_call_ents);
+	}
+
+	/*
+	 * This may be no bug in reality - but most implementations of the
+	 * DMA API don't handle this properly, so check for it here
+	 */
+	if (ref->direction != entry->direction) {
+		err_printk(ref->dev, "DMA-API: device driver frees "
+			   "DMA memory with different direction "
+			   "[device address=0x%016llx] [size=%llu bytes] "
+			   "[mapped with %s] [unmapped with %s]\n",
+			   ref->dev_addr, ref->size,
+			   dir2name[entry->direction],
+			   dir2name[ref->direction]);
+	}
+
+	hash_bucket_del(entry);
+	dma_entry_free(entry);
+
+out:
+	put_hash_bucket(bucket, &flags);
+}
+
+static void check_for_stack(struct device *dev, void *addr)
+{
+	if (object_is_on_stack(addr))
+		err_printk(dev, "DMA-API: device driver maps memory from stack"
+				" [addr=%p]\n", addr);
+}
+
+static void check_sync(struct device *dev, dma_addr_t addr,
+		       u64 size, u64 offset, int direction, bool to_cpu)
+{
+	struct dma_debug_entry ref = {
+		.dev            = dev,
+		.dev_addr       = addr,
+		.size           = size,
+		.direction      = direction,
+	};
+	struct dma_debug_entry *entry;
+	struct hash_bucket *bucket;
+	unsigned long flags;
+
+	bucket = get_hash_bucket(&ref, &flags);
+
+	entry = hash_bucket_find(bucket, &ref);
+
+	if (!entry) {
+		err_printk(dev, "DMA-API: device driver tries "
+				"to sync DMA memory it has not allocated "
+				"[device address=0x%016llx] [size=%llu bytes]\n",
+				addr, size);
+		goto out;
+	}
+
+	if ((offset + size) > entry->size) {
+		err_printk(dev, "DMA-API: device driver syncs"
+				" DMA memory outside allocated range "
+				"[device address=0x%016llx] "
+				"[allocation size=%llu bytes] [sync offset=%llu] "
+				"[sync size=%llu]\n", entry->dev_addr, entry->size,
+				offset, size);
+	}
+
+	if (direction != entry->direction) {
+		err_printk(dev, "DMA-API: device driver syncs "
+				"DMA memory with different direction "
+				"[device address=0x%016llx] [size=%llu bytes] "
+				"[mapped with %s] [synced with %s]\n",
+				addr, entry->size,
+				dir2name[entry->direction],
+				dir2name[direction]);
+	}
+
+	if (entry->direction == DMA_BIDIRECTIONAL)
+		goto out;
+
+	if (to_cpu && !(entry->direction == DMA_FROM_DEVICE) &&
+		      !(direction == DMA_TO_DEVICE))
+		err_printk(dev, "DMA-API: device driver syncs "
+				"device read-only DMA memory for cpu "
+				"[device address=0x%016llx] [size=%llu bytes] "
+				"[mapped with %s] [synced with %s]\n",
+				addr, entry->size,
+				dir2name[entry->direction],
+				dir2name[direction]);
+
+	if (!to_cpu && !(entry->direction == DMA_TO_DEVICE) &&
+		       !(direction == DMA_FROM_DEVICE))
+		err_printk(dev, "DMA-API: device driver syncs "
+				"device write-only DMA memory to device "
+				"[device address=0x%016llx] [size=%llu bytes] "
+				"[mapped with %s] [synced with %s]\n",
+				addr, entry->size,
+				dir2name[entry->direction],
+				dir2name[direction]);
+
+out:
+	put_hash_bucket(bucket, &flags);
+
+}
+
-- 
1.5.6.4

[PATCH 09/18] dma-debug: add checking for map/unmap_page/single

[Joerg Roedel]

Impact: add debug callbacks for dma_{un}map_[page|single]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   23 +++++++++++++++++++
 lib/dma-debug.c           |   53 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 345d538..65f7352 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -28,12 +28,35 @@ struct device;
 
 extern void dma_debug_init(u32 num_entries);
 
+extern void debug_dma_map_page(struct device *dev, struct page *page,
+			       size_t offset, size_t size,
+			       int direction, dma_addr_t dma_addr,
+			       bool map_single);
+
+extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+				 size_t size, int direction, bool map_single);
+
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
 {
 }
 
+static inline void debug_dma_map_page(struct device *dev, struct page *page,
+				      size_t offset, size_t size,
+				      int direction, dma_addr_t dma_addr,
+				      bool map_single)
+{
+}
+
+static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+					size_t size, int direction,
+					bool map_single)
+{
+}
+
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index d0cb47a..a2ed2b7 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -566,3 +566,56 @@ out:
 
 }
 
+void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
+			size_t size, int direction, dma_addr_t dma_addr,
+			bool map_single)
+{
+	struct dma_debug_entry *entry;
+
+	if (unlikely(global_disable))
+		return;
+
+	if (unlikely(dma_mapping_error(dev, dma_addr)))
+		return;
+
+	entry = dma_entry_alloc();
+	if (!entry)
+		return;
+
+	entry->dev       = dev;
+	entry->type      = dma_debug_page;
+	entry->paddr     = page_to_phys(page) + offset;
+	entry->dev_addr  = dma_addr;
+	entry->size      = size;
+	entry->direction = direction;
+
+	if (map_single) {
+		entry->type = dma_debug_single;
+		check_for_stack(dev, page_address(page) + offset);
+	}
+
+	add_dma_entry(entry);
+}
+EXPORT_SYMBOL(debug_dma_map_page);
+
+void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
+			  size_t size, int direction, bool map_single)
+{
+	struct dma_debug_entry ref = {
+		.type           = dma_debug_page,
+		.dev            = dev,
+		.dev_addr       = addr,
+		.size           = size,
+		.direction      = direction,
+	};
+
+	if (unlikely(global_disable))
+		return;
+
+	if (map_single)
+		ref.type = dma_debug_single;
+
+	check_unmap(&ref);
+}
+EXPORT_SYMBOL(debug_dma_unmap_page);
+
-- 
1.5.6.4

[PATCH 10/18] dma-debug: add add checking for map/unmap_sg

[Joerg Roedel]

Impact: add debug callbacks for dma_{un}map_sg

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   16 ++++++++++
 lib/dma-debug.c           |   73 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 65f7352..ee9fdb3 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -23,6 +23,7 @@
 #include <linux/types.h>
 
 struct device;
+struct scatterlist;
 
 #ifdef CONFIG_DMA_API_DEBUG
 
@@ -36,6 +37,11 @@ extern void debug_dma_map_page(struct device *dev, struct page *page,
 extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
 				 size_t size, int direction, bool map_single);
 
+extern void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+			     int nents, int mapped_ents, int direction);
+
+extern void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
+			       int nelems, int dir);
 
 #else /* CONFIG_DMA_API_DEBUG */
 
@@ -56,6 +62,16 @@ static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
 {
 }
 
+static inline void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+				    int nents, int mapped_ents, int direction)
+{
+}
+
+static inline void debug_dma_unmap_sg(struct device *dev,
+				      struct scatterlist *sglist,
+				      int nelems, int dir)
+{
+}
 
 #endif /* CONFIG_DMA_API_DEBUG */
 
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index a2ed2b7..26e40e9 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -17,6 +17,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <linux/scatterlist.h>
 #include <linux/dma-mapping.h>
 #include <linux/dma-debug.h>
 #include <linux/spinlock.h>
@@ -619,3 +620,75 @@ void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
 }
 EXPORT_SYMBOL(debug_dma_unmap_page);
 
+void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
+		      int nents, int mapped_ents, int direction)
+{
+	struct dma_debug_entry *entry;
+	struct scatterlist *s;
+	int i;
+
+	if (unlikely(global_disable))
+		return;
+
+	for_each_sg(sg, s, mapped_ents, i) {
+		entry = dma_entry_alloc();
+		if (!entry)
+			return;
+
+		entry->type           = dma_debug_sg;
+		entry->dev            = dev;
+		entry->paddr          = sg_phys(s);
+		entry->size           = s->length;
+		entry->dev_addr       = s->dma_address;
+		entry->direction      = direction;
+		entry->sg_call_ents   = nents;
+		entry->sg_mapped_ents = mapped_ents;
+
+		check_for_stack(dev, sg_virt(s));
+
+		add_dma_entry(entry);
+	}
+}
+EXPORT_SYMBOL(debug_dma_map_sg);
+
+void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
+			int nelems, int dir)
+{
+	struct dma_debug_entry *entry;
+	struct scatterlist *s;
+	int mapped_ents = 0, i;
+	unsigned long flags;
+
+	if (unlikely(global_disable))
+		return;
+
+	for_each_sg(sglist, s, nelems, i) {
+
+		struct dma_debug_entry ref = {
+			.type           = dma_debug_sg,
+			.dev            = dev,
+			.paddr          = sg_phys(s),
+			.dev_addr       = s->dma_address,
+			.size           = s->length,
+			.direction      = dir,
+			.sg_call_ents   = 0,
+		};
+
+		if (mapped_ents && i >= mapped_ents)
+			break;
+
+		if (mapped_ents == 0) {
+			struct hash_bucket *bucket;
+			ref.sg_call_ents = nelems;
+			bucket = get_hash_bucket(&ref, &flags);
+			entry = hash_bucket_find(bucket, &ref);
+			if (entry)
+				mapped_ents = entry->sg_mapped_ents;
+			put_hash_bucket(bucket, &flags);
+		}
+
+		check_unmap(&ref);
+	}
+}
+EXPORT_SYMBOL(debug_dma_unmap_sg);
+
-- 
1.5.6.4

[PATCH 11/18] dma-debug: add checking for [alloc|free]_coherent

[Joerg Roedel]

Impact: add debug callbacks for dma_[alloc|free]_coherent

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   16 ++++++++++++++++
 lib/dma-debug.c           |   45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index ee9fdb3..cb72dfd 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -43,6 +43,12 @@ extern void debug_dma_map_sg(struct device *dev, struct scatterlist *sg,
 extern void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
 			       int nelems, int dir);
 
+extern void debug_dma_alloc_coherent(struct device *dev, size_t size,
+				     dma_addr_t dma_addr, void *virt);
+
+extern void debug_dma_free_coherent(struct device *dev, size_t size,
+				    void *virt, dma_addr_t addr);
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
@@ -73,6 +79,16 @@ static inline void debug_dma_unmap_sg(struct device *dev,
 {
 }
 
+static inline void debug_dma_alloc_coherent(struct device *dev, size_t size,
+					    dma_addr_t dma_addr, void *virt)
+{
+}
+
+static inline void debug_dma_free_coherent(struct device *dev, size_t size,
+					   void *virt, dma_addr_t addr)
+{
+}
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 26e40e9..44af837 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -692,3 +692,48 @@ void debug_dma_unmap_sg(struct device *dev, struct scatterlist *sglist,
 }
 EXPORT_SYMBOL(debug_dma_unmap_sg);
 
+void debug_dma_alloc_coherent(struct device *dev, size_t size,
+			      dma_addr_t dma_addr, void *virt)
+{
+	struct dma_debug_entry *entry;
+
+	if (unlikely(global_disable))
+		return;
+
+	if (unlikely(virt == NULL))
+		return;
+
+	entry = dma_entry_alloc();
+	if (!entry)
+		return;
+
+	entry->type      = dma_debug_coherent;
+	entry->dev       = dev;
+	entry->paddr     = virt_to_phys(virt);
+	entry->size      = size;
+	entry->dev_addr  = dma_addr;
+	entry->direction = DMA_BIDIRECTIONAL;
+
+	add_dma_entry(entry);
+}
+EXPORT_SYMBOL(debug_dma_alloc_coherent);
+
+void debug_dma_free_coherent(struct device *dev, size_t size,
+			 void *virt, dma_addr_t addr)
+{
+	struct dma_debug_entry ref = {
+		.type           = dma_debug_coherent,
+		.dev            = dev,
+		.paddr          = virt_to_phys(virt),
+		.dev_addr       = addr,
+		.size           = size,
+		.direction      = DMA_BIDIRECTIONAL,
+	};
+
+	if (unlikely(global_disable))
+		return;
+
+	check_unmap(&ref);
+}
+EXPORT_SYMBOL(debug_dma_free_coherent);
+
-- 
1.5.6.4

[PATCH 12/18] dma-debug: add checks for sync_single_*

[Joerg Roedel]

Impact: add debug callbacks for dma_sync_single_for_* functions

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   20 ++++++++++++++++++++
 lib/dma-debug.c           |   21 +++++++++++++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index cb72dfd..0eee7af 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -49,6 +49,14 @@ extern void debug_dma_alloc_coherent(struct device *dev, size_t size,
 extern void debug_dma_free_coherent(struct device *dev, size_t size,
 				    void *virt, dma_addr_t addr);
 
+extern void debug_dma_sync_single_for_cpu(struct device *dev,
+					  dma_addr_t dma_handle, size_t size,
+					  int direction);
+
+extern void debug_dma_sync_single_for_device(struct device *dev,
+					     dma_addr_t dma_handle,
+					     size_t size, int direction);
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
@@ -89,6 +97,18 @@ static inline void debug_dma_free_coherent(struct device *dev, size_t size,
 {
 }
 
+static inline void debug_dma_sync_single_for_cpu(struct device *dev,
+						 dma_addr_t dma_handle,
+						 size_t size, int direction)
+{
+}
+
+static inline void debug_dma_sync_single_for_device(struct device *dev,
+						    dma_addr_t dma_handle,
+						    size_t size, int direction)
+{
+}
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 44af837..714cfb6 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -737,3 +737,24 @@ void debug_dma_free_coherent(struct device *dev, size_t size,
 }
 EXPORT_SYMBOL(debug_dma_free_coherent);
 
+void debug_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle,
+				   size_t size, int direction)
+{
+	if (unlikely(global_disable))
+		return;
+
+	check_sync(dev, dma_handle, size, 0, direction, true);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_for_cpu);
+
+void debug_dma_sync_single_for_device(struct device *dev,
+				      dma_addr_t dma_handle, size_t size,
+				      int direction)
+{
+	if (unlikely(global_disable))
+		return;
+
+	check_sync(dev, dma_handle, size, 0, direction, false);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_for_device);
+
-- 
1.5.6.4

[PATCH 13/18] dma-debug: add checks for sync_single_range_*

[Joerg Roedel]

Impact: add debug callbacks for dma_sync_single_range_for_* functions

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   27 +++++++++++++++++++++++++++
 lib/dma-debug.c           |   24 ++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 0eee7af..e9b9035 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -57,6 +57,17 @@ extern void debug_dma_sync_single_for_device(struct device *dev,
 					     dma_addr_t dma_handle,
 					     size_t size, int direction);
 
+extern void debug_dma_sync_single_range_for_cpu(struct device *dev,
+						dma_addr_t dma_handle,
+						unsigned long offset,
+						size_t size,
+						int direction);
+
+extern void debug_dma_sync_single_range_for_device(struct device *dev,
+						   dma_addr_t dma_handle,
+						   unsigned long offset,
+						   size_t size, int direction);
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
@@ -109,6 +120,22 @@ static inline void debug_dma_sync_single_for_device(struct device *dev,
 {
 }
 
+static inline void debug_dma_sync_single_range_for_cpu(struct device *dev,
+						       dma_addr_t dma_handle,
+						       unsigned long offset,
+						       size_t size,
+						       int direction)
+{
+}
+
+static inline void debug_dma_sync_single_range_for_device(struct device *dev,
+							  dma_addr_t dma_handle,
+							  unsigned long offset,
+							  size_t size,
+							  int direction)
+{
+}
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 714cfb6..d1c0ac1 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -758,3 +758,27 @@ void debug_dma_sync_single_for_device(struct device *dev,
 }
 EXPORT_SYMBOL(debug_dma_sync_single_for_device);
 
+void debug_dma_sync_single_range_for_cpu(struct device *dev,
+					 dma_addr_t dma_handle,
+					 unsigned long offset, size_t size,
+					 int direction)
+{
+	if (unlikely(global_disable))
+		return;
+
+	check_sync(dev, dma_handle, size, offset, direction, true);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_range_for_cpu);
+
+void debug_dma_sync_single_range_for_device(struct device *dev,
+					    dma_addr_t dma_handle,
+					    unsigned long offset,
+					    size_t size, int direction)
+{
+	if (unlikely(global_disable))
+		return;
+
+	check_sync(dev, dma_handle, size, offset, direction, false);
+}
+EXPORT_SYMBOL(debug_dma_sync_single_range_for_device);
+
-- 
1.5.6.4

[PATCH 14/18] dma-debug: add checks for sync_single_sg_*

[Joerg Roedel]

Impact: add debug callbacks for dma_sync_sg_* functions

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 include/linux/dma-debug.h |   20 ++++++++++++++++++++
 lib/dma-debug.c           |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index e9b9035..4985c6c 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -68,6 +68,14 @@ extern void debug_dma_sync_single_range_for_device(struct device *dev,
 						   unsigned long offset,
 						   size_t size, int direction);
 
+extern void debug_dma_sync_sg_for_cpu(struct device *dev,
+				      struct scatterlist *sg,
+				      int nelems, int direction);
+
+extern void debug_dma_sync_sg_for_device(struct device *dev,
+					 struct scatterlist *sg,
+					 int nelems, int direction);
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
@@ -136,6 +144,18 @@ static inline void debug_dma_sync_single_range_for_device(struct device *dev,
 {
 }
 
+static inline void debug_dma_sync_sg_for_cpu(struct device *dev,
+					     struct scatterlist *sg,
+					     int nelems, int direction)
+{
+}
+
+static inline void debug_dma_sync_sg_for_device(struct device *dev,
+						struct scatterlist *sg,
+						int nelems, int direction)
+{
+}
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index d1c0ac1..9d11e89 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -782,3 +782,35 @@ void debug_dma_sync_single_range_for_device(struct device *dev,
 }
 EXPORT_SYMBOL(debug_dma_sync_single_range_for_device);
 
+void debug_dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
+			       int nelems, int direction)
+{
+	struct scatterlist *s;
+	int i;
+
+	if (unlikely(global_disable))
+		return;
+
+	for_each_sg(sg, s, nelems, i) {
+		check_sync(dev, s->dma_address, s->dma_length, 0,
+				direction, true);
+	}
+}
+EXPORT_SYMBOL(debug_dma_sync_sg_for_cpu);
+
+void debug_dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
+				  int nelems, int direction)
+{
+	struct scatterlist *s;
+	int i;
+
+	if (unlikely(global_disable))
+		return;
+
+	for_each_sg(sg, s, nelems, i) {
+		check_sync(dev, s->dma_address, s->dma_length, 0,
+				direction, false);
+	}
+}
+EXPORT_SYMBOL(debug_dma_sync_sg_for_device);
+
-- 
1.5.6.4

[PATCH 15/18] dma-debug: add function to dump dma mappings

[Joerg Roedel]

From: David Woodhouse <dwmw2@infradead.org>

This adds a function to dump the DMA mappings that the debugging code is
aware of -- either for a single device, or for _all_ devices.

This can be useful for debugging -- sticking a call to it in the DMA
page fault handler, for example, to see if the faulting address _should_
be mapped or not, and hence work out whether it's IOMMU bugs we're
seeing, or driver bugs.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 include/linux/dma-debug.h |    6 ++++++
 lib/dma-debug.c           |   30 ++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 4985c6c..46a11c1 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -76,6 +76,8 @@ extern void debug_dma_sync_sg_for_device(struct device *dev,
 					 struct scatterlist *sg,
 					 int nelems, int direction);
 
+extern void debug_dma_dump_mappings(struct device *dev);
+
 #else /* CONFIG_DMA_API_DEBUG */
 
 static inline void dma_debug_init(u32 num_entries)
@@ -156,6 +158,10 @@ static inline void debug_dma_sync_sg_for_device(struct device *dev,
 {
 }
 
+static inline void debug_dma_dump_mappings(struct device *dev)
+{
+}
+
 #endif /* CONFIG_DMA_API_DEBUG */
 
 #endif /* __DMA_DEBUG_H */
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 9d11e89..1b6b650 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -194,6 +194,36 @@ static void hash_bucket_del(struct dma_debug_entry *entry)
 }
 
 /*
+ * Dump mapping entries for debugging purposes
+ */
+void debug_dma_dump_mappings(struct device *dev)
+{
+	int idx;
+
+	for (idx = 0; idx < HASH_SIZE; idx++) {
+		struct hash_bucket *bucket = &dma_entry_hash[idx];
+		struct dma_debug_entry *entry;
+		unsigned long flags;
+
+		spin_lock_irqsave(&bucket->lock, flags);
+
+		list_for_each_entry(entry, &bucket->list, list) {
+			if (!dev || dev == entry->dev) {
+				dev_info(entry->dev,
+					 "%s idx %d P=%Lx D=%Lx L=%Lx %s\n",
+					 type2name[entry->type], idx,
+					 (unsigned long long)entry->paddr,
+					 entry->dev_addr, entry->size,
+					 dir2name[entry->direction]);
+			}
+		}
+
+			spin_unlock_irqrestore(&bucket->lock, flags);
+	}
+}
+EXPORT_SYMBOL(debug_dma_dump_mappings);
+
+/*
  * Wrapper function for adding an entry to the hash.
  * This function takes care of locking itself.
  */
-- 
1.5.6.4

[PATCH 16/18] dma-debug: x86 architecture bindings

[Joerg Roedel]

Impact: make use of DMA-API debugging code in x86

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/Kconfig                   |    1 +
 arch/x86/include/asm/dma-mapping.h |   45 +++++++++++++++++++++++++++++++----
 arch/x86/kernel/pci-dma.c          |    6 ++++
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index bc2fbad..f2cb677 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -40,6 +40,7 @@ config X86
 	select HAVE_GENERIC_DMA_COHERENT if X86_32
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select USER_STACKTRACE_SUPPORT
+	select HAVE_DMA_API_DEBUG
 
 config ARCH_DEFCONFIG
 	string
diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
index 9c78bd4..cea7b74 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -7,6 +7,7 @@
  */
 
 #include <linux/scatterlist.h>
+#include <linux/dma-debug.h>
 #include <linux/dma-attrs.h>
 #include <asm/io.h>
 #include <asm/swiotlb.h>
@@ -56,11 +57,16 @@ dma_map_single(struct device *hwdev, void *ptr, size_t size,
 	       enum dma_data_direction dir)
 {
 	struct dma_map_ops *ops = get_dma_ops(hwdev);
+	dma_addr_t addr;
 
 	BUG_ON(!valid_dma_direction(dir));
-	return ops->map_page(hwdev, virt_to_page(ptr),
+	addr = ops->map_page(hwdev, virt_to_page(ptr),
 			     (unsigned long)ptr & ~PAGE_MASK, size,
 			     dir, NULL);
+	debug_dma_map_page(hwdev, virt_to_page(ptr),
+			   (unsigned long)ptr & ~PAGE_MASK, size,
+			   dir, addr, true);
+	return addr;
 }
 
 static inline void
@@ -72,6 +78,7 @@ dma_unmap_single(struct device *dev, dma_addr_t addr, size_t size,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->unmap_page)
 		ops->unmap_page(dev, addr, size, dir, NULL);
+	debug_dma_unmap_page(dev, addr, size, dir, true);
 }
 
 static inline int
@@ -79,9 +86,13 @@ dma_map_sg(struct device *hwdev, struct scatterlist *sg,
 	   int nents, enum dma_data_direction dir)
 {
 	struct dma_map_ops *ops = get_dma_ops(hwdev);
+	int ents;
 
 	BUG_ON(!valid_dma_direction(dir));
-	return ops->map_sg(hwdev, sg, nents, dir, NULL);
+	ents = ops->map_sg(hwdev, sg, nents, dir, NULL);
+	debug_dma_map_sg(hwdev, sg, nents, ents, dir);
+
+	return ents;
 }
 
 static inline void
@@ -91,6 +102,7 @@ dma_unmap_sg(struct device *hwdev, struct scatterlist *sg, int nents,
 	struct dma_map_ops *ops = get_dma_ops(hwdev);
 
 	BUG_ON(!valid_dma_direction(dir));
+	debug_dma_unmap_sg(hwdev, sg, nents, dir);
 	if (ops->unmap_sg)
 		ops->unmap_sg(hwdev, sg, nents, dir, NULL);
 }
@@ -104,6 +116,7 @@ dma_sync_single_for_cpu(struct device *hwdev, dma_addr_t dma_handle,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_single_for_cpu)
 		ops->sync_single_for_cpu(hwdev, dma_handle, size, dir);
+	debug_dma_sync_single_for_cpu(hwdev, dma_handle, size, dir);
 	flush_write_buffers();
 }
 
@@ -116,6 +129,7 @@ dma_sync_single_for_device(struct device *hwdev, dma_addr_t dma_handle,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_single_for_device)
 		ops->sync_single_for_device(hwdev, dma_handle, size, dir);
+	debug_dma_sync_single_for_device(hwdev, dma_handle, size, dir);
 	flush_write_buffers();
 }
 
@@ -130,6 +144,8 @@ dma_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dma_handle,
 	if (ops->sync_single_range_for_cpu)
 		ops->sync_single_range_for_cpu(hwdev, dma_handle, offset,
 					       size, dir);
+	debug_dma_sync_single_range_for_cpu(hwdev, dma_handle,
+					    offset, size, dir);
 	flush_write_buffers();
 }
 
@@ -144,6 +160,8 @@ dma_sync_single_range_for_device(struct device *hwdev, dma_addr_t dma_handle,
 	if (ops->sync_single_range_for_device)
 		ops->sync_single_range_for_device(hwdev, dma_handle,
 						  offset, size, dir);
+	debug_dma_sync_single_range_for_device(hwdev, dma_handle,
+					       offset, size, dir);
 	flush_write_buffers();
 }
 
@@ -156,6 +174,7 @@ dma_sync_sg_for_cpu(struct device *hwdev, struct scatterlist *sg,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_sg_for_cpu)
 		ops->sync_sg_for_cpu(hwdev, sg, nelems, dir);
+	debug_dma_sync_sg_for_cpu(hwdev, sg, nelems, dir);
 	flush_write_buffers();
 }
 
@@ -168,6 +187,7 @@ dma_sync_sg_for_device(struct device *hwdev, struct scatterlist *sg,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_sg_for_device)
 		ops->sync_sg_for_device(hwdev, sg, nelems, dir);
+	debug_dma_sync_sg_for_device(hwdev, sg, nelems, dir);
 
 	flush_write_buffers();
 }
@@ -177,15 +197,24 @@ static inline dma_addr_t dma_map_page(struct device *dev, struct page *page,
 				      enum dma_data_direction dir)
 {
 	struct dma_map_ops *ops = get_dma_ops(dev);
+	dma_addr_t addr;
 
 	BUG_ON(!valid_dma_direction(dir));
-	return ops->map_page(dev, page, offset, size, dir, NULL);
+	addr = ops->map_page(dev, page, offset, size, dir, NULL);
+	debug_dma_map_page(dev, page, offset, size, dir, addr, false);
+
+	return addr;
 }
 
 static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
 				  size_t size, enum dma_data_direction dir)
 {
-	dma_unmap_single(dev, addr, size, dir);
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->unmap_page)
+		ops->unmap_page(dev, addr, size, dir, NULL);
+	debug_dma_unmap_page(dev, addr, size, dir, false);
 }
 
 static inline void
@@ -250,8 +279,11 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 	if (!ops->alloc_coherent)
 		return NULL;
 
-	return ops->alloc_coherent(dev, size, dma_handle,
-				   dma_alloc_coherent_gfp_flags(dev, gfp));
+	memory = ops->alloc_coherent(dev, size, dma_handle,
+				     dma_alloc_coherent_gfp_flags(dev, gfp));
+	debug_dma_alloc_coherent(dev, size, *dma_handle, memory);
+
+	return memory;
 }
 
 static inline void dma_free_coherent(struct device *dev, size_t size,
@@ -264,6 +296,7 @@ static inline void dma_free_coherent(struct device *dev, size_t size,
 	if (dma_release_from_coherent(dev, get_order(size), vaddr))
 		return;
 
+	debug_dma_free_coherent(dev, size, vaddr, bus);
 	if (ops->free_coherent)
 		ops->free_coherent(dev, size, vaddr, bus);
 }
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f293a8d..ebf7d45 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -1,4 +1,5 @@
 #include <linux/dma-mapping.h>
+#include <linux/dma-debug.h>
 #include <linux/dmar.h>
 #include <linux/bootmem.h>
 #include <linux/pci.h>
@@ -44,6 +45,9 @@ struct device x86_dma_fallback_dev = {
 };
 EXPORT_SYMBOL(x86_dma_fallback_dev);
 
+/* Number of entries preallocated for DMA-API debugging */
+#define PREALLOC_DMA_DEBUG_ENTRIES       32768
+
 int dma_set_mask(struct device *dev, u64 mask)
 {
 	if (!dev->dma_mask || !dma_supported(dev, mask))
@@ -265,6 +269,8 @@ EXPORT_SYMBOL(dma_supported);
 
 static int __init pci_iommu_init(void)
 {
+	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
+
 	calgary_iommu_init();
 
 	intel_iommu_init();
-- 
1.5.6.4

[PATCH 17/18] dma-debug: Documentation update

[Joerg Roedel]

Impact: add documentation about DMA-API debugging to DMA-API.txt

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 Documentation/DMA-API.txt |  106 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 2a3fcc5..d9aa43d 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -609,3 +609,109 @@ size is the size (and should be a page-sized multiple).
 The return value will be either a pointer to the processor virtual
 address of the memory, or an error (via PTR_ERR()) if any part of the
 region is occupied.
+
+Part III - Debug drivers use of the DMA-API
+-------------------------------------------
+
+The DMA-API as described above as some constraints. DMA addresses must be
+released with the corresponding function with the same size for example. With
+the advent of hardware IOMMUs it becomes more and more important that drivers
+do not violate those constraints. In the worst case such a violation can
+result in data corruption up to destroyed filesystems.
+
+To debug drivers and find bugs in the usage of the DMA-API checking code can
+be compiled into the kernel which will tell the developer about those
+violations. If your architecture supports it you can select the "Enable
+debugging of DMA-API usage" option in your kernel configuration. Enabling this
+option has a performance impact. Do not enable it in production kernels.
+
+If you boot the resulting kernel will contain code which does some bookkeeping
+about what DMA memory was allocated for which device. If this code detects an
+error it prints a warning message with some details into your kernel log. An
+example warning message may look like this:
+
+------------[ cut here ]------------
+WARNING: at /data2/repos/linux-2.6-iommu/lib/dma-debug.c:448
+	check_unmap+0x203/0x490()
+Hardware name:
+forcedeth 0000:00:08.0: DMA-API: device driver frees DMA memory with wrong
+	function [device address=0x00000000640444be] [size=66 bytes] [mapped as
+single] [unmapped as page]
+Modules linked in: nfsd exportfs bridge stp llc r8169
+Pid: 0, comm: swapper Tainted: G        W  2.6.28-dmatest-09289-g8bb99c0 #1
+Call Trace:
+ <IRQ>  [<ffffffff80240b22>] warn_slowpath+0xf2/0x130
+ [<ffffffff80647b70>] _spin_unlock+0x10/0x30
+ [<ffffffff80537e75>] usb_hcd_link_urb_to_ep+0x75/0xc0
+ [<ffffffff80647c22>] _spin_unlock_irqrestore+0x12/0x40
+ [<ffffffff8055347f>] ohci_urb_enqueue+0x19f/0x7c0
+ [<ffffffff80252f96>] queue_work+0x56/0x60
+ [<ffffffff80237e10>] enqueue_task_fair+0x20/0x50
+ [<ffffffff80539279>] usb_hcd_submit_urb+0x379/0xbc0
+ [<ffffffff803b78c3>] cpumask_next_and+0x23/0x40
+ [<ffffffff80235177>] find_busiest_group+0x207/0x8a0
+ [<ffffffff8064784f>] _spin_lock_irqsave+0x1f/0x50
+ [<ffffffff803c7ea3>] check_unmap+0x203/0x490
+ [<ffffffff803c8259>] debug_dma_unmap_page+0x49/0x50
+ [<ffffffff80485f26>] nv_tx_done_optimized+0xc6/0x2c0
+ [<ffffffff80486c13>] nv_nic_irq_optimized+0x73/0x2b0
+ [<ffffffff8026df84>] handle_IRQ_event+0x34/0x70
+ [<ffffffff8026ffe9>] handle_edge_irq+0xc9/0x150
+ [<ffffffff8020e3ab>] do_IRQ+0xcb/0x1c0
+ [<ffffffff8020c093>] ret_from_intr+0x0/0xa
+ <EOI> <4>---[ end trace f6435a98e2a38c0e ]---
+
+The driver developer can find the driver and the device including a stacktrace
+of the DMA-API call which caused this warning.
+
+Per default only the first error will result in a warning message. All other
+errors will only silently counted. This limitation exist to prevent the code
+from flooding your kernel log. To support debugging a device driver this can
+be disabled via debugfs. See the debugfs interface documentation below for
+details.
+
+The debugfs directory for the DMA-API debugging code is called dma-api/. In
+this directory the following files can currently be found:
+
+	dma-api/all_errors	This file contains a numeric value. If this
+				value is not equal to zero the debugging code
+				will print a warning for every error it finds
+				into the kernel log. Be carefull with this
+				option. It can easily flood your logs.
+
+	dma-api/disabled	This read-only file contains the character 'Y'
+				if the debugging code is disabled. This can
+				happen when it runs out of memory or if it was
+				disabled at boot time
+
+	dma-api/error_count	This file is read-only and shows the total
+				numbers of errors found.
+
+	dma-api/num_errors	The number in this file shows how many
+				warnings will be printed to the kernel log
+				before it stops. This number is initialized to
+				one at system boot and be set by writing into
+				this file
+
+	dma-api/min_free_entries
+				This read-only file can be read to get the
+				minimum number of free dma_debug_entries the
+				allocator has ever seen. If this value goes
+				down to zero the code will disable itself
+				because it is not longer reliable.
+
+	dma-api/num_free_entries
+				The current number of free dma_debug_entries
+				in the allocator.
+
+If you have this code compiled into your kernel it will be enabled by default.
+If you want to boot without the bookkeeping anyway you can provide
+'dma_debug=off' as a boot parameter. This will disable DMA-API debugging.
+Notice that you can not enable it again at runtime. You have to reboot to do
+so.
+
+When the code disables itself at runtime this is most likely because it ran
+out of dma_debug_entries. These entries are preallocated at boot. The number
+of preallocated entries is defined per architecture. If it is too low for you
+boot with 'dma_debug_entries=<your_desired_number>' to overwrite the
+architectural default.
-- 
1.5.6.4

[PATCH 18/18] dma-debug: print stacktrace of mapping path on unmap error

[Joerg Roedel]

From: David Woodhouse <dwmw2@infradead.org>

Impact: saves stacktrace of a dma mapping and prints it if there is an  error

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 lib/dma-debug.c |   52 ++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 1b6b650..128d9de 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -19,6 +19,7 @@
 
 #include <linux/scatterlist.h>
 #include <linux/dma-mapping.h>
+#include <linux/stacktrace.h>
 #include <linux/dma-debug.h>
 #include <linux/spinlock.h>
 #include <linux/debugfs.h>
@@ -39,6 +40,8 @@ enum {
 	dma_debug_coherent,
 };
 
+#define DMA_DEBUG_STACKTRACE_ENTRIES 5
+
 struct dma_debug_entry {
 	struct list_head list;
 	struct device    *dev;
@@ -49,6 +52,10 @@ struct dma_debug_entry {
 	int              direction;
 	int		 sg_call_ents;
 	int		 sg_mapped_ents;
+#ifdef CONFIG_STACKTRACE
+	struct		 stack_trace stacktrace;
+	unsigned long	 st_entries[DMA_DEBUG_STACKTRACE_ENTRIES];
+#endif
 };
 
 struct hash_bucket {
@@ -108,12 +115,23 @@ static const char *dir2name[4] = { "DMA_BIDIRECTIONAL", "DMA_TO_DEVICE",
  *                    system log than the user configured. This variable is
  *                    writeable via debugfs.
  */
-#define err_printk(dev, format, arg...) do {			\
+static inline void dump_entry_trace(struct dma_debug_entry *entry)
+{
+#ifdef CONFIG_STACKTRACE
+	if (entry) {
+		printk(KERN_WARNING "Mapped at:\n");
+		print_stack_trace(&entry->stacktrace, 0);
+	}
+#endif
+}
+
+#define err_printk(dev, entry, format, arg...) do {		\
 		error_count += 1;				\
 		if (show_all_errors || show_num_errors > 0) {	\
 			WARN(1, "%s %s: " format,		\
 			     dev_driver_string(dev),		\
 			     dev_name(dev) , ## arg);		\
+			dump_entry_trace(entry);		\
 		}						\
 		if (!show_all_errors && show_num_errors > 0)	\
 			show_num_errors -= 1;			\
@@ -260,6 +278,12 @@ static struct dma_debug_entry *dma_entry_alloc(void)
 	list_del(&entry->list);
 	memset(entry, 0, sizeof(*entry));
 
+#ifdef CONFIG_STACKTRACE
+	entry->stacktrace.max_entries = DMA_DEBUG_STACKTRACE_ENTRIES;
+	entry->stacktrace.entries = entry->st_entries;
+	entry->stacktrace.skip = 2;
+	save_stack_trace(&entry->stacktrace);
+#endif
 	num_free_entries -= 1;
 	if (num_free_entries < min_free_entries)
 		min_free_entries = num_free_entries;
@@ -457,7 +481,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 	entry = hash_bucket_find(bucket, ref);
 
 	if (!entry) {
-		err_printk(ref->dev, "DMA-API: device driver tries "
+		err_printk(ref->dev, NULL, "DMA-API: device driver tries "
 			   "to free DMA memory it has not allocated "
 			   "[device address=0x%016llx] [size=%llu bytes]\n",
 			   ref->dev_addr, ref->size);
@@ -465,7 +489,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 	}
 
 	if (ref->size != entry->size) {
-		err_printk(ref->dev, "DMA-API: device driver frees "
+		err_printk(ref->dev, entry, "DMA-API: device driver frees "
 			   "DMA memory with different size "
 			   "[device address=0x%016llx] [map size=%llu bytes] "
 			   "[unmap size=%llu bytes]\n",
@@ -473,7 +497,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 	}
 
 	if (ref->type != entry->type) {
-		err_printk(ref->dev, "DMA-API: device driver frees "
+		err_printk(ref->dev, entry, "DMA-API: device driver frees "
 			   "DMA memory with wrong function "
 			   "[device address=0x%016llx] [size=%llu bytes] "
 			   "[mapped as %s] [unmapped as %s]\n",
@@ -481,7 +505,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 			   type2name[entry->type], type2name[ref->type]);
 	} else if ((entry->type == dma_debug_coherent) &&
 		   (ref->paddr != entry->paddr)) {
-		err_printk(ref->dev, "DMA-API: device driver frees "
+		err_printk(ref->dev, entry, "DMA-API: device driver frees "
 			   "DMA memory with different CPU address "
 			   "[device address=0x%016llx] [size=%llu bytes] "
 			   "[cpu alloc address=%p] [cpu free address=%p]",
@@ -491,7 +515,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 
 	if (ref->sg_call_ents && ref->type == dma_debug_sg &&
 	    ref->sg_call_ents != entry->sg_call_ents) {
-		err_printk(ref->dev, "DMA-API: device driver frees "
+		err_printk(ref->dev, entry, "DMA-API: device driver frees "
 			   "DMA sg list with different entry count "
 			   "[map count=%d] [unmap count=%d]\n",
 			   entry->sg_call_ents, ref->sg_call_ents);
@@ -502,7 +526,7 @@ static void check_unmap(struct dma_debug_entry *ref)
 	 * DMA API don't handle this properly, so check for it here
 	 */
 	if (ref->direction != entry->direction) {
-		err_printk(ref->dev, "DMA-API: device driver frees "
+		err_printk(ref->dev, entry, "DMA-API: device driver frees "
 			   "DMA memory with different direction "
 			   "[device address=0x%016llx] [size=%llu bytes] "
 			   "[mapped with %s] [unmapped with %s]\n",
@@ -521,8 +545,8 @@ out:
 static void check_for_stack(struct device *dev, void *addr)
 {
 	if (object_is_on_stack(addr))
-		err_printk(dev, "DMA-API: device driver maps memory from stack"
-				" [addr=%p]\n", addr);
+		err_printk(dev, NULL, "DMA-API: device driver maps memory from"
+				"stack [addr=%p]\n", addr);
 }
 
 static void check_sync(struct device *dev, dma_addr_t addr,
@@ -543,7 +567,7 @@ static void check_sync(struct device *dev, dma_addr_t addr,
 	entry = hash_bucket_find(bucket, &ref);
 
 	if (!entry) {
-		err_printk(dev, "DMA-API: device driver tries "
+		err_printk(dev, NULL, "DMA-API: device driver tries "
 				"to sync DMA memory it has not allocated "
 				"[device address=0x%016llx] [size=%llu bytes]\n",
 				addr, size);
@@ -551,7 +575,7 @@ static void check_sync(struct device *dev, dma_addr_t addr,
 	}
 
 	if ((offset + size) > entry->size) {
-		err_printk(dev, "DMA-API: device driver syncs"
+		err_printk(dev, entry, "DMA-API: device driver syncs"
 				" DMA memory outside allocated range "
 				"[device address=0x%016llx] "
 				"[allocation size=%llu bytes] [sync offset=%llu] "
@@ -560,7 +584,7 @@ static void check_sync(struct device *dev, dma_addr_t addr,
 	}
 
 	if (direction != entry->direction) {
-		err_printk(dev, "DMA-API: device driver syncs "
+		err_printk(dev, entry, "DMA-API: device driver syncs "
 				"DMA memory with different direction "
 				"[device address=0x%016llx] [size=%llu bytes] "
 				"[mapped with %s] [synced with %s]\n",
@@ -574,7 +598,7 @@ static void check_sync(struct device *dev, dma_addr_t addr,
 
 	if (to_cpu && !(entry->direction == DMA_FROM_DEVICE) &&
 		      !(direction == DMA_TO_DEVICE))
-		err_printk(dev, "DMA-API: device driver syncs "
+		err_printk(dev, entry, "DMA-API: device driver syncs "
 				"device read-only DMA memory for cpu "
 				"[device address=0x%016llx] [size=%llu bytes] "
 				"[mapped with %s] [synced with %s]\n",
@@ -584,7 +608,7 @@ static void check_sync(struct device *dev, dma_addr_t addr,
 
 	if (!to_cpu && !(entry->direction == DMA_TO_DEVICE) &&
 		       !(direction == DMA_FROM_DEVICE))
-		err_printk(dev, "DMA-API: device driver syncs "
+		err_printk(dev, entry, "DMA-API: device driver syncs "
 				"device write-only DMA memory to device "
 				"[device address=0x%016llx] [size=%llu bytes] "
 				"[mapped with %s] [synced with %s]\n",
-- 
1.5.6.4

Re: [PATCH 07/18] dma-debug: add debugfs interface

[Frederic Weisbecker]

On Fri, Mar 06, 2009 at 02:30:18PM +0100, Joerg Roedel wrote:
> Impact: add debugfs interface for configuring DMA-API debugging
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  lib/dma-debug.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 78 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index 2ede463..20d6cdb 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -19,6 +19,7 @@
>  
>  #include <linux/dma-debug.h>
>  #include <linux/spinlock.h>
> +#include <linux/debugfs.h>
>  #include <linux/types.h>
>  #include <linux/list.h>
>  #include <linux/slab.h>
> @@ -61,12 +62,29 @@ static DEFINE_SPINLOCK(free_entries_lock);
>  /* Global disable flag - will be set in case of an error */
>  static bool global_disable __read_mostly;
>  
> +/* Global error count */
> +static u32 error_count;
> +
> +/* Global error show enable*/
> +static u32 show_all_errors __read_mostly;
> +/* Number of errors to show */
> +static u32 show_num_errors = 1;
> +
>  static u32 num_free_entries;
>  static u32 min_free_entries;
>  
>  /* number of preallocated entries requested by kernel cmdline */
>  static u32 req_entries;
>  
> +/* debugfs dentry's for the stuff above */
> +static struct dentry *dma_debug_dent        __read_mostly;
> +static struct dentry *global_disable_dent   __read_mostly;
> +static struct dentry *error_count_dent      __read_mostly;
> +static struct dentry *show_all_errors_dent  __read_mostly;
> +static struct dentry *show_num_errors_dent  __read_mostly;
> +static struct dentry *num_free_entries_dent __read_mostly;
> +static struct dentry *min_free_entries_dent __read_mostly;
> +
>  /*
>   * Hash related functions
>   *
> @@ -241,6 +259,58 @@ out_err:
>  	return -ENOMEM;
>  }
>  
> +static int dma_debug_fs_init(void)
> +{
> +	dma_debug_dent = debugfs_create_dir("dma-api", NULL);
> +	if (!dma_debug_dent) {
> +		printk(KERN_ERR "DMA-API: can not create debugfs directory\n");
> +		return -ENOMEM;
> +	}
> +
> +	global_disable_dent = debugfs_create_bool("disabled", 0444,
> +			dma_debug_dent,
> +			(u32 *)&global_disable);
> +	if (!global_disable_dent)
> +		goto out_err;
> +
> +	error_count_dent = debugfs_create_u32("error_count", 0444,
> +			dma_debug_dent, &error_count);
> +	if (!error_count_dent)
> +		goto out_err;
> +
> +	show_all_errors_dent = debugfs_create_u32("all_errors", 0644,
> +			dma_debug_dent,
> +			&show_all_errors);
> +	if (!show_all_errors_dent)
> +		goto out_err;
> +
> +	show_num_errors_dent = debugfs_create_u32("num_errors", 0644,
> +			dma_debug_dent,
> +			&show_num_errors);
> +	if (!show_num_errors_dent)
> +		goto out_err;
> +
> +	num_free_entries_dent = debugfs_create_u32("num_free_entries", 0444,
> +			dma_debug_dent,
> +			&num_free_entries);
> +	if (!num_free_entries_dent)
> +		goto out_err;
> +
> +	min_free_entries_dent = debugfs_create_u32("min_free_entries", 0444,
> +			dma_debug_dent,
> +			&min_free_entries);
> +	if (!min_free_entries_dent)
> +		goto out_err;


Hi,

At least for the read-only files, why not create a single file
which displays num_free_entries, min_free_entries and global disable?


> +
> +	return 0;
> +
> +out_err:
> +	debugfs_remove_recursive(dma_debug_dent);
> +
> +	return -ENOMEM;
> +}
> +
> +
>  /*
>   * Let the architectures decide how many entries should be preallocated.
>   */
> @@ -256,6 +326,14 @@ void dma_debug_init(u32 num_entries)
>  		dma_entry_hash[i].lock = SPIN_LOCK_UNLOCKED;
>  	}
>  
> +	if (dma_debug_fs_init() != 0) {
> +		printk(KERN_ERR "DMA-API: error creating debugfs entries "
> +				"- disabling\n");
> +		global_disable = true;
> +
> +		return;
> +	}
> +
>  	if (req_entries)
>  		num_entries = req_entries;
>  
> -- 
> 1.5.6.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Frederic Weisbecker]

On Fri, Mar 06, 2009 at 02:30:14PM +0100, Joerg Roedel wrote:
> Impact: implement necessary functions for the core hash
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  lib/dma-debug.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 101 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index 3109971..5ff7d2e 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -18,9 +18,14 @@
>   */
>  
>  #include <linux/dma-debug.h>
> +#include <linux/spinlock.h>
>  #include <linux/types.h>
>  #include <linux/list.h>
>  
> +#define HASH_SIZE       1024ULL
> +#define HASH_FN_SHIFT   13
> +#define HASH_FN_MASK    (HASH_SIZE - 1)
> +
>  enum {
>  	dma_debug_single,
>  	dma_debug_page,
> @@ -40,3 +45,99 @@ struct dma_debug_entry {
>  	int		 sg_mapped_ents;
>  };
>  
> +struct hash_bucket {
> +	struct list_head list;
> +	spinlock_t lock;
> +} __cacheline_aligned_in_smp;
> +
> +/* Hash list to save the allocated dma addresses */
> +static struct hash_bucket dma_entry_hash[HASH_SIZE];
> +
> +/*
> + * Hash related functions
> + *
> + * Every DMA-API request is saved into a struct dma_debug_entry. To
> + * have quick access to these structs they are stored into a hash.
> + */
> +static int hash_fn(struct dma_debug_entry *entry)
> +{
> +	/*
> +	 * Hash function is based on the dma address.
> +	 * We use bits 20-27 here as the index into the hash
> +	 */
> +	return (entry->dev_addr >> HASH_FN_SHIFT) & HASH_FN_MASK;
> +}
> +
> +/*
> + * Request exclusive access to a hash bucket for a given dma_debug_entry.
> + */
> +static struct hash_bucket *get_hash_bucket(struct dma_debug_entry *entry,
> +					   unsigned long *flags)
> +{
> +	int idx = hash_fn(entry);
> +	unsigned long __flags;
> +
> +	spin_lock_irqsave(&dma_entry_hash[idx].lock, __flags);
> +	*flags = __flags;
> +	return &dma_entry_hash[idx];
> +}
> +
> +/*
> + * Give up exclusive access to the hash bucket
> + */
> +static void put_hash_bucket(struct hash_bucket *bucket,
> +			    unsigned long *flags)
> +{
> +	unsigned long __flags = *flags;
> +
> +	spin_unlock_irqrestore(&bucket->lock, __flags);
> +}
> +
> +/*
> + * Search a given entry in the hash bucket list
> + */
> +static struct dma_debug_entry *hash_bucket_find(struct hash_bucket *bucket,
> +						struct dma_debug_entry *ref)
> +{
> +	struct dma_debug_entry *entry;
> +
> +	list_for_each_entry(entry, &bucket->list, list) {
> +		if ((entry->dev_addr == ref->dev_addr) &&
> +		    (entry->dev == ref->dev))
> +			return entry;
> +	}
> +
> +	return NULL;
> +}
> +
> +/*
> + * Add an entry to a hash bucket
> + */
> +static void hash_bucket_add(struct hash_bucket *bucket,
> +			    struct dma_debug_entry *entry)
> +{
> +	list_add_tail(&entry->list, &bucket->list);
> +}
> +
> +/*
> + * Remove entry from a hash bucket list
> + */
> +static void hash_bucket_del(struct dma_debug_entry *entry)
> +{
> +	list_del(&entry->list);
> +}


Perhaps the two wrappers above are unnecessary, since they are actually
used once and only wrap a single list operation. No?

Frederic.

> +/*
> + * Wrapper function for adding an entry to the hash.
> + * This function takes care of locking itself.
> + */
> +static void add_dma_entry(struct dma_debug_entry *entry)
> +{
> +	struct hash_bucket *bucket;
> +	unsigned long flags;
> +
> +	bucket = get_hash_bucket(entry, &flags);
> +	hash_bucket_add(bucket, entry);
> +	put_hash_bucket(bucket, &flags);
> +}
> +
> -- 
> 1.5.6.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Cyrill Gorcunov]

[Frederic Weisbecker - Fri, Mar 06, 2009 at 02:50:52PM +0100]
...
| > +/*
| > + * Add an entry to a hash bucket
| > + */
| > +static void hash_bucket_add(struct hash_bucket *bucket,
| > +			    struct dma_debug_entry *entry)
| > +{
| > +	list_add_tail(&entry->list, &bucket->list);
| > +}
| > +
| > +/*
| > + * Remove entry from a hash bucket list
| > + */
| > +static void hash_bucket_del(struct dma_debug_entry *entry)
| > +{
| > +	list_del(&entry->list);
| > +}
| 
| 
| Perhaps the two wrappers above are unnecessary, since they are actually
| used once and only wrap a single list operation. No?
| 
| Frederic.

Hi Frederic,

I think it would be better to make them 'inline' only but remain
the wrappers as is, since it show logic flow and hides internal data
details. But it's my personal opinion.

| 
...

	- Cyrill -

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Frederic Weisbecker]

On Fri, Mar 06, 2009 at 09:45:14PM +0300, Cyrill Gorcunov wrote:
> [Frederic Weisbecker - Fri, Mar 06, 2009 at 02:50:52PM +0100]
> ...
> | > +/*
> | > + * Add an entry to a hash bucket
> | > + */
> | > +static void hash_bucket_add(struct hash_bucket *bucket,
> | > +			    struct dma_debug_entry *entry)
> | > +{
> | > +	list_add_tail(&entry->list, &bucket->list);
> | > +}
> | > +
> | > +/*
> | > + * Remove entry from a hash bucket list
> | > + */
> | > +static void hash_bucket_del(struct dma_debug_entry *entry)
> | > +{
> | > +	list_del(&entry->list);
> | > +}
> | 
> | 
> | Perhaps the two wrappers above are unnecessary, since they are actually
> | used once and only wrap a single list operation. No?
> | 
> | Frederic.
> 
> Hi Frederic,
> 
> I think it would be better to make them 'inline' only but remain
> the wrappers as is, since it show logic flow and hides internal data
> details. But it's my personal opinion.


Yeah, I guess it's only a matter of taste :-)
Anyway, as you said, it should be inlined.

 
> | 
> ...
> 
> 	- Cyrill -

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Cyrill Gorcunov]

[Frederic Weisbecker - Fri, Mar 06, 2009 at 08:11:00PM +0100]
| On Fri, Mar 06, 2009 at 09:45:14PM +0300, Cyrill Gorcunov wrote:
| > [Frederic Weisbecker - Fri, Mar 06, 2009 at 02:50:52PM +0100]
| > ...
| > | > +/*
| > | > + * Add an entry to a hash bucket
| > | > + */
| > | > +static void hash_bucket_add(struct hash_bucket *bucket,
| > | > +			    struct dma_debug_entry *entry)
| > | > +{
| > | > +	list_add_tail(&entry->list, &bucket->list);
| > | > +}
| > | > +
| > | > +/*
| > | > + * Remove entry from a hash bucket list
| > | > + */
| > | > +static void hash_bucket_del(struct dma_debug_entry *entry)
| > | > +{
| > | > +	list_del(&entry->list);
| > | > +}
| > | 
| > | 
| > | Perhaps the two wrappers above are unnecessary, since they are actually
| > | used once and only wrap a single list operation. No?
| > | 
| > | Frederic.
| > 
| > Hi Frederic,
| > 
| > I think it would be better to make them 'inline' only but remain
| > the wrappers as is, since it show logic flow and hides internal data
| > details. But it's my personal opinion.
| 
| 
| Yeah, I guess it's only a matter of taste :-)
| Anyway, as you said, it should be inlined.

Nod :) The only problem could be (it depends) -- is that
if one day some locking would be needed instead of fixing
one function you would need to grep all list_add/del entries :)

| 
|  
	- Cyrill -

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Frederic Weisbecker]

On Fri, Mar 06, 2009 at 10:16:41PM +0300, Cyrill Gorcunov wrote:
> [Frederic Weisbecker - Fri, Mar 06, 2009 at 08:11:00PM +0100]
> | On Fri, Mar 06, 2009 at 09:45:14PM +0300, Cyrill Gorcunov wrote:
> | > [Frederic Weisbecker - Fri, Mar 06, 2009 at 02:50:52PM +0100]
> | > ...
> | > | > +/*
> | > | > + * Add an entry to a hash bucket
> | > | > + */
> | > | > +static void hash_bucket_add(struct hash_bucket *bucket,
> | > | > +			    struct dma_debug_entry *entry)
> | > | > +{
> | > | > +	list_add_tail(&entry->list, &bucket->list);
> | > | > +}
> | > | > +
> | > | > +/*
> | > | > + * Remove entry from a hash bucket list
> | > | > + */
> | > | > +static void hash_bucket_del(struct dma_debug_entry *entry)
> | > | > +{
> | > | > +	list_del(&entry->list);
> | > | > +}
> | > | 
> | > | 
> | > | Perhaps the two wrappers above are unnecessary, since they are actually
> | > | used once and only wrap a single list operation. No?
> | > | 
> | > | Frederic.
> | > 
> | > Hi Frederic,
> | > 
> | > I think it would be better to make them 'inline' only but remain
> | > the wrappers as is, since it show logic flow and hides internal data
> | > details. But it's my personal opinion.
> | 
> | 
> | Yeah, I guess it's only a matter of taste :-)
> | Anyway, as you said, it should be inlined.
> 
> Nod :) The only problem could be (it depends) -- is that
> if one day some locking would be needed instead of fixing
> one function you would need to grep all list_add/del entries :)


Yes, although the locking is already here but I understand your point.

 
> | 
> |  
> 	- Cyrill -

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Joerg Roedel]

On Fri, Mar 06, 2009 at 10:16:41PM +0300, Cyrill Gorcunov wrote:
> [Frederic Weisbecker - Fri, Mar 06, 2009 at 08:11:00PM +0100]
> | On Fri, Mar 06, 2009 at 09:45:14PM +0300, Cyrill Gorcunov wrote:
> | > [Frederic Weisbecker - Fri, Mar 06, 2009 at 02:50:52PM +0100]
> | > ...
> | > | > +/*
> | > | > + * Add an entry to a hash bucket
> | > | > + */
> | > | > +static void hash_bucket_add(struct hash_bucket *bucket,
> | > | > +			    struct dma_debug_entry *entry)
> | > | > +{
> | > | > +	list_add_tail(&entry->list, &bucket->list);
> | > | > +}
> | > | > +
> | > | > +/*
> | > | > + * Remove entry from a hash bucket list
> | > | > + */
> | > | > +static void hash_bucket_del(struct dma_debug_entry *entry)
> | > | > +{
> | > | > +	list_del(&entry->list);
> | > | > +}
> | > | 
> | > | 
> | > | Perhaps the two wrappers above are unnecessary, since they are actually
> | > | used once and only wrap a single list operation. No?
> | > | 
> | > | Frederic.
> | > 
> | > Hi Frederic,
> | > 
> | > I think it would be better to make them 'inline' only but remain
> | > the wrappers as is, since it show logic flow and hides internal data
> | > details. But it's my personal opinion.
> | 
> | 
> | Yeah, I guess it's only a matter of taste :-)
> | Anyway, as you said, it should be inlined.
> 
> Nod :) The only problem could be (it depends) -- is that
> if one day some locking would be needed instead of fixing
> one function you would need to grep all list_add/del entries :)

The access is already locked. And as the functions are only called
once each gcc should inline them automatically. At least gcc inlined
them in my kernels :)

	Joerg

-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Cyrill Gorcunov]

[Joerg Roedel - Fri, Mar 06, 2009 at 08:25:35PM +0100]
...
| > Nod :) The only problem could be (it depends) -- is that
| > if one day some locking would be needed instead of fixing
| > one function you would need to grep all list_add/del entries :)
| 
| The access is already locked. And as the functions are only called
| once each gcc should inline them automatically. At least gcc inlined
| them in my kernels :)
| 
| 	Joerg

I didn't checked the precise code logic neither details, just wanted
to point out that 'wrapping' functions are beneficial sometimes (especially
when they hide details of internal data and provide some kind of interface
to play with). Dunno Joerg, I think it would be better to point out that
we want those functions being inlined by gcc 'inline' attribute explicitly.
But you choose :)

	- Cyrill -

Re: [PATCH 03/18] dma-debug: add hash functions for dma_debug_entries

[Joerg Roedel]

On Fri, Mar 06, 2009 at 10:38:23PM +0300, Cyrill Gorcunov wrote:
> [Joerg Roedel - Fri, Mar 06, 2009 at 08:25:35PM +0100]
> ...
> | > Nod :) The only problem could be (it depends) -- is that
> | > if one day some locking would be needed instead of fixing
> | > one function you would need to grep all list_add/del entries :)
> | 
> | The access is already locked. And as the functions are only called
> | once each gcc should inline them automatically. At least gcc inlined
> | them in my kernels :)
> | 
> | 	Joerg
> 
> I didn't checked the precise code logic neither details, just wanted
> to point out that 'wrapping' functions are beneficial sometimes (especially
> when they hide details of internal data and provide some kind of interface
> to play with).

True. I agree with this. These functions improve the readability of the
code imho.

> Dunno Joerg, I think it would be better to point out that we want
> those functions being inlined by gcc 'inline' attribute explicitly.
> But you choose :)

Yeah, I think its better to let gcc choose what to inline and what not.
It has a good heuristic for that task :)

	Joerg


-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632

Re: [PATCH 09/18] dma-debug: add checking for map/unmap_page/single

[FUJITA Tomonori]

On Fri, 6 Mar 2009 14:30:20 +0100
Joerg Roedel <joerg.roedel@amd.com> wrote:

> Impact: add debug callbacks for dma_{un}map_[page|single]
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  include/linux/dma-debug.h |   23 +++++++++++++++++++
>  lib/dma-debug.c           |   53 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
> index 345d538..65f7352 100644
> --- a/include/linux/dma-debug.h
> +++ b/include/linux/dma-debug.h
> @@ -28,12 +28,35 @@ struct device;
>  
>  extern void dma_debug_init(u32 num_entries);
>  
> +extern void debug_dma_map_page(struct device *dev, struct page *page,
> +			       size_t offset, size_t size,
> +			       int direction, dma_addr_t dma_addr,
> +			       bool map_single);
> +
> +extern void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
> +				 size_t size, int direction, bool map_single);
> +
> +
>  #else /* CONFIG_DMA_API_DEBUG */
>  
>  static inline void dma_debug_init(u32 num_entries)
>  {
>  }
>  
> +static inline void debug_dma_map_page(struct device *dev, struct page *page,
> +				      size_t offset, size_t size,
> +				      int direction, dma_addr_t dma_addr,
> +				      bool map_single)
> +{
> +}
> +
> +static inline void debug_dma_unmap_page(struct device *dev, dma_addr_t addr,
> +					size_t size, int direction,
> +					bool map_single)
> +{
> +}
> +
> +
>  #endif /* CONFIG_DMA_API_DEBUG */
>  
>  #endif /* __DMA_DEBUG_H */
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index d0cb47a..a2ed2b7 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -566,3 +566,56 @@ out:
>  
>  }
>  
> +void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
> +			size_t size, int direction, dma_addr_t dma_addr,
> +			bool map_single)
> +{
> +	struct dma_debug_entry *entry;
> +
> +	if (unlikely(global_disable))
> +		return;
> +
> +	if (unlikely(dma_mapping_error(dev, dma_addr)))
> +		return;
> +
> +	entry = dma_entry_alloc();
> +	if (!entry)
> +		return;
> +
> +	entry->dev       = dev;
> +	entry->type      = dma_debug_page;
> +	entry->paddr     = page_to_phys(page) + offset;
> +	entry->dev_addr  = dma_addr;
> +	entry->size      = size;
> +	entry->direction = direction;
> +
> +	if (map_single) {
> +		entry->type = dma_debug_single;
> +		check_for_stack(dev, page_address(page) + offset);

Why you don't call check_for_stack() for dma_map_page()?

page_address(page) could be invalid with dma_map_page() so the check
can be pointless. However, you call check_for_stack() with dma_map_sg,
which the check can be pointless too with; I think that you call
check_for_stack() in an inconsistent way.

Re: [PATCH 09/18] dma-debug: add checking for map/unmap_page/single

[Joerg Roedel]

On Thu, Mar 19, 2009 at 10:39:30AM +0900, FUJITA Tomonori wrote:
> On Fri, 6 Mar 2009 14:30:20 +0100
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> > +	if (map_single) {
> > +		entry->type = dma_debug_single;
> > +		check_for_stack(dev, page_address(page) + offset);
> 
> Why you don't call check_for_stack() for dma_map_page()?
> 
> page_address(page) could be invalid with dma_map_page() so the check
> can be pointless. However, you call check_for_stack() with dma_map_sg,
> which the check can be pointless too with; I think that you call
> check_for_stack() in an inconsistent way.

I wasn't aware that sg mappings support highmem too. I did the check
only for map_single because I havn't found a way to check if the address
is mapped. Its not so important because the pointer is never
dereferenced but only compared. Looking again at it I think a check for
PageHighMem() should be sufficient to check if the page is mapped. I
will update the code. Thanks for pointing this out.

Joerg