Messages in this thread Patch in this message | | | Date | Mon, 5 Nov 2012 12:05:06 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] dmapool : make DMAPOOL_DEBUG detect corruption of free marker |
| |
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); _
| |