Messages in this thread Patch in this message | | | Date | Wed, 6 May 2015 16:30:16 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 2/3] mm/memblock: Allocate boot time data structures from mirrored memory |
| |
On Tue, 3 Feb 2015 14:38:02 -0800 Tony Luck <tony.luck@intel.com> wrote:
> Try to allocate all boot time kernel data structures from mirrored > memory. If we run out of mirrored memory print warnings, but fall > back to using non-mirrored memory to make sure that we still boot. > > ... > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h > index 1d448879caae..20bf3dfab564 100644 > --- a/include/linux/memblock.h > +++ b/include/linux/memblock.h > @@ -22,6 +22,7 @@ > > /* Definition of memblock flags. */ > #define MEMBLOCK_HOTPLUG 0x1 /* hotpluggable region */ > +#define MEMBLOCK_MIRROR 0x2 /* mirrored region */
It would be nice to make these an enum. Then all those literal "0"'s which were added in [1/3] become MEMBLOCK_NONE, which is self-documenting.
> > ... > > +static inline bool memblock_is_mirror(struct memblock_region *m) > +{ > + return m->flags & MEMBLOCK_MIRROR; > +} > + > > ... > > +u32 __init_memblock memblock_has_mirror(void) > +{ > + return memblock_have_mirror ? MEMBLOCK_MIRROR : 0; > +}
hm, these are very similar. But I guess they're different enough.
Gramatically, a function called "memblock_has_mirror()" should return a bool. This guy is misnamed. "memblock_mirror_flag()"?
> /* inline so we don't get a warning when pr_debug is compiled out */ > static __init_memblock const char * > memblock_type_name(struct memblock_type *type) > @@ -257,8 +263,19 @@ phys_addr_t __init_memblock memblock_find_in_range(phys_addr_t start, > phys_addr_t end, phys_addr_t size, > phys_addr_t align) > { > - return memblock_find_in_range_node(size, align, start, end, > + phys_addr_t ret; > + u32 flag = memblock_has_mirror(); > + > + ret = memblock_find_in_range_node(size, align, start, end, > + NUMA_NO_NODE, flag); > + > + if (!ret && flag) { > + pr_warn("Could not allocate %lld bytes of mirrored memory\n", size);
This printk will warn on some configs. Print a phys_addr_t with %pap. I think. See huge comment over lib/vsprintf.c:pointer(). There are other instances of this.
> + ret = memblock_find_in_range_node(size, align, start, end, > NUMA_NO_NODE, 0); > + } > + > + return ret; > } > > ... > > phys_addr_t __init memblock_alloc_nid(phys_addr_t size, phys_addr_t align, int nid) > { > - return memblock_alloc_base_nid(size, align, MEMBLOCK_ALLOC_ACCESSIBLE, nid, 0); > + u32 flag = memblock_has_mirror(); > + phys_addr_t ret; > + > +again: > + ret = memblock_alloc_base_nid(size, align, MEMBLOCK_ALLOC_ACCESSIBLE, nid, flag); > + > + if (!ret && flag) { > + flag = 0; > + goto again; > + }
What's going on here? This is where we're falling back to non-mirrored. But it's happening silently? Should it warn, or is that handled elsewhere?
This function isn't specific to mirrored memory - for any future flags, falling back to flags==0 may not be the desired behavior. What do we do then? I guess
if (!ret && (flag & MEMBLOCK_MIRROR)) ( flag &= ~MEMBLOCK_MIRROR; goto again;
yes?
That can be done later if needed, I suppose.
> + return ret; > } > > > ... > > @@ -1181,13 +1232,13 @@ static void * __init memblock_virt_alloc_internal( > > again: > alloc = memblock_find_in_range_node(size, align, min_addr, max_addr, > - nid, 0); > + nid, flag); > if (alloc) > goto done; > > if (nid != NUMA_NO_NODE) { > alloc = memblock_find_in_range_node(size, align, min_addr, > - max_addr, NUMA_NO_NODE, 0); > + max_addr, NUMA_NO_NODE, flag); > if (alloc) > goto done; > } > @@ -1195,10 +1246,15 @@ again: > if (min_addr) { > min_addr = 0; > goto again; > - } else { > - goto error; > } > > + if (flag) { > + flag = 0; > + pr_warn("Could not allocate %lld bytes of mirrored memory\n", size);
printk warning.
Please don't torture people who use 80-col displays!
> + goto again; > + } > + > + return NULL; > > ... > > @@ -37,11 +37,19 @@ static void * __init __alloc_memory_core_early(int nid, u64 size, u64 align, > { > void *ptr; > u64 addr; > + u32 flag = memblock_has_mirror(); > > if (limit > memblock.current_limit) > limit = memblock.current_limit; > > - addr = memblock_find_in_range_node(size, align, goal, limit, nid, 0); > +again: > + addr = memblock_find_in_range_node(size, align, goal, limit, nid, flag); > + > + if (flag && !addr) { > + flag = 0; > + pr_warn("Could not allocate %lld bytes of mirrored memory\n", size);
dittoes.
> + goto again; > + } > if (!addr) > return NULL; >
| |