Re: [PATCH 8/10] memory-hotplug : remove page table of x86_64 architecture

3 messages, 3 authors, 2012-10-11 · open the first message on its own page

Re: [PATCH 8/10] memory-hotplug : remove page table of x86_64 architecture

From: Andi Kleen <hidden>
Date: 2012-10-08 04:37:10

Yasuaki Ishimatsu [off-list ref] writes:
+			}
+
+			/*
+			 * We use 2M page, but we need to remove part of them,
+			 * so split 2M page to 4K page.
+			 */
+			pte = alloc_low_page(&pte_phys);
What happens when the allocation fails?

alloc_low_page seems to be buggy there too, it would __pa a NULL 
pointer.
+		if (pud_large(*pud)) {
+			if ((addr & ~PUD_MASK) == 0 && next <= end) {
+				set_pud(pud, __pud(0));
+				pages++;
+				continue;
+			}
+
+			/*
+			 * We use 1G page, but we need to remove part of them,
+			 * so split 1G page to 2M page.
+			 */
+			pmd = alloc_low_page(&pmd_phys);
Same here
+			__split_large_page((pte_t *)pud, addr, (pte_t *)pmd);
+
+			spin_lock(&init_mm.page_table_lock);
+			pud_populate(&init_mm, pud, __va(pmd_phys));
+			spin_unlock(&init_mm.page_table_lock);
+		}
+
+		pmd = map_low_page(pmd_offset(pud, 0));
+		phys_pmd_remove(pmd, addr, end);
+		unmap_low_page(pmd);
+		__flush_tlb_all();
+	}
+	__flush_tlb_all();
This doesn't flush the other CPUs doesn't it?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH 8/10] memory-hotplug : remove page table of x86_64 architecture

From: Wen Congyang <hidden>
Date: 2012-10-08 05:18:05

At 10/08/2012 12:37 PM, Andi Kleen Wrote:
Yasuaki Ishimatsu [off-list ref] writes:
quoted
+			}
+
+			/*
+			 * We use 2M page, but we need to remove part of them,
+			 * so split 2M page to 4K page.
+			 */
+			pte = alloc_low_page(&pte_phys);
What happens when the allocation fails?

alloc_low_page seems to be buggy there too, it would __pa a NULL 
pointer.
Yes, it will cause kernek panicked in __pa() if CONFI_DEBUG_VIRTUAL is set.
Otherwise, it will return a NULL pointer. I will update this patch to deal
with NULL pointer.
quoted
+		if (pud_large(*pud)) {
+			if ((addr & ~PUD_MASK) == 0 && next <= end) {
+				set_pud(pud, __pud(0));
+				pages++;
+				continue;
+			}
+
+			/*
+			 * We use 1G page, but we need to remove part of them,
+			 * so split 1G page to 2M page.
+			 */
+			pmd = alloc_low_page(&pmd_phys);
Same here
quoted
+			__split_large_page((pte_t *)pud, addr, (pte_t *)pmd);
+
+			spin_lock(&init_mm.page_table_lock);
+			pud_populate(&init_mm, pud, __va(pmd_phys));
+			spin_unlock(&init_mm.page_table_lock);
+		}
+
+		pmd = map_low_page(pmd_offset(pud, 0));
+		phys_pmd_remove(pmd, addr, end);
+		unmap_low_page(pmd);
+		__flush_tlb_all();
+	}
+	__flush_tlb_all();
This doesn't flush the other CPUs doesn't it?
How to flush the other CPU's tlb? use on_each_cpu() to run __flush_tlb_all()
on each online cpu?

Thanks
Wen Congyang
-Andi

Re: [PATCH 8/10] memory-hotplug : remove page table of x86_64 architecture

From: Ni zhan Chen <hidden>
Date: 2012-10-11 00:36:06

On 10/08/2012 01:23 PM, Wen Congyang wrote:
At 10/08/2012 12:37 PM, Andi Kleen Wrote:
quoted
Yasuaki Ishimatsu [off-list ref] writes:
quoted
+			}
+
+			/*
+			 * We use 2M page, but we need to remove part of them,
+			 * so split 2M page to 4K page.
+			 */
+			pte = alloc_low_page(&pte_phys);
What happens when the allocation fails?

alloc_low_page seems to be buggy there too, it would __pa a NULL
pointer.
Yes, it will cause kernek panicked in __pa() if CONFI_DEBUG_VIRTUAL is set.
Otherwise, it will return a NULL pointer. I will update this patch to deal
with NULL pointer.
quoted
quoted
+		if (pud_large(*pud)) {
+			if ((addr & ~PUD_MASK) == 0 && next <= end) {
+				set_pud(pud, __pud(0));
+				pages++;
+				continue;
+			}
+
+			/*
+			 * We use 1G page, but we need to remove part of them,
+			 * so split 1G page to 2M page.
+			 */
+			pmd = alloc_low_page(&pmd_phys);
Same here
quoted
+			__split_large_page((pte_t *)pud, addr, (pte_t *)pmd);
+
+			spin_lock(&init_mm.page_table_lock);
+			pud_populate(&init_mm, pud, __va(pmd_phys));
+			spin_unlock(&init_mm.page_table_lock);
+		}
+
+		pmd = map_low_page(pmd_offset(pud, 0));
+		phys_pmd_remove(pmd, addr, end);
+		unmap_low_page(pmd);
+		__flush_tlb_all();
+	}
+	__flush_tlb_all();
Hi Congyang,

I see you call __flush_tlb_all() every pud entry(all pmd, pte related to 
it changed) modified, then how to determine the flush frequency? why not 
every pmd entry?

Regards,
Chen
quoted
This doesn't flush the other CPUs doesn't it?
How to flush the other CPU's tlb? use on_each_cpu() to run __flush_tlb_all()
on each online cpu?

Thanks
Wen Congyang
quoted
-Andi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help