On 2014/7/29 15:53, David Rientjes wrote:
> On Tue, 29 Jul 2014, Zhang Zhen wrote:
>
>> Commit ea0854170c95245a258b386c7a9314399c949fe0 added a fuction
>
> This would normally be written as
>
> Commit ea0854170c95 ("memory hotplug: fix a bug on /dev/mem for 64-bit
> kernels") ...
>
ok.
>> update_end_of_memory_vars() to update high_memory, max_pfn and
>> max_low_pfn.
>>
>> I modified the function according to Dave Hansen and David Rientjes's
>> suggestions.
>> And call it in arch_remove_memory() to update these variables too.
>>
>
> When the x86 maintainers merge this patch, they'll need to make a judgment
> call on how urgent the fix is and that will guide them in whether they
> want it backported to stable kernels as well. It would be useful to
> provide the rationale for the change; in other words, why is the change
> needed and what breaks if we don't have it?
Got it, here we may access wrong memory via /dev/mem without this patch.
I will provide the rationale for the change.
>
>> Change v1->v2:
>> - according to Dave Hansen and David Rientjes's suggestions modified
>> update_end_of_memory_vars().
>> Signed-off-by: Zhang Zhen <zhenzhang.zhang@xxxxxxxxxx>
>
> Acked-by: David Rientjes <rientjes@xxxxxxxxxx>
>
> You'll want to email all the x86 maintainers who would handle this patch,
> check the output of scripts/get_maintainer.pl when run on this diff.
Ok, Thanks!
>
>> ---
>> arch/x86/mm/init_64.c | 23 ++++++++++++++---------
>> 1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
>> index df1a992..fd7bd6b 100644
>> --- a/arch/x86/mm/init_64.c
>> +++ b/arch/x86/mm/init_64.c
>> @@ -673,15 +673,11 @@ void __init paging_init(void)
>> * After memory hotplug the variables max_pfn, max_low_pfn and high_memory need
>> * updating.
>> */
>> -static void update_end_of_memory_vars(u64 start, u64 size)
>> +static void update_end_of_memory_vars(u64 end_pfn)
>
> Extra space that can be removed here at the same time as a cleanup.
>
Sorry, where is the extra space here?
>> {
>> - unsigned long end_pfn = PFN_UP(start + size);
>> -
>> - if (end_pfn > max_pfn) {
>> - max_pfn = end_pfn;
>> - max_low_pfn = end_pfn;
>> - high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;
>> - }
>> + max_pfn = end_pfn;
>> + max_low_pfn = end_pfn;
>> + high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;
>> }
>>
>> /*
>> @@ -694,6 +690,7 @@ int arch_add_memory(int nid, u64 start, u64 size)
>> struct zone *zone = pgdat->node_zones + ZONE_NORMAL;
>> unsigned long start_pfn = start >> PAGE_SHIFT;
>> unsigned long nr_pages = size >> PAGE_SHIFT;
>> + unsigned long end_pfn;
>> int ret;
>>
>> init_memory_mapping(start, start + size);
>> @@ -702,7 +699,9 @@ int arch_add_memory(int nid, u64 start, u64 size)
>> WARN_ON_ONCE(ret);
>>
>> /* update max_pfn, max_low_pfn and high_memory */
>> - update_end_of_memory_vars(start, size);
>> + end_pfn = start_pfn + nr_pages;
>> + if (end_pfn > max_pfn)
>> + update_end_of_memory_vars(end_pfn);
>>
>> return ret;
>> }
>> @@ -1018,6 +1017,7 @@ int __ref arch_remove_memory(u64 start, u64 size)
>> unsigned long start_pfn = start >> PAGE_SHIFT;
>> unsigned long nr_pages = size >> PAGE_SHIFT;
>> struct zone *zone;
>> + unsigned long end_pfn;
>> int ret;
>>
>> zone = page_zone(pfn_to_page(start_pfn));
>> @@ -1025,6 +1025,11 @@ int __ref arch_remove_memory(u64 start, u64 size)
>> ret = __remove_pages(zone, start_pfn, nr_pages);
>> WARN_ON_ONCE(ret);
>>
>> + /* update max_pfn, max_low_pfn and high_memory */
>> + end_pfn = start_pfn + nr_pages;
>> + if ((max_pfn >= start_pfn) && (max_pfn < end_pfn))
>> + update_end_of_memory_vars(start_pfn);
>
> Not sure if we really need the new variable here; if you choose to
> repropose this patch then you may want to consider just using
> start_pfn + nr_pages in the conditional.
yeah, i will remove it in v3.
>
>> +
>> return ret;
>> }
>> #endif
>
> .
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>