Pull request: DMA pool updates

Pull request: DMA pool updates

[Matthew Wilcox]

G'day Linus, mate

Could you pull the dmapool branch of
git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git please?

All the patches have been posted to linux-kernel before, and various
comments (and acks) have been taken into account.

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.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Pull request: DMA pool updates

[Andrew Morton]

On Mon, 28 Jan 2008 17:11:47 -0700
Matthew Wilcox <matthew@wil.cx> wrote:

> 
> G'day Linus, mate
> 
> Could you pull the dmapool branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git please?

The usual form is, I believe,

	git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git dmapool

Otherwise people get all confused and think it's an empty tree (like I just
did).

> All the patches have been posted to linux-kernel before, and various
> comments (and acks) have been taken into account.
> 
> 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.

Could we please at least have a shortlog so we can find out what the patch
titles are so we can google them so we can find out what the heck you're
proposing we add to the kernel?

<does a pull, goes on an archive hunt>

It shouldn't be this hard!

There were no replies to v2 of the patch series.  It all looks reasonable
from a quick scan (assuming the patches are unchanged since then).

afaik these patches have been tested by nobody except thyself?

Re: Pull request: DMA pool updates

[Matthew Wilcox]

On Mon, Jan 28, 2008 at 05:07:34PM -0800, Andrew Morton wrote:
> The usual form is, I believe,
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git dmapool
> 
> Otherwise people get all confused and think it's an empty tree (like I just
> did).

Sorry!

> There were no replies to v2 of the patch series.  It all looks reasonable
> from a quick scan (assuming the patches are unchanged since then).

I haven't changed them, correct.

> afaik these patches have been tested by nobody except thyself?

I've tested them myself, then I sent them to the perf team who ran the
(4 hour long) benchmark, and they reported success.  As with many patches
these days, they sank into a pit of indifference.  Perhaps I need to
take a leaf from my former government's book and sex up my patch
descriptions a bit.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: Pull request: DMA pool updates

[Andrew Morton]

On Mon, 28 Jan 2008 19:45:25 -0700 Matthew Wilcox <matthew@wil.cx> wrote:

> > afaik these patches have been tested by nobody except thyself?
> 
> I've tested them myself, then I sent them to the perf team who ran the
> (4 hour long) benchmark, and they reported success.  As with many patches
> these days, they sank into a pit of indifference.

I like to think that's because everyone is all fired up about bugfixes and
the regression reports.  heh.

It's a simple matter for me to add another git tree, which gets things a
bit more exposure.

>  Perhaps I need to
> take a leaf from my former government's book and sex up my patch
> descriptions a bit.

Well these two pulls came with effectively no description at all.  Put
yourself in Linus's position...

Re: Pull request: DMA pool updates

[mark gross]

On Mon, Jan 28, 2008 at 05:11:47PM -0700, Matthew Wilcox wrote:
> G'day Linus, mate
> 
> Could you pull the dmapool branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/willy/misc.git please?
> 
> All the patches have been posted to linux-kernel before, and various
> comments (and acks) have been taken into account.
> 
> 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.

I haven't looked at this yet but, I've been doing some work with the
IOMMU performance impacts on DMA operations.  DMApooling  could provide
a way to mitigate some of the spankage gotten when using IOMMU's.

--mgross

> 
> -- 
> Intel are signing my paycheques ... these opinions are still mine
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours.  We can't possibly take such
> a retrograde step."
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Pull request: DMA pool updates

[Andrew Morton]

On Tue, 5 Feb 2008 19:52:47 -0700 Matthew Wilcox <matthew@wil.cx> 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 *'.  

Pull request: DMA pool updates

[Matthew Wilcox]

Hi Linus,

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

Matthew Wilcox (7):
      Move dmapool.c to mm/ directory
      dmapool: Fix style problems
      Avoid taking waitqueue lock in dmapool
      dmapool: Validate parameters to dma_pool_create
      dmapool: Tidy up includes and add comments
      Change dmapool free block management
      pool: Improve memory usage for devices which can't cross boundaries

 drivers/base/Makefile  |    2 +-
 drivers/base/dmapool.c |  481 ----------------------------------------------
 mm/Makefile            |    1 +
 mm/dmapool.c           |  500 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 502 insertions(+), 482 deletions(-)
 delete mode 100644 drivers/base/dmapool.c
 create mode 100644 mm/dmapool.c

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."