Re: [PATCH 03/10] x86: add initialization code for DMA-API debugging

Joerg Roedel [off-list ref] writes:

+/* Hash list to save the allocated dma addresses */
+static struct list_head dma_entry_hash[HASH_SIZE];

Hash tables should use hlists.

+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
+	 */
+	BUG_ON(entry->dev_addr == bad_dma_address);
+
+	return (entry->dev_addr >> HASH_FN_SHIFT) & HASH_FN_MASK;

It would be probably safer to use a stronger hash like FNV
There are a couple to reuse in include/

+}
+
+static struct dma_debug_entry *dma_entry_alloc(void)
+{
+	gfp_t gfp = GFP_KERNEL | __GFP_ZERO;
+
+	if (in_atomic())
+		gfp |= GFP_ATOMIC;
+
+	return kmem_cache_alloc(dma_entry_cache, gfp);
+}

While the basic idea is reasonable this function is unfortunately
broken. It's not always safe to allocate memory (e.g. in the block
write out path which uses map_sg). You would need to use
a mempool or something.

Besides the other problem of using GFP_ATOMIC is that it can 
fail under high load and you don't handle this case very well
(would report a bug incorrectly). And stress tests tend to 
trigger that, reporting false positives in such a case is a very very
bad thing, it leads to QA people putting these messages
on their blacklists.

-Andi

-- 
ak@linux.intel.com