Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-19 Thread Dave Young 
On Mon, Oct 18, 2010 at 04:46:47PM -0700, Andrew Morton wrote:
 On Sat, 16 Oct 2010 12:33:31 +0800
 Dave Young hidave.darks...@gmail.com wrote:
 
  Add vzalloc for convinience of vmalloc-then-memset-zero case 
  
  Use __GFP_ZERO in vzalloc to zero fill the allocated memory.
  
  Signed-off-by: Dave Young hidave.darks...@gmail.com
  ---
   include/linux/vmalloc.h |1 +
   mm/vmalloc.c|   13 +
   2 files changed, 14 insertions(+)
  
  --- linux-2.6.orig/include/linux/vmalloc.h  2010-08-22 15:31:38.0 
  +0800
  +++ linux-2.6/include/linux/vmalloc.h   2010-10-16 10:50:54.739996121 
  +0800
  @@ -53,6 +53,7 @@ static inline void vmalloc_init(void)
   #endif
   
   extern void *vmalloc(unsigned long size);
  +extern void *vzalloc(unsigned long size);
   extern void *vmalloc_user(unsigned long size);
   extern void *vmalloc_node(unsigned long size, int node);
   extern void *vmalloc_exec(unsigned long size);
  --- linux-2.6.orig/mm/vmalloc.c 2010-08-22 15:31:39.0 +0800
  +++ linux-2.6/mm/vmalloc.c  2010-10-16 10:51:57.126665918 +0800
  @@ -1604,6 +1604,19 @@ void *vmalloc(unsigned long size)
   EXPORT_SYMBOL(vmalloc);
   
   /**
  + * vzalloc  -  allocate virtually contiguous memory with zero filled
 
 s/filled/fill/
 
  + * @size:  allocation size
  + * Allocate enough pages to cover @size from the page level
  + * allocator and map them into contiguous kernel virtual space.
  + */
  +void *vzalloc(unsigned long size)
  +{
  +   return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO,
  +   PAGE_KERNEL, -1, __builtin_return_address(0));
  +}
  +EXPORT_SYMBOL(vzalloc);
 
 We'd need to add the same interface to nommu, please.
 
 Also, a slightly better implementation would be
 
 static inline void *__vmalloc_node_flags(unsigned long size, gfp_t flags)
 {
   return __vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
   __builtin_return_address(0));
 }
 
 void *vzalloc(unsigned long size)
 {
   return __vmalloc_node_flags(size,
   GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
 }
 
 void *vmalloc(unsigned long size)
 {
   return __vmalloc_node_flags(size, GFP_KERNEL | __GFP_HIGHMEM);
 }
 
 just to avoid code duplication (and possible later errors derived from it).
 
 Perhaps it should be always_inline, so the __builtin_return_address()
 can't get broken.
 
 Or just leave it the way you had it :)
 
 

Hi, here is the updated version:
---

Add vzalloc and vzalloc_node for convinience of vmalloc-then-memset-zero case 

Use __GFP_ZERO in vzalloc to zero fill the allocated memory.

changes from first submit:
nommu part (Minchan kim and Andrew Morton) 
comment fixes / __vmalloc_node_flags helper for clean code (Andrew Morton)
add vzalloc_node for completeness

Signed-off-by: Dave Young hidave.darks...@gmail.com
---
 include/linux/vmalloc.h |2 +
 mm/nommu.c  |   49 +++-
 mm/vmalloc.c|   46 +++--
 3 files changed, 94 insertions(+), 3 deletions(-)

--- linux-2.6.orig/include/linux/vmalloc.h  2010-10-19 20:44:20.38459 
+0800
+++ linux-2.6/include/linux/vmalloc.h   2010-10-19 20:45:07.36782 +0800
@@ -53,8 +53,10 @@ static inline void vmalloc_init(void)
 #endif
 
 extern void *vmalloc(unsigned long size);
