v1 -> v2:
* remove failed.
preallocate_pmds will continue to preallocate pmds even if failure
occurrence, and then free all the preallocate pmds if there is
failure, this patch fix it by stop preallocate if failure occurrence
and go to free path.
Reviewed-by: Dave Hansen <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/mm/pgtable.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index dfa537a..65c2106 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -196,21 +196,18 @@ static void free_pmds(pmd_t *pmds[])
static int preallocate_pmds(pmd_t *pmds[])
{
int i;
- bool failed = false;
for(i = 0; i < PREALLOCATED_PMDS; i++) {
pmd_t *pmd = (pmd_t *)__get_free_page(PGALLOC_GFP);
if (pmd == NULL)
- failed = true;
+ goto err;
pmds[i] = pmd;
}
- if (failed) {
- free_pmds(pmds);
- return -ENOMEM;
- }
-
return 0;
+err:
+ free_pmds(pmds);
+ return -ENOMEM;
}
/*
--
1.8.1.2
It's not used globally and could be static.
Signed-off-by: Wanpeng Li <[email protected]>
---
fs/fs-writeback.c | 2 +-
include/linux/writeback.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 87d7781..54b3c31 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -723,7 +723,7 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb,
return wrote;
}
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
+static long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
enum wb_reason reason)
{
struct wb_writeback_work work = {
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 4e198ca..021b8a3 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -98,8 +98,6 @@ int try_to_writeback_inodes_sb(struct super_block *, enum wb_reason reason);
int try_to_writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
enum wb_reason reason);
void sync_inodes_sb(struct super_block *);
-long writeback_inodes_wb(struct bdi_writeback *wb, long nr_pages,
- enum wb_reason reason);
void wakeup_flusher_threads(long nr_pages, enum wb_reason reason);
void inode_wait_for_writeback(struct inode *inode);
--
1.8.1.2
v1 -> v2:
* add comments to describe alloc_usemap_and_memmap
After commit 9bdac91424075("sparsemem: Put mem map for one node together."),
vmemmap for one node will be allocated together, its logic is similar as
memory allocation for pageblock flags. This patch introduce alloc_usemap_and_memmap
to extract the same logic of memory alloction for pageblock flags and vmemmap.
Signed-off-by: Wanpeng Li <[email protected]>
---
mm/sparse.c | 140 ++++++++++++++++++++++++++++--------------------------------
1 file changed, 66 insertions(+), 74 deletions(-)
diff --git a/mm/sparse.c b/mm/sparse.c
index 308d503..d27db9b 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -439,6 +439,14 @@ static void __init sparse_early_mem_maps_alloc_node(struct page **map_map,
map_count, 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;
@@ -460,6 +468,62 @@ 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)
+{
+ unsigned long pnum;
+ unsigned long map_count;
+ int nodeid_begin = 0;
+ unsigned long pnum_begin = 0;
+
+ for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
+ struct mem_section *ms;
+
+ if (!present_section_nr(pnum))
+ continue;
+ ms = __nr_to_section(pnum);
+ nodeid_begin = sparse_early_nid(ms);
+ pnum_begin = pnum;
+ break;
+ }
+ map_count = 1;
+ for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
+ struct mem_section *ms;
+ int nodeid;
+
+ if (!present_section_nr(pnum))
+ continue;
+ ms = __nr_to_section(pnum);
+ nodeid = sparse_early_nid(ms);
+ if (nodeid == nodeid_begin) {
+ map_count++;
+ continue;
+ }
+ /* 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);
+ /* 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);
+}
+
/*
* Allocate the accumulated non-linear sections, allocate a mem_map
* for each and record the physical to section mapping.
@@ -471,11 +535,7 @@ void __init sparse_init(void)
unsigned long *usemap;
unsigned long **usemap_map;
int size;
- int nodeid_begin = 0;
- unsigned long pnum_begin = 0;
- unsigned long usemap_count;
#ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
- unsigned long map_count;
int size2;
struct page **map_map;
#endif
@@ -501,82 +561,14 @@ void __init sparse_init(void)
usemap_map = alloc_bootmem(size);
if (!usemap_map)
panic("can not allocate usemap_map\n");
-
- for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
- struct mem_section *ms;
-
- if (!present_section_nr(pnum))
- continue;
- ms = __nr_to_section(pnum);
- nodeid_begin = sparse_early_nid(ms);
- pnum_begin = pnum;
- break;
- }
- usemap_count = 1;
- for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
- struct mem_section *ms;
- int nodeid;
-
- if (!present_section_nr(pnum))
- continue;
- ms = __nr_to_section(pnum);
- nodeid = sparse_early_nid(ms);
- if (nodeid == nodeid_begin) {
- usemap_count++;
- continue;
- }
- /* ok, we need to take cake of from pnum_begin to pnum - 1*/
- sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, pnum,
- usemap_count, nodeid_begin);
- /* new start, update count etc*/
- nodeid_begin = nodeid;
- pnum_begin = pnum;
- usemap_count = 1;
- }
- /* ok, last chunk */
- sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
- usemap_count, nodeid_begin);
+ alloc_usemap_and_memmap(usemap_map, true);
#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");
-
- for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
- struct mem_section *ms;
-
- if (!present_section_nr(pnum))
- continue;
- ms = __nr_to_section(pnum);
- nodeid_begin = sparse_early_nid(ms);
- pnum_begin = pnum;
- break;
- }
- map_count = 1;
- for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
- struct mem_section *ms;
- int nodeid;
-
- if (!present_section_nr(pnum))
- continue;
- ms = __nr_to_section(pnum);
- nodeid = sparse_early_nid(ms);
- if (nodeid == nodeid_begin) {
- map_count++;
- continue;
- }
- /* ok, we need to take cake of from pnum_begin to pnum - 1*/
- sparse_early_mem_maps_alloc_node(map_map, pnum_begin, pnum,
- map_count, nodeid_begin);
- /* new start, update count etc*/
- nodeid_begin = nodeid;
- pnum_begin = pnum;
- map_count = 1;
- }
- /* ok, last chunk */
- sparse_early_mem_maps_alloc_node(map_map, pnum_begin, NR_MEM_SECTIONS,
- map_count, nodeid_begin);
+ alloc_usemap_and_memmap((unsigned long **)map_map, false);
#endif
for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
--
1.8.1.2
v1 -> v2:
* replace all the spots by function get_vm_area_size().
Use wrapper function get_vm_area_size to calculate size of vm area.
Signed-off-by: Wanpeng Li <[email protected]>
---
mm/vmalloc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 93d3182..1074543 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1258,7 +1258,7 @@ void unmap_kernel_range(unsigned long addr, unsigned long size)
int map_vm_area(struct vm_struct *area, pgprot_t prot, struct page ***pages)
{
unsigned long addr = (unsigned long)area->addr;
- unsigned long end = addr + area->size - PAGE_SIZE;
+ unsigned long end = addr + get_vm_area_size(area);
int err;
err = vmap_page_range(addr, end, prot, *pages);
@@ -1553,7 +1553,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
unsigned int nr_pages, array_size, i;
gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO;
- nr_pages = (area->size - PAGE_SIZE) >> PAGE_SHIFT;
+ nr_pages = get_vm_area_size(area) >> PAGE_SHIFT;
array_size = (nr_pages * sizeof(struct page *));
area->nr_pages = nr_pages;
@@ -1985,7 +1985,7 @@ long vread(char *buf, char *addr, unsigned long count)
vm = va->vm;
vaddr = (char *) vm->addr;
- if (addr >= vaddr + vm->size - PAGE_SIZE)
+ if (addr >= vaddr + get_vm_area_size(vm))
continue;
while (addr < vaddr) {
if (count == 0)
@@ -1995,7 +1995,7 @@ long vread(char *buf, char *addr, unsigned long count)
addr++;
count--;
}
- n = vaddr + vm->size - PAGE_SIZE - addr;
+ n = vaddr + get_vm_area_size(vm) - addr;
if (n > count)
n = count;
if (!(vm->flags & VM_IOREMAP))
@@ -2067,7 +2067,7 @@ long vwrite(char *buf, char *addr, unsigned long count)
vm = va->vm;
vaddr = (char *) vm->addr;
- if (addr >= vaddr + vm->size - PAGE_SIZE)
+ if (addr >= vaddr + get_vm_area_size(vm))
continue;
while (addr < vaddr) {
if (count == 0)
@@ -2076,7 +2076,7 @@ long vwrite(char *buf, char *addr, unsigned long count)
addr++;
count--;
}
- n = vaddr + vm->size - PAGE_SIZE - addr;
+ n = vaddr + get_vm_area_size(vm) - addr;
if (n > count)
n = count;
if (!(vm->flags & VM_IOREMAP)) {
--
1.8.1.2
On Tue, 20 Aug 2013 14:54:53 +0800 Wanpeng Li <[email protected]> wrote:
> preallocate_pmds will continue to preallocate pmds even if failure
> occurrence, and then free all the preallocate pmds if there is
> failure, this patch fix it by stop preallocate if failure occurrence
> and go to free path.
>
> ...
>
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -196,21 +196,18 @@ static void free_pmds(pmd_t *pmds[])
> static int preallocate_pmds(pmd_t *pmds[])
> {
> int i;
> - bool failed = false;
>
> for(i = 0; i < PREALLOCATED_PMDS; i++) {
> pmd_t *pmd = (pmd_t *)__get_free_page(PGALLOC_GFP);
> if (pmd == NULL)
> - failed = true;
> + goto err;
> pmds[i] = pmd;
> }
>
> - if (failed) {
> - free_pmds(pmds);
> - return -ENOMEM;
> - }
> -
> return 0;
> +err:
> + free_pmds(pmds);
> + return -ENOMEM;
> }
Nope. If the error path is taken, free_pmds() will free uninitialised
items from pmds[], which is a local in pgd_alloc() and contains random
stack junk. The kernel will crash.
You could pass an nr_pmds argument to free_pmds(), or zero out the
remaining items on the error path. However, although the current code
is a bit kooky, I don't see that it is harmful in any way.
> Reviewed-by: Dave Hansen <[email protected]>
Ahem.
On Tue, 20 Aug 2013 14:54:54 +0800 Wanpeng Li <[email protected]> wrote:
> v1 -> v2:
> * add comments to describe alloc_usemap_and_memmap
>
> After commit 9bdac91424075("sparsemem: Put mem map for one node together."),
> vmemmap for one node will be allocated together, its logic is similar as
> memory allocation for pageblock flags. This patch introduce alloc_usemap_and_memmap
> to extract the same logic of memory alloction for pageblock flags and vmemmap.
>
9bdac91424075 was written by Yinghai. He is an excellent reviewer, as
long as people remember to cc him!
> ---
> mm/sparse.c | 140 ++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 66 insertions(+), 74 deletions(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 308d503..d27db9b 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -439,6 +439,14 @@ static void __init sparse_early_mem_maps_alloc_node(struct page **map_map,
> map_count, 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;
> @@ -460,6 +468,62 @@ 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)
> +{
> + unsigned long pnum;
> + unsigned long map_count;
> + int nodeid_begin = 0;
> + unsigned long pnum_begin = 0;
> +
> + for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> + struct mem_section *ms;
> +
> + if (!present_section_nr(pnum))
> + continue;
> + ms = __nr_to_section(pnum);
> + nodeid_begin = sparse_early_nid(ms);
> + pnum_begin = pnum;
> + break;
> + }
> + map_count = 1;
> + for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
> + struct mem_section *ms;
> + int nodeid;
> +
> + if (!present_section_nr(pnum))
> + continue;
> + ms = __nr_to_section(pnum);
> + nodeid = sparse_early_nid(ms);
> + if (nodeid == nodeid_begin) {
> + map_count++;
> + continue;
> + }
> + /* 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);
> + /* 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);
> +}
> +
> /*
> * Allocate the accumulated non-linear sections, allocate a mem_map
> * for each and record the physical to section mapping.
> @@ -471,11 +535,7 @@ void __init sparse_init(void)
> unsigned long *usemap;
> unsigned long **usemap_map;
> int size;
> - int nodeid_begin = 0;
> - unsigned long pnum_begin = 0;
> - unsigned long usemap_count;
> #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
> - unsigned long map_count;
> int size2;
> struct page **map_map;
> #endif
> @@ -501,82 +561,14 @@ void __init sparse_init(void)
> usemap_map = alloc_bootmem(size);
> if (!usemap_map)
> panic("can not allocate usemap_map\n");
> -
> - for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> - struct mem_section *ms;
> -
> - if (!present_section_nr(pnum))
> - continue;
> - ms = __nr_to_section(pnum);
> - nodeid_begin = sparse_early_nid(ms);
> - pnum_begin = pnum;
> - break;
> - }
> - usemap_count = 1;
> - for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
> - struct mem_section *ms;
> - int nodeid;
> -
> - if (!present_section_nr(pnum))
> - continue;
> - ms = __nr_to_section(pnum);
> - nodeid = sparse_early_nid(ms);
> - if (nodeid == nodeid_begin) {
> - usemap_count++;
> - continue;
> - }
> - /* ok, we need to take cake of from pnum_begin to pnum - 1*/
> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, pnum,
> - usemap_count, nodeid_begin);
> - /* new start, update count etc*/
> - nodeid_begin = nodeid;
> - pnum_begin = pnum;
> - usemap_count = 1;
> - }
> - /* ok, last chunk */
> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
> - usemap_count, nodeid_begin);
> + alloc_usemap_and_memmap(usemap_map, true);
>
> #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");
> -
> - for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> - struct mem_section *ms;
> -
> - if (!present_section_nr(pnum))
> - continue;
> - ms = __nr_to_section(pnum);
> - nodeid_begin = sparse_early_nid(ms);
> - pnum_begin = pnum;
> - break;
> - }
> - map_count = 1;
> - for (pnum = pnum_begin + 1; pnum < NR_MEM_SECTIONS; pnum++) {
> - struct mem_section *ms;
> - int nodeid;
> -
> - if (!present_section_nr(pnum))
> - continue;
> - ms = __nr_to_section(pnum);
> - nodeid = sparse_early_nid(ms);
> - if (nodeid == nodeid_begin) {
> - map_count++;
> - continue;
> - }
> - /* ok, we need to take cake of from pnum_begin to pnum - 1*/
> - sparse_early_mem_maps_alloc_node(map_map, pnum_begin, pnum,
> - map_count, nodeid_begin);
> - /* new start, update count etc*/
> - nodeid_begin = nodeid;
> - pnum_begin = pnum;
> - map_count = 1;
> - }
> - /* ok, last chunk */
> - sparse_early_mem_maps_alloc_node(map_map, pnum_begin, NR_MEM_SECTIONS,
> - map_count, nodeid_begin);
> + alloc_usemap_and_memmap((unsigned long **)map_map, false);
> #endif
>
> for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
> --
> 1.8.1.2
On Tue, Aug 20, 2013 at 4:07 PM, Andrew Morton
<[email protected]> wrote:
> On Tue, 20 Aug 2013 14:54:54 +0800 Wanpeng Li <[email protected]> wrote:
>
>> v1 -> v2:
>> * add comments to describe alloc_usemap_and_memmap
>>
>> After commit 9bdac91424075("sparsemem: Put mem map for one node together."),
>> vmemmap for one node will be allocated together, its logic is similar as
>> memory allocation for pageblock flags. This patch introduce alloc_usemap_and_memmap
>> to extract the same logic of memory alloction for pageblock flags and vmemmap.
>>
>
> 9bdac91424075 was written by Yinghai. He is an excellent reviewer, as
> long as people remember to cc him!
could be that he forgot to use scripts/get_maintainer.pl
or get_maintainer.pl has some problem.
>
>> ---
>> mm/sparse.c | 140 ++++++++++++++++++++++++++++--------------------------------
>> 1 file changed, 66 insertions(+), 74 deletions(-)
>>
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index 308d503..d27db9b 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -439,6 +439,14 @@ static void __init sparse_early_mem_maps_alloc_node(struct page **map_map,
>> map_count, 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)
>> +{
>> +}
>> +
...
could be avoided, if passing function pointer instead.
>> static struct page __init *sparse_early_mem_map_alloc(unsigned long pnum)
>> {
>> struct page *map;
>> @@ -460,6 +468,62 @@ 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)
...
>> @@ -471,11 +535,7 @@ void __init sparse_init(void)
>> unsigned long *usemap;
>> unsigned long **usemap_map;
>> int size;
...
>> - /* ok, last chunk */
>> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
>> - usemap_count, nodeid_begin);
>> + alloc_usemap_and_memmap(usemap_map, true);
alloc_usemap_and_memmap() is somehow confusing.
Please check if you can pass function pointer instead of true/false.
Thanks
Yinghai
On Wed, 21 Aug 2013 07:39:35 +0800 Wanpeng Li <[email protected]> wrote:
> >Nope. If the error path is taken, free_pmds() will free uninitialised
> >items from pmds[], which is a local in pgd_alloc() and contains random
> >stack junk. The kernel will crash.
> >
> >You could pass an nr_pmds argument to free_pmds(), or zero out the
> >remaining items on the error path. However, although the current code
> >is a bit kooky, I don't see that it is harmful in any way.
> >
>
> There is a check in free_pmds():
>
> if (pmds[i])
> free_page((unsigned long)pmds[i]);
>
> which will avoid the issue you mentioned.
pmds[i] is uninitialized. It gets allocated
on the stack in pgd_alloc() and does not get zeroed.
On Tue, Aug 20, 2013 at 8:11 PM, Wanpeng Li <[email protected]> wrote:
> Hi Yinghai,
> On Tue, Aug 20, 2013 at 05:02:17PM -0700, Yinghai Lu wrote:
>>>> - /* ok, last chunk */
>>>> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
>>>> - usemap_count, nodeid_begin);
>>>> + alloc_usemap_and_memmap(usemap_map, true);
>>
>>alloc_usemap_and_memmap() is somehow confusing.
>>
>>Please check if you can pass function pointer instead of true/false.
>>
>
> sparse_early_usemaps_alloc_node and sparse_early_mem_maps_alloc_node is
> similar, however, one has a parameter unsigned long ** and the other has
> struct page **. function pointer can't help, isn't it? ;-)
you could have one generic function pointer like
void *alloc_func(void *data);
and in the every alloc function, have own struct data to pass in/out...
Yinghai
On Wed, Aug 21, 2013 at 12:29 AM, Wanpeng Li <[email protected]> wrote:
> Hi Yinghai,
> On Tue, Aug 20, 2013 at 09:28:29PM -0700, Yinghai Lu wrote:
>>On Tue, Aug 20, 2013 at 8:11 PM, Wanpeng Li <[email protected]> wrote:
>>> Hi Yinghai,
>>> On Tue, Aug 20, 2013 at 05:02:17PM -0700, Yinghai Lu wrote:
>>>>>> - /* ok, last chunk */
>>>>>> - sparse_early_usemaps_alloc_node(usemap_map, pnum_begin, NR_MEM_SECTIONS,
>>>>>> - usemap_count, nodeid_begin);
>>>>>> + alloc_usemap_and_memmap(usemap_map, true);
>>>>
>>>>alloc_usemap_and_memmap() is somehow confusing.
>>>>
>>>>Please check if you can pass function pointer instead of true/false.
>>>>
>>>
>>> sparse_early_usemaps_alloc_node and sparse_early_mem_maps_alloc_node is
>>> similar, however, one has a parameter unsigned long ** and the other has
>>> struct page **. function pointer can't help, isn't it? ;-)
>>
>>you could have one generic function pointer like
>>void *alloc_func(void *data);
>>
>>and in the every alloc function, have own struct data to pass in/out...
>>
>>Yinghai
>
> How about this?
>From a78e12a9ff31f2a73b87145ce7ad943a0f712708 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <[email protected]>
Date: Wed, 21 Aug 2013 15:23:08 +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 <[email protected]>
---
mm/sparse.c | 54 +++++++++++++++++++++++++-----------------------------
1 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/mm/sparse.c b/mm/sparse.c
index 55e5752..06adf3c 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -339,14 +339,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,
+static void __init sparse_early_usemaps_alloc_node(void **usemap_map,
unsigned long pnum_begin,
unsigned long pnum_end,
unsigned long usemap_count, int nodeid)
{
void *usemap;
unsigned long pnum;
+ unsigned long **map;
int size = usemap_size();
+ map = (unsigned long **) usemap_map;
usemap = sparse_early_usemaps_alloc_pgdat_section(NODE_DATA(nodeid),
size * usemap_count);
@@ -358,9 +360,9 @@ static void __init
sparse_early_usemaps_alloc_node(unsigned long**usemap_map,
for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
if (!present_section_nr(pnum))
continue;
- usemap_map[pnum] = usemap;
+ map[pnum] = usemap;
usemap += size;
- check_usemap_section_nr(nodeid, usemap_map[pnum]);
+ check_usemap_section_nr(nodeid, map[pnum]);
}
}
@@ -430,23 +432,16 @@ 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,
+static void __init sparse_early_mem_maps_alloc_node(void **map_map,
unsigned long pnum_begin,
unsigned long pnum_end,
unsigned long map_count, int nodeid)
{
- sparse_mem_maps_populate_node(map_map, pnum_begin, pnum_end,
+ struct page **map = (struct page **)map_map;
+ sparse_mem_maps_populate_node(map, pnum_begin, pnum_end,
map_count, nodeid);
}
======> maybe you can have:
struct alloc_info {
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)
{
struct alloc_info *info = (struct alloc_info *)data;
sparse_mem_maps_populate_node(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,9 +466,10 @@ 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 **, unsigned long, unsigned long,
+ unsigned long, int), void **map)
{
unsigned long pnum;
unsigned long map_count;
@@ -504,24 +500,16 @@ static void alloc_usemap_and_memmap(unsigned
long **map, bool use_map)
continue;
}
/* 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)(map, pnum_begin, pnum,
+ map_count, nodeid_begin);
========> can you use sparse_early_maps_alloc_node() ?
/* 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);
+ (*sparse_early_maps_alloc_node)(map, pnum_begin, NR_MEM_SECTIONS,
+ map_count, nodeid_begin);
}
/*
@@ -540,6 +528,10 @@ void __init sparse_init(void)
struct page **map_map;
#endif
+ void (*sparse_early_maps_alloc_node)(void **map,
+ unsigned long pnum_begin, unsigned long pnum_end,
+ unsigned long map_count, int nodeid);
+
/* see include/linux/mmzone.h 'struct mem_section' definition */
BUILD_BUG_ON(!is_power_of_2(sizeof(struct mem_section)));
@@ -561,14 +553,18 @@ 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;
why do you need to assign again?
+ alloc_usemap_and_memmap(sparse_early_maps_alloc_node,
+ (void **)usemap_map);
#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;
+ alloc_usemap_and_memmap(sparse_early_maps_alloc_node,
+ (void **)map_map);
#endif
for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
--
1.7.7.6
On Thu, Aug 22, 2013 at 5:14 AM, Wanpeng Li <[email protected]> wrote:
>From f21640b6dc15c76ac10fccada96e6b9fdce5a092 Mon Sep 17 00:00:00 2001
From: Wanpeng Li <[email protected]>
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 <[email protected]>
---
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
On Wed, Aug 28, 2013 at 7:18 PM, Yinghai Lu <[email protected]> wrote:
> 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.
looks like that is what is your first version did.
I updated it a little bit. please check it.
Thanks
Yinghai
On Wed, Aug 28, 2013 at 7:34 PM, Yinghai Lu <[email protected]> wrote:
> On Wed, Aug 28, 2013 at 7:18 PM, Yinghai Lu <[email protected]> wrote:
>> 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.
>
> looks like that is what is your first version did.
>
> I updated it a little bit. please check it.
>
removed more lines.
Yinghai
On Wed, Aug 28, 2013 at 7:51 PM, Wanpeng Li <[email protected]> wrote:
> Hi Yinghai,
>>> looks like that is what is your first version did.
>>>
>>> I updated it a little bit. please check it.
>>>
>>
>>removed more lines.
>
> Thanks for your great work!
>
> The fixed patch looks good to me. If this is the last fix and I can
> ignore http://marc.info/?l=linux-mm&m=137774271220239&w=2?
Yes, you can ignore that.
Yinghai