LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Pull request: DMA pool updates
@ 2008-02-06  2:52 Matthew Wilcox
  2008-02-06  3:22 ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2008-02-06  2:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, linux-mm

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."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Pull request: DMA pool updates
  2008-02-06  2:52 Pull request: DMA pool updates Matthew Wilcox
@ 2008-02-06  3:22 ` Andrew Morton
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2008-02-06  3:22 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Linus Torvalds, linux-kernel, linux-mm

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 *'.  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Pull request: DMA pool updates
  2008-01-29  0:11 Matthew Wilcox
  2008-01-29  1:07 ` Andrew Morton
@ 2008-02-04 17:55 ` mark gross
  1 sibling, 0 replies; 7+ messages in thread
From: mark gross @ 2008-02-04 17:55 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Linus Torvalds, Andrew Morton, linux-mm, linux-kernel

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>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Pull request: DMA pool updates
  2008-01-29  2:45   ` Matthew Wilcox
@ 2008-01-29  2:57     ` Andrew Morton
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2008-01-29  2:57 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, linux-mm, linux-kernel

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...


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Pull request: DMA pool updates
  2008-01-29  1:07 ` Andrew Morton
@ 2008-01-29  2:45   ` Matthew Wilcox
  2008-01-29  2:57     ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2008-01-29  2:45 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-mm, linux-kernel

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."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Pull request: DMA pool updates
  2008-01-29  0:11 Matthew Wilcox
@ 2008-01-29  1:07 ` Andrew Morton
  2008-01-29  2:45   ` Matthew Wilcox
  2008-02-04 17:55 ` mark gross
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2008-01-29  1:07 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, linux-mm, linux-kernel

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?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Pull request: DMA pool updates
@ 2008-01-29  0:11 Matthew Wilcox
  2008-01-29  1:07 ` Andrew Morton
  2008-02-04 17:55 ` mark gross
  0 siblings, 2 replies; 7+ messages in thread
From: Matthew Wilcox @ 2008-01-29  0:11 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andrew Morton, linux-mm, linux-kernel


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."

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-02-06  3:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-06  2:52 Pull request: DMA pool updates Matthew Wilcox
2008-02-06  3:22 ` Andrew Morton
  -- strict thread matches above, loose matches on Subject: below --
2008-01-29  0:11 Matthew Wilcox
2008-01-29  1:07 ` Andrew Morton
2008-01-29  2:45   ` Matthew Wilcox
2008-01-29  2:57     ` Andrew Morton
2008-02-04 17:55 ` mark gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).