On Tue, 5 Feb 2008 19:52:47 -0700 Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> Could I ask you to pull the DMA Pool changes detailed below? > > All the patches have been posted to linux-kernel before, and various > comments (and acks) have been taken into account. (see > http://thread.gmane.org/gmane.linux.kernel/609943) > > It's a fairly nice performance improvement, so would be good to get in. > It's survived a few hours of *mumble* high-stress database benchmark, > so I have high confidence in its stability. > > The following changes since commit 21511abd0a248a3f225d3b611cfabb93124605a7: > Linus Torvalds (1): > Merge branch 'release' of git://git.kernel.org/.../aegl/linux-2.6 > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git dmapool Looks OK to me - I think I reviewed these a while back? We really should have had this tree in -mm for general tyre-kicking. What's with the #ifdef CONFIG_SLAB stuff in the dmapool code? Are we just overloading a convenient Kconfig label here, or is it required for some reason? Why not CONFIG_SLUB_DEBUG too? For the future: This code: +struct dma_pool *dma_pool_create(const char *name, struct device *dev, + size_t size, size_t align, size_t boundary) +{ + struct dma_pool *retval; + size_t allocation; + + if (align == 0) { + align = 1; + } else if (align & (align - 1)) { + return NULL; + } + + if (size == 0) { + return NULL; + } else if (size < 4) { + size = 4; + } + + if ((size % align) != 0) + size = ALIGN(size, align); + + allocation = max_t(size_t, size, PAGE_SIZE); + + if (!boundary) { + boundary = allocation; + } else if ((boundary < size) || (boundary & (boundary - 1))) { + return NULL; + } could do with some relief from its brace fetish and some education about is_power_of_2(). And the `if ((size % align) != 0)' can just go away, which will save code and is likely faster. It's a separate thing I guess, but I don't see any reason why dma_pool_alloc() _has_ to use GFP_ATOMIC. Looks like it can do the allocation outside spin_lock_irqsave() and use mem_flags instead. If that has __GFP_WAIT then we're in much better shape. I wish all this code wouldn't do struct dma_page *page; because one very much expects a local variable called "page" to be of type `struct page *'. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