+extern void *vzalloc(unsigned long size);
 extern void *vmalloc_user(unsigned long size);
 extern void *vmalloc_node(unsigned long size, int node);
+extern void *vzalloc_node(unsigned long size, int node);
 extern void *vmalloc_exec(unsigned long size);
 extern void *vmalloc_32(unsigned long size);
 extern void *vmalloc_32_user(unsigned long size);
--- linux-2.6.orig/mm/vmalloc.c 2010-10-19 20:44:20.38459 +0800
+++ linux-2.6/mm/vmalloc.c  2010-10-19 20:53:01.29793 +0800
@@ -1587,6 +1587,13 @@ void *__vmalloc(unsigned long size, gfp_
 }
 EXPORT_SYMBOL(__vmalloc);
 
+static inline void *__vmalloc_node_flags(unsigned long size,
+   int node, gfp_t flags)
+{
+   return __vmalloc_node(size, 1, flags, PAGE_KERNEL,
+   node, __builtin_return_address(0));
+}
+
 /**
  * vmalloc  -  allocate virtually contiguous memory
  * @size:  allocation size
@@ -1598,12 +1605,28 @@ EXPORT_SYMBOL(__vmalloc);
  */
 void *vmalloc(unsigned long size)
 {
-   return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM, PAGE_KERNEL,
-   -1, __builtin_return_address(0));
+   return __vmalloc_node_flags(size, -1, GFP_KERNEL | __GFP_HIGHMEM);
 }
 EXPORT_SYMBOL(vmalloc);
 
 /**
+ * vzalloc - allocate virtually contiguous memory with zero fill
+ * @size:  allocation size
+ * Allocate enough pages to cover @size from the page level
+ * allocator and map them into contiguous kernel virtual space.
+ * The memory allocated

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Christoph Lameter 
On Sat, 16 Oct 2010, Dave Young wrote:

 Add vzalloc for convinience of vmalloc-then-memset-zero case

Reviewed-by: Christoph Lameter c...@linux.com

Wish we would also have vzalloc_node() but I guess that can wait.

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Andrew Morton 
On Sat, 16 Oct 2010 12:33:31 +0800
Dave Young hidave.darks...@gmail.com wrote:

 Add vzalloc for convinience of vmalloc-then-memset-zero case 
 
 Use __GFP_ZERO in vzalloc to zero fill the allocated memory.
 
 Signed-off-by: Dave Young hidave.darks...@gmail.com
 ---
  include/linux/vmalloc.h |1 +
  mm/vmalloc.c|   13 +
  2 files changed, 14 insertions(+)
 
 --- linux-2.6.orig/include/linux/vmalloc.h2010-08-22 15:31:38.0 
 +0800
 +++ linux-2.6/include/linux/vmalloc.h 2010-10-16 10:50:54.739996121 +0800
 @@ -53,6 +53,7 @@ static inline void vmalloc_init(void)
  #endif
  
  extern void *vmalloc(unsigned long size);
 +extern void *vzalloc(unsigned long size);
  extern void *vmalloc_user(unsigned long size);
  extern void *vmalloc_node(unsigned long size, int node);
  extern void *vmalloc_exec(unsigned long size);
 --- linux-2.6.orig/mm/vmalloc.c   2010-08-22 15:31:39.0 +0800
 +++ linux-2.6/mm/vmalloc.c2010-10-16 10:51:57.126665918 +0800
 @@ -1604,6 +1604,19 @@ void *vmalloc(unsigned long size)
  EXPORT_SYMBOL(vmalloc);
  
  /**
 + *   vzalloc  -  allocate virtually contiguous memory with zero filled

s/filled/fill/

 + *   @size:  allocation size
 + *   Allocate enough pages to cover @size from the page level
 + *   allocator and map them into contiguous kernel virtual space.
 + */
 +void *vzalloc(unsigned long size)
 +{
 + return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO,
 + PAGE_KERNEL, -1, __builtin_return_address(0));
 +}
 +EXPORT_SYMBOL(vzalloc);

