lkml.org 
[lkml]   [2013]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v3 07/13] x86, numa, mem-hotplug: Mark nodes which the kernel resides in.
Hi Vasilis,

On 06/01/2013 12:24 AM, Vasilis Liaskovitis wrote:
......
>> +void __init_memblock memblock_mark_kernel_nodes()
>> +{
>> + int i, nid;
>> + struct memblock_type *reserved =&memblock.reserved;
>> +
>> + for (i = 0; i< reserved->cnt; i++)
>> + if (reserved->regions[i].flags == MEMBLK_FLAGS_DEFAULT) {
>> + nid = memblock_get_region_node(&reserved->regions[i]);
>> + node_set(nid, memblock_kernel_nodemask);
>> + }
>> +}
>
> I think there is a problem here because memblock_set_region_node is sometimes
> called with nid == MAX_NUMNODES. This means the correct node is not properly
> masked in the memblock_kernel_nodemask bitmap.
> E.g. in a VM test, memblock_mark_kernel_nodes with extra pr_warn calls iterates
> over the following memblocks (ranges below are memblks base-(base+size)):
>
> [ 0.000000] memblock_mark_kernel_nodes nid=64 0x00000000000000-0x00000000010000
> [ 0.000000] memblock_mark_kernel_nodes nid=64 0x00000000098000-0x00000000100000
> [ 0.000000] memblock_mark_kernel_nodes nid=64 0x00000001000000-0x00000001a5a000
> [ 0.000000] memblock_mark_kernel_nodes nid=64 0x00000037000000-0x000000377f8000
>
> where MAX_NUMNODES is 64 because CONFIG_NODES_SHIFT=6.
> The ranges above belong to node 0, but the node's bit is never marked.
>
> With a buggy bios that marks all memory as hotpluggable, this results in a
> panic, because both checks against hotpluggable bit and memblock_kernel_bitmask
> (in early_mem_hotplug_init) fail, the numa regions have all been merged together
> and memblock_reserve_hotpluggable is called for all memory.
>
> With a correct bios (some part of initial memory is not hotplug-able) the kernel
> can boot since the hotpluggable bit check works ok, but extra dimms on node 0
> will still be allowed to be in MOVABLE_ZONE.
>

OK, I see the problem. But would you please give me a call trace that
can show
how this could happen. I think the memory block info should be the same as
numa_meminfo. Can we fix the caller to make it set nid correctly ?

> Actually this behaviour (being able to have MOVABLE memory on nodes with kernel
> reserved memblocks) sort of matches the policy I requested in v2 :). But i
> suspect that is not your intent i.e. you want memblock_kernel_nodemask_bitmap to
> prevent movable reservations for the whole node where kernel has reserved
> memblocks.

I intended to set the whole node which the kernel resides in as
un-hotpluggable.

>
> Is there a way to get accurate nid information for memblocks at early boot? I
> suspect pfn_to_nid doesn't work yet at this stage (i got a panic when I
> attempted iirc)

In such an early time, I think we can only get nid from numa_meminfo. So
as I
said above, I'd like to fix this problem by making memblock has correct nid.

And I read the patch below. I think if we get nid from numa_meminfo,
than we
don't need to call memblock_get_region_node().

Thanks. :)

>
> I used the hack below but it depends on CONFIG_NUMA, hopefully there is a
> cleaner general way:
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index cfd8c2f..af8ad2a 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -133,6 +133,19 @@ void __init setup_node_to_cpumask_map(void)
> pr_debug("Node to cpumask map for %d nodes\n", nr_node_ids);
> }
>
> +int __init numa_find_range_nid(u64 start, u64 size)
> +{
> + unsigned int i;
> + struct numa_meminfo *mi =&numa_meminfo;
> +
> + for (i = 0; i< mi->nr_blks; i++) {
> + if (start>= mi->blk[i].start&& start + size -1<= mi->blk[i].end)
> + return mi->blk[i].nid;
> + }
> + return -1;
> +}
> +EXPORT_SYMBOL(numa_find_range_nid);
> +
> static int __init numa_add_memblk_to(int nid, u64 start, u64 end,
> bool hotpluggable,
> struct numa_meminfo *mi)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 77a71fb..194b7c7 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1600,6 +1600,9 @@ unsigned long change_prot_numa(struct vm_area_struct *vma,
> unsigned long start, unsigned long end);
> #endif
>
> +#ifdef CONFIG_NUMA
> +int __init numa_find_range_nid(u64 start, u64 size);
> +#endif
> struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
> int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
> unsigned long pfn, unsigned long size, pgprot_t);
> diff --git a/mm/memblock.c b/mm/memblock.c
> index a6b7845..284aced 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -834,15 +834,26 @@ int __init_memblock memblock_set_node(phys_addr_t base, phys_addr_t size,
>
> void __init_memblock memblock_mark_kernel_nodes()
> {
> - int i, nid;
> + int i, nid, tmpnid;
> struct memblock_type *reserved =&memblock.reserved;
>
> for (i = 0; i< reserved->cnt; i++)
> if (reserved->regions[i].flags == MEMBLK_FLAGS_DEFAULT) {
> nid = memblock_get_region_node(&reserved->regions[i]);
> + if (nid == MAX_NUMNODES) {
> + tmpnid = numa_find_range_nid(reserved->regions[i].base,
> + reserved->regions[i].size);
> + if (tmpnid>= 0)
> + nid = tmpnid;
> + }
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index e862311..84d6e64 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -667,11 +667,7 @@ static bool srat_used __initdata;
> */
> static void __init early_x86_numa_init(void)
> {
> - /*
> - * Need to find out which nodes the kernel resides in, and arrange
> - * them as un-hotpluggable when parsing SRAT.
> - */
> - memblock_mark_kernel_nodes();
>
> if (!numa_off) {
> #ifdef CONFIG_X86_NUMAQ
> @@ -779,6 +775,12 @@ void __init early_initmem_init(void)
> load_cr3(swapper_pg_dir);
> __flush_tlb_all();
>
> + /*
> + * Need to find out which nodes the kernel resides in, and arrange
> + * them as un-hotpluggable when parsing SRAT.
> + */
> +
> + memblock_mark_kernel_nodes();
> early_mem_hotplug_init();
>
> early_memtest(0, max_pfn_mapped<<PAGE_SHIFT);


\
 
 \ /
  Last update: 2013-06-03 10:01    [W:0.092 / U:0.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site