Re: Pull request: DMA pool updates
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/
Pull request: DMA pool updates
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." -- 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/
Pull request: DMA pool updates
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. -- 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/
Re: Pull request: DMA pool updates
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/
Re: Pull request: DMA pool updates
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 [EMAIL PROTECTED] For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: mailto:"[EMAIL PROTECTED]"> [EMAIL PROTECTED] -- 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/
Re: Pull request: DMA pool updates
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 [EMAIL PROTECTED] For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: a href=mailto:[EMAIL PROTECTED] [EMAIL PROTECTED] /a -- 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/
Re: Pull request: DMA pool updates
On Mon, 28 Jan 2008 19:45:25 -0700 Matthew Wilcox <[EMAIL PROTECTED]> 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... -- 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/
Re: Pull request: DMA pool updates
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." -- 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/
Re: Pull request: DMA pool updates
On Mon, 28 Jan 2008 17:11:47 -0700 Matthew Wilcox <[EMAIL PROTECTED]> 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? 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? -- 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/
Pull request: DMA pool updates
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." -- 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/
Pull request: DMA pool updates
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. -- 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/
Re: Pull request: DMA pool updates
On Mon, 28 Jan 2008 17:11:47 -0700 Matthew Wilcox [EMAIL PROTECTED] 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? -- 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/
Re: Pull request: DMA pool updates
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. -- 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/
Re: Pull request: DMA pool updates
On Mon, 28 Jan 2008 19:45:25 -0700 Matthew Wilcox [EMAIL PROTECTED] 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... -- 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/
