Re: [Patch v4 09/12] memory-hotplug: remove page table of x86_64 architecture

Jianguo Wu
2012-12-07T07:06:16Z

On 2012/12/7 14:43, Tang Chen wrote:

> On 11/27/2012 06:00 PM, Wen Congyang wrote:
>> For hot removing memory, we sholud remove page table about the memory.
>> So the patch searches a page table about the removed memory, and clear
>> page table.
>
> (snip)
>
>> +void __meminit
>> +kernel_physical_mapping_remove(unsigned long start, unsigned long end)
>> +{
>> + unsigned long next;
>> + bool pgd_changed = false;
>> +
>> + start = (unsigned long)__va(start);
>> + end = (unsigned long)__va(end);
>
> Hi Wu,
>
> Here, you expect start and end are physical addresses. But in
> phys_xxx_remove() function, I think using virtual addresses is just
> fine. Functions like pmd_addr_end() and pud_index() only calculate
> an offset.
>

Hi Tang,

Virtual addresses will work fine, I used physical addresses in order to
keep consistent with phys_pud[pmd/pte]_init(), So I think we should keep this.

Thanks,
Jianguo Wu

> So, would you please tell me if we have to use physical addresses here ?
>
> Thanks. :)
>
>> +
>> + for (; start< end; start = next) {
>> + pgd_t *pgd = pgd_offset_k(start);
>> + pud_t *pud;
>> +
>> + next = pgd_addr_end(start, end);
>> +
>> + if (!pgd_present(*pgd))
>> + continue;
>> +
>> + pud = map_low_page((pud_t *)pgd_page_vaddr(*pgd));
>> + phys_pud_remove(pud, __pa(start), __pa(next));
>> + if (free_pud_table(pud, pgd))
>> + pgd_changed = true;
>> + unmap_low_page(pud);
>> + }
>> +
>> + if (pgd_changed)
>> + sync_global_pgds(start, end - 1);
>> +
>> + flush_tlb_all();
>> +}
>> +
>> #ifdef CONFIG_MEMORY_HOTREMOVE
>> int __ref arch_remove_memory(u64 start, u64 size)
>> {
>> @@ -692,6 +921,8 @@ int __ref arch_remove_memory(u64 start, u64 size)
>> ret = __remove_pages(zone, start_pfn, nr_pages);
>> WARN_ON_ONCE(ret);
>>
>> + kernel_physical_mapping_remove(start, start + size);
>> +
>> return ret;
>> }
>> #endif
>
>
>
> .
>