lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 28 Aug 2013 19:18:29 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Wanpeng Li <liwanp@...ux.vnet.ibm.com>
Cc:	Dave Hansen <dave.hansen@...ux.intel.com>,
	Rik van Riel <riel@...hat.com>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Johannes Weiner <hannes@...xchg.org>,
	Tejun Heo <tj@...nel.org>,
	Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>,
	David Rientjes <rientjes@...gle.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Jiri Kosina <jkosina@...e.cz>, Linux MM <linux-mm@...ck.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/4] mm/sparse: introduce alloc_usemap_and_memmap

On Thu, Aug 22, 2013 at 5:14 AM, Wanpeng Li <liwanp@...ux.vnet.ibm.com> wrote:
>From f21640b6dc15c76ac10fccada96e6b9fdce5a092 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <liwanp@...ux.vnet.ibm.com>
Date: Thu, 22 Aug 2013 19:57:54 +0800
Subject: [PATCH] mm/sparse: introduce alloc_usemap_and_memmap fix

Pass function pointer to alloc_usemap_and_memmap() instead of true/false.

Signed-off-by: Wanpeng Li <liwanp@...ux.vnet.ibm.com>
---
 mm/sparse.c | 78 ++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 55e5752..22a1a26 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -14,6 +14,14 @@
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>

+struct alloc_info {
+    unsigned long **map_map;
+    unsigned long pnum_begin;
+    unsigned long pnum_end;
+    unsigned long map_count;
+    int nodeid;
+};
+
 /*
  * Permanent SPARSEMEM data:
  *
@@ -339,13 +347,16 @@ static void __init check_usemap_section_nr(int
nid, unsigned long *usemap)
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */

-static void __init sparse_early_usemaps_alloc_node(unsigned long**usemap_map,
-                 unsigned long pnum_begin,
-                 unsigned long pnum_end,
-                 unsigned long usemap_count, int nodeid)
+static void __init sparse_early_usemaps_alloc_node(void *data)
 {
     void *usemap;
     unsigned long pnum;
+    struct alloc_info *info = (struct alloc_info *)data;
+    unsigned long **usemap_map = info->map_map;
+    unsigned long pnum_begin = info->pnum_begin;
+    unsigned long pnum_end = info->pnum_end;

+    unsigned long usemap_count = info->map_count;
+    int nodeid = info->nodeid;

===>
usemap_count and nodeid is only referred one time, you may omit those
two temp variables.


     int size = usemap_size();

     usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nodeid),