We'd need to add the same interface to nommu, please.

Also, a slightly better implementation would be

static inline void *__vmalloc_node_flags(unsigned long size, gfp_t flags)
{
return __vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
__builtin_return_address(0));
}

void *vzalloc(unsigned long size)
{
return __vmalloc_node_flags(size,
GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
}

void *vmalloc(unsigned long size)
{
return __vmalloc_node_flags(size, GFP_KERNEL | __GFP_HIGHMEM);
}

just to avoid code duplication (and possible later errors derived from it).

Perhaps it should be always_inline, so the __builtin_return_address()
can't get broken.

Or just leave it the way you had it :)


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Dave Young 
On Tue, Oct 19, 2010 at 7:46 AM, Andrew Morton
a...@linux-foundation.org wrote:
 On Sat, 16 Oct 2010 12:33:31 +0800
 Dave Young hidave.darks...@gmail.com wrote:

 Add vzalloc for convinience of vmalloc-then-memset-zero case

 Use __GFP_ZERO in vzalloc to zero fill the allocated memory.

 Signed-off-by: Dave Young hidave.darks...@gmail.com
 ---
  include/linux/vmalloc.h |    1 +
  mm/vmalloc.c            |   13 +
  2 files changed, 14 insertions(+)

 --- linux-2.6.orig/include/linux/vmalloc.h    2010-08-22 15:31:38.0 
 +0800
 +++ linux-2.6/include/linux/vmalloc.h 2010-10-16 10:50:54.739996121 +0800
 @@ -53,6 +53,7 @@ static inline void vmalloc_init(void)
  #endif

  extern void *vmalloc(unsigned long size);
 +extern void *vzalloc(unsigned long size);
  extern void *vmalloc_user(unsigned long size);
  extern void *vmalloc_node(unsigned long size, int node);
  extern void *vmalloc_exec(unsigned long size);
 --- linux-2.6.orig/mm/vmalloc.c       2010-08-22 15:31:39.0 +0800
 +++ linux-2.6/mm/vmalloc.c    2010-10-16 10:51:57.126665918 +0800
 @@ -1604,6 +1604,19 @@ void *vmalloc(unsigned long size)
  EXPORT_SYMBOL(vmalloc);

  /**
 + *   vzalloc  -  allocate virtually contiguous memory with zero filled

 s/filled/fill/

Thanks, Will fix


 + *   @size:          allocation size
 + *   Allocate enough pages to cover @size from the page level
 + *   allocator and map them into contiguous kernel virtual space.
 + */
 +void *vzalloc(unsigned long size)
 +{
 +     return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO,
 +                             PAGE_KERNEL, -1, __builtin_return_address(0));
 +}
 +EXPORT_SYMBOL(vzalloc);

 We'd need to add the same interface to nommu, please.

Ok, will do

Minchan kim, thanks as well. I missed your comments about nommu before.


 Also, a slightly better implementation would be

 static inline void *__vmalloc_node_flags(unsigned long size, gfp_t flags)
 {
        return __vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
                                __builtin_return_address(0));
 }

 void *vzalloc(unsigned long size)
 {
        return __vmalloc_node_flags(size,
                                GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
 }

 void *vmalloc(unsigned long size)
 {
        return __vmalloc_node_flags(size, GFP_KERNEL | __GFP_HIGHMEM);
 }

 just to avoid code duplication (and possible later errors derived from it).

 Perhaps it should be always_inline, so the __builtin_return_address()
 can't get broken.

 Or just leave it the way you had it :)

Andrew, your suggestion is cleaner and better. I will do as yours.

-- 
Regards
dave
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Dave Young 
On Tue, Oct 19, 2010 at 9:27 AM, Dave Young hidave.darks...@gmail.com wrote:
 On Tue, Oct 19, 2010 at 7:46 AM, Andrew Morton
 a...@linux-foundation.org wrote:
 On Sat, 16 Oct 2010 12:33:31 +0800
 Dave Young hidave.darks...@gmail.com wrote:

 Add vzalloc for convinience of vmalloc-then-memset-zero case

 Use __GFP_ZERO in vzalloc to zero fill the allocated memory.

 Signed-off-by: Dave Young hidave.darks...@gmail.com
 ---
  include/linux/vmalloc.h |    1 +
  mm/vmalloc.c            |   13 +
  2 files changed, 14 insertions(+)

 --- linux-2.6.orig/include/linux/vmalloc.h    2010-08-22 15:31:38.0 
 +0800
 +++ linux-2.6/include/linux/vmalloc.h 2010-10-16 10:50:54.739996121 +0800
 @@ -53,6 +53,7 @@ static inline void vmalloc_init(void)
  #endif

  extern void *vmalloc(unsigned long size);
 +extern void *vzalloc(unsigned long size);
  extern void *vmalloc_user(unsigned long size);
  extern void *vmalloc_node(unsigned long size, int node);
  extern void *vmalloc_exec(unsigned long size);
 --- linux-2.6.orig/mm/vmalloc.c       2010-08-22 15:31:39.0 +0800
 +++ linux-2.6/mm/vmalloc.c    2010-10-16 10:51:57.126665918 +0800
 @@ -1604,6 +1604,19 @@ void *vmalloc(unsigned long size)
  EXPORT_SYMBOL(vmalloc);

  /**
 + *   vzalloc  -  allocate virtually contiguous memory with zero filled

 s/filled/fill/

 Thanks, Will fix


 + *   @size:          allocation size
 + *   Allocate enough pages to cover @size from the page level
 + *   allocator and map them into contiguous kernel virtual space.
 + */
 +void *vzalloc(unsigned long size)
 +{
 +     return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM | 
 __GFP_ZERO,
 +                             PAGE_KERNEL, -1, __builtin_return_address(0));
 +}
 +EXPORT_SYMBOL(vzalloc);

 We'd need to add the same interface to nommu, please.

 Ok, will do

 Minchan kim, thanks as well. I missed your comments about nommu before.


 Also, a slightly better implementation would be

 static inline void *__vmalloc_node_flags(unsigned long size, gfp_t flags)
 {
        return __vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
                                __builtin_return_address(0));
 }

Is this better? might __vmalloc_node_flags would be used by other than vmalloc?

static inline void *__vmalloc_node_flags(unsigned long size, int node,
gfp_t flags)


 void *vzalloc(unsigned long size)
 {
        return __vmalloc_node_flags(size,
                                GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
 }

 void *vmalloc(unsigned long size)
 {
        return __vmalloc_node_flags(size, GFP_KERNEL | __GFP_HIGHMEM);
 }

 just to avoid code duplication (and possible later errors derived from it).

 Perhaps it should be always_inline, so the __builtin_return_address()
 can't get broken.

 Or just leave it the way you had it :)

 Andrew, your suggestion is cleaner and better. I will do as yours.

 --
 Regards
 dave




-- 
Regards
dave
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Andrew Morton 
On Tue, 19 Oct 2010 09:55:17 +0800 Dave Young hidave.darks...@gmail.com wrote:

 On Tue, Oct 19, 2010 at 9:27 AM, Dave Young hidave.darks...@gmail.com wrote:
  On Tue, Oct 19, 2010 at 7:46 AM, Andrew Morton
 
  Also, a slightly better implementation would be
 
  static inline void * vmalloc_node_flags(unsigned long size, gfp_t flags)
  {
 return  vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
  builtin_return_address(0));
  }
 
 Is this better? might  vmalloc_node_flags would be used by other than vmalloc?
 
 static inline void * vmalloc_node_flags(unsigned long size, int node,
 gfp_t flags)

I have no strong opinions, really.  If we add more and more arguments
to vmalloc_node_flags() it ends up looking like vmalloc_node(), so we
may as well just call vmalloc_node().  Do whatever feels good ;)

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-18 Thread Dave Young 
On Tue, Oct 19, 2010 at 10:18 AM, Andrew Morton
a...@linux-foundation.org wrote:
 On Tue, 19 Oct 2010 09:55:17 +0800 Dave Young hidave.darks...@gmail.com 
 wrote:

 On Tue, Oct 19, 2010 at 9:27 AM, Dave Young hidave.darks...@gmail.com 
 wrote:
  On Tue, Oct 19, 2010 at 7:46 AM, Andrew Morton
 
  Also, a slightly better implementation would be
 
  static inline void * vmalloc_node_flags(unsigned long size, gfp_t flags)
  {
         return  vmalloc_node(size, 1, flags, PAGE_KERNEL, -1,
                                  builtin_return_address(0));
  }

 Is this better? might  vmalloc_node_flags would be used by other than 
 vmalloc?

 static inline void * vmalloc_node_flags(unsigned long size, int node,
 gfp_t flags)

 I have no strong opinions, really.  If we add more and more arguments
 to vmalloc_node_flags() it ends up looking like vmalloc_node(), so we
 may as well just call vmalloc_node().  Do whatever feels good ;)

Ok, thanks.

Then I would prefer add 'node' argument due to the function name of
vmalloc_node_flags

-- 
Regards
dave
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Add vzalloc shortcut

2010-10-16 Thread Minchan Kim 
On Sat, Oct 16, 2010 at 1:33 PM, Dave Young hidave.darks...@gmail.com wrote:
 Add vzalloc for convinience of vmalloc-then-memset-zero case

 Use __GFP_ZERO in vzalloc to zero fill the allocated memory.
Looks good to me.

There are many place we need this.
Although it affects meta pages for vmalloc as well as data pages, it's
not a big.
In this case, Maintaining code simple is better than little bit
performance overhead.


 Signed-off-by: Dave Young hidave.darks...@gmail.com
Reviewed-by: Minchan Kim minchan@gmail.com

Isn't it useful in nommu, either?


 ---
  include/linux/vmalloc.h |    1 +
  mm/vmalloc.c            |   13 +
  2 files changed, 14 insertions(+)

 --- linux-2.6.orig/include/linux/vmalloc.h      2010-08-22 15:31:38.0 
 +0800
 +++ linux-2.6/include/linux/vmalloc.h   2010-10-16 10:50:54.739996121 +0800
 @@ -53,6 +53,7 @@ static inline void vmalloc_init(void)
  #endif

  extern void *vmalloc(unsigned long size);
 +extern void *vzalloc(unsigned long size);
  extern void *vmalloc_user(unsigned long size);
  extern void *vmalloc_node(unsigned long size, int node);
  extern void *vmalloc_exec(unsigned long size);
 --- linux-2.6.orig/mm/vmalloc.c 2010-08-22 15:31:39.0 +0800
 +++ linux-2.6/mm/vmalloc.c      2010-10-16 10:51:57.126665918 +0800
 @@ -1604,6 +1604,19 @@ void *vmalloc(unsigned long size)
  EXPORT_SYMBOL(vmalloc);

  /**
 + *     vzalloc  -  allocate virtually contiguous memory with zero filled
 + *     @size:          allocation size
 + *     Allocate enough pages to cover @size from the page level
 + *     allocator and map them into contiguous kernel virtual space.
 + */
 +void *vzalloc(unsigned long size)
 +{
 +       return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_HIGHMEM | 
 __GFP_ZERO,
 +                               PAGE_KERNEL, -1, __builtin_return_address(0));
 +}
 +EXPORT_SYMBOL(vzalloc);
 +
 +/**
  * vmalloc_user - allocate zeroed virtually contiguous memory for userspace
  * @size: allocation size
  *

 --
 To unsubscribe, send a message with 'unsubscribe linux-mm' in
 the body to majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a





-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html