Re: [PATCH] dmapool : make DMAPOOL_DEBUG detect corruption of free marker

[Andrew Morton <akpm@linux-foundation.org>]

On Mon,  5 Nov 2012 14:40:05 +0100
Matthieu CASTET <matthieu.castet@parrot.com> wrote:

> This can help to catch case where hardware is writting after dma free.
> 
> ...
>
> --- a/mm/dmapool.c
> +++ b/mm/dmapool.c
> @@ -346,6 +346,29 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
>  	retval = offset + page->vaddr;
>  	*handle = offset + page->dma;
>  #ifdef	DMAPOOL_DEBUG
> +	{
> +		int i;
> +		u8 *data = retval;
> +		/* page->offset is stored in first 4 bytes */
> +		for (i = sizeof(int); i < pool->size; i++) {
> +			if (data[i] != POOL_POISON_FREED) {
> +				if (pool->dev)
> +					dev_err(pool->dev,
> +							"dma_pool_alloc %s, %p (corruped)\n",
> +							pool->name, retval);
> +				else
> +					printk(KERN_ERR
> +							"dma_pool_alloc %s, %p (corruped)\n",
> +							pool->name, retval);
> +
> +				/* we dump the first 4 bytes even if there are not
> +				   POOL_POISON_FREED */
> +				print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 16, 1,
> +						data, pool->size, 1);
> +				break;
> +			}
> +		}
> +	}
>  	memset(retval, POOL_POISON_ALLOCATED, pool->size);
>  #endif

Seems reasonable.  I fidded wth a few things.

- The code is quite painful to look at in an 80-col display, and
  there's no real reason it has to be that way.

- The comment layout is strange, it misses capitalisation and needed
  a grammatical fix

- Use sizeof(page->offset), not sizeof(int).  Just in case someone
  changes the type of dma_page.offset.

- Use pr_err(), as suggested by checkpatch.  Please use checkpatch.

--- a/mm/dmapool.c~dmapool-make-dmapool_debug-detect-corruption-of-free-marker-fix
+++ a/mm/dmapool.c
@@ -350,23 +350,24 @@ void *dma_pool_alloc(struct dma_pool *po
 		int i;
 		u8 *data = retval;
 		/* page->offset is stored in first 4 bytes */
-		for (i = sizeof(int); i < pool->size; i++) {
-			if (data[i] != POOL_POISON_FREED) {
-				if (pool->dev)
-					dev_err(pool->dev,
-							"dma_pool_alloc %s, %p (corruped)\n",
-							pool->name, retval);
-				else
-					printk(KERN_ERR
-							"dma_pool_alloc %s, %p (corruped)\n",
-							pool->name, retval);
-
-				/* we dump the first 4 bytes even if there are not
-				   POOL_POISON_FREED */
-				print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 16, 1,
-						data, pool->size, 1);
-				break;
-			}
+		for (i = sizeof(page->offset); i < pool->size; i++) {
+			if (data[i] == POOL_POISON_FREED)
+				continue;
+			if (pool->dev)
+				dev_err(pool->dev,
+					"dma_pool_alloc %s, %p (corruped)\n",
+					pool->name, retval);
+			else
+				pr_err("dma_pool_alloc %s, %p (corruped)\n",
+					pool->name, retval);
+
+			/*
+			 * Dump the first 4 bytes even if they are not
+			 * POOL_POISON_FREED
+			*/
+			print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 16, 1,
+					data, pool->size, 1);
+			break;
 		}
 	}
 	memset(retval, POOL_POISON_ALLOCATED, pool->size);
_