@@ -430,23 +441,13 @@ void __init sparse_mem_maps_populate_node(struct
page **map_map,
 #endif /* !CONFIG_SPARSEMEM_VMEMMAP */

 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
-static void __init sparse_early_mem_maps_alloc_node(struct page **map_map,
-                 unsigned long pnum_begin,
-                 unsigned long pnum_end,
-                 unsigned long map_count, int nodeid)
+static void __init sparse_early_mem_maps_alloc_node(void *data)
 {
-    sparse_mem_maps_populate_node(map_map, pnum_begin, pnum_end,
-                     map_count, nodeid);
+    struct alloc_info *info = (struct alloc_info *)data;
+    sparse_mem_maps_populate_node((struct page **)info->map_map,
+    info->pnum_begin, info->pnum_end, info->map_count, info->nodeid);
 }
 #else
-
-static void __init sparse_early_mem_maps_alloc_node(struct page **map_map,
-                unsigned long pnum_begin,
-                unsigned long pnum_end,
-                unsigned long map_count, int nodeid)
-{
-}
-
 static struct page __init *sparse_early_mem_map_alloc(unsigned long pnum)
 {
     struct page *map;
@@ -471,14 +472,15 @@ void __attribute__((weak)) __meminit
vmemmap_populate_print_last(void)
 /**
  *  alloc_usemap_and_memmap - memory alloction for pageblock flags and vmemmap
  *  @map: usemap_map for pageblock flags or mmap_map for vmemmap
- *  @use_map: true if memory allocated for pageblock flags, otherwise false
  */
-static void alloc_usemap_and_memmap(unsigned long **map, bool use_map)
+static void alloc_usemap_and_memmap(void (*sparse_early_maps_alloc_node)
+                (void *data), void *data)

==> how about changing to :

+static void alloc_map(void (*alloc_fn)(void *), void *data)


 {
     unsigned long pnum;
     unsigned long map_count;
     int nodeid_begin = 0;
     unsigned long pnum_begin = 0;
+    struct alloc_info *info = (struct alloc_info *)data;

     for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
         struct mem_section *ms;
@@ -503,25 +505,24 @@ static void alloc_usemap_and_memmap(unsigned
long **map, bool use_map)
             map_count++;
             continue;
         }
+
+        info->pnum_begin = pnum_begin;
+        info->pnum_end = pnum;
+        info->map_count = map_count;
+        info->nodeid = nodeid_begin;
         /* ok, we need to take cake of from pnum_begin to pnum - 1*/
-        if (use_map)
-            sparse_early_usemaps_alloc_node(map, pnum_begin, pnum,
-                         map_count, nodeid_begin);
-        else
-            sparse_early_mem_maps_alloc_node((struct page **)map,
-                pnum_begin, pnum, map_count, nodeid_begin);
+        sparse_early_maps_alloc_node((void *)info);
         /* new start, update count etc*/
         nodeid_begin = nodeid;
         pnum_begin = pnum;
         map_count = 1;
     }
     /* ok, last chunk */
-    if (use_map)
-        sparse_early_usemaps_alloc_node(map, pnum_begin,
-                NR_MEM_SECTIONS, map_count, nodeid_begin);
-    else
-        sparse_early_mem_maps_alloc_node((struct page **)map,
-            pnum_begin, NR_MEM_SECTIONS, map_count, nodeid_begin);
+    info->pnum_begin = pnum_begin;
+    info->pnum_end = NR_MEM_SECTIONS;
+    info->map_count = map_count;
+    info->nodeid = nodeid_begin;
+    sparse_early_maps_alloc_node((void *)info);
 }

 /*
@@ -535,11 +536,14 @@ void __init sparse_init(void)
     unsigned long *usemap;
     unsigned long **usemap_map;
     int size;
+    struct alloc_info data;
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
     int size2;
     struct page **map_map;
 #endif

+    void (*sparse_early_maps_alloc_node)(void *data);
+
     /* see include/linux/mmzone.h 'struct mem_section' definition */
     BUILD_BUG_ON(!is_power_of_2(sizeof(struct mem_section)));

@@ -561,14 +565,20 @@ void __init sparse_init(void)
     usemap_map = alloc_bootmem(size);
     if (!usemap_map)
         panic("can not allocate usemap_map\n");
-    alloc_usemap_and_memmap(usemap_map, true);
+    sparse_early_maps_alloc_node = sparse_early_usemaps_alloc_node;

so you can not use sparse_early_usemaps_alloc_nod() directly?

+    data.map_map = usemap_map;
+    alloc_usemap_and_memmap(sparse_early_maps_alloc_node, (void *)&data);

+    usemap_map = data.map_map;

why do you need to this assign ? data.map_map value get changed?



 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
     size2 = sizeof(struct page *) * NR_MEM_SECTIONS;
     map_map = alloc_bootmem(size2);
     if (!map_map)
         panic("can not allocate map_map\n");
-    alloc_usemap_and_memmap((unsigned long **)map_map, false);
+    sparse_early_maps_alloc_node = sparse_early_mem_maps_alloc_node;
+    data.map_map = (unsigned long **)map_map;
+    alloc_usemap_and_memmap(sparse_early_maps_alloc_node, (void *)&data);
+    map_map = (struct page **)data.map_map;
 #endif


please change to function pointer to
void  (*alloc_func)(void *data,
                                 unsigned long pnum_begin,
                                 unsigned long pnum_end,
                                 unsigned long map_count, int nodeid)

pnum_begin, pnum_end, map_coun, nodeid, should not be in the struct.

please send patch against linus/master, so i could send back updated
version directly.

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