diff mbox

[04/10] x86: add helper functions for consistency checks

Message ID 1227284770-19215-5-git-send-email-joerg.roedel@amd.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Joerg Roedel Nov. 21, 2008, 4:26 p.m. UTC 
Impact: adds helper functions to be used later

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

Comments

Ingo Molnar Nov. 21, 2008, 5:07 p.m. UTC | #1 
* Joerg Roedel <joerg.roedel@amd.com> wrote:

> +static bool check_unmap(struct dma_debug_entry *ref,
> +		struct dma_debug_entry *entry)
> +{
> +	bool errors = false;
> +
> +	if (!entry) {
> +		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: device driver tries "
> +			   "to free DMA memory it has not allocated "
> +			   "[device address=0x%016llx] [size=%llu bytes]\n",
> +			   ref->dev_addr, ref->size);
> +		dump_stack();
> +
> +		return false;

okay, the warnings need to be one-shot. It will be enabled by distros 
in debug kernels to test a wide range of drivers, and the output will 
be collected by kerneloops.org. Distros will disable the debug feature 
fast if it spams the logs.

So please make it WARN_ONCE() type of warnings. Dont use dump_stack() 
directly but use the WARN() signature so that it's picked up by 
automated bug collection mechanisms.

This holds true for all the other warnings as well. Plus probably the 
whole mechanism should self-deactivate like lockdep does, when it 
notices the first error. That guarantees that even if it has a false 
positive or some other bug it wont break more stuff.

> +	struct dma_debug_entry ref = {
> +		.dev = dev,
> +		.dev_addr = addr,
> +		.size = size,
> +		.direction = direction,
> +	};

(align the field init vertically please.)

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kernel/pci-dma-debug.c b/arch/x86/kernel/pci-dma-debug.c
index c2d3408..fc95631 100644
--- a/arch/x86/kernel/pci-dma-debug.c
+++ b/arch/x86/kernel/pci-dma-debug.c
@@ -42,6 +42,11 @@  static struct kmem_cache *dma_entry_cache;
 /* lock to protect the data structures */
 static DEFINE_SPINLOCK(dma_lock);
 
+static char *type2name[3] = { "single", "scather-gather", "coherent" };
+
+static char *dir2name[4] = { "DMA_BIDIRECTIONAL", "DMA_TO_DEVICE",
+			     "DMA_FROM_DEVICE", "DMA_NONE" };
+
 static int hash_fn(struct dma_debug_entry *entry)
 {
 	/*
@@ -95,6 +100,126 @@  static void remove_dma_entry(struct dma_debug_entry *entry)
 	list_del(&entry->list);
 }
 
+static bool check_unmap(struct dma_debug_entry *ref,
+		struct dma_debug_entry *entry)
+{
+	bool errors = false;
+
+	if (!entry) {
+		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: device driver tries "
+			   "to free DMA memory it has not allocated "
+			   "[device address=0x%016llx] [size=%llu bytes]\n",
+			   ref->dev_addr, ref->size);
+		dump_stack();
+
+		return false;
+	}
+
+	if (ref->size != entry->size) {
+		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: 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);
+		errors = true;
+	}
+
+	if (ref->type != entry->type) {
+		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: device driver frees "
+			   "DMA memory different that it was allocated "
+			   "[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]);
+		errors = true;
+	} else if ((entry->type == DMA_DEBUG_COHERENT) &&
+		   (ref->cpu_addr != entry->cpu_addr)) {
+		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: 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,
+			   entry->cpu_addr, ref->cpu_addr);
+		errors = true;
+
+	}
+
+	/*
+	 * 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) {
+		dev_printk(KERN_ERR, ref->dev, "PCI-DMA: 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]);
+		errors = true;
+	}
+
+	if (errors)
+		dump_stack();
+
+	return true;
+}
+
+static void check_sync(struct device *dev, dma_addr_t addr,
+		u64 size, u64 offset, int direction, bool to_cpu)
+{
+	bool error = false;
+	unsigned long flags;
+	struct dma_debug_entry ref = {
+		.dev = dev,
+		.dev_addr = addr,
+		.size = size,
+		.direction = direction,
+	};
+	struct dma_debug_entry *entry;
+
+	spin_lock_irqsave(&dma_lock, flags);
+
+	entry = find_dma_entry(&ref);
+
+	if (!entry) {
+		dev_printk(KERN_ERR, dev, "PCI-DMA: device driver tries "
+			   "to sync DMA memory it has not allocated "
+			   "[device address=0x%016llx] [size=%llu bytes]\n",
+			   addr, size);
+		error = true;
+		goto out;
+	}
+
+	if ((offset + size) > entry->size) {
+		dev_printk(KERN_ERR, dev, "PCI-DMA: 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);
+		error = true;
+	}
+
+	if (direction != entry->direction) {
+		dev_printk(KERN_ERR, dev, "PCI-DMA: 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]);
+		error = true;
+	}
+
+out:
+	spin_unlock_irqrestore(&dma_lock, flags);
+
+	if (error)
+		dump_stack();
+}
+
+
 void dma_debug_init(void)
 {
 	int i;