Messages in this thread | | | Date | Fri, 21 Mar 2008 18:41:59 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH] [7/7] CPA: Add statistics about state of direct mapping v2 |
| |
On Wed, 12 Mar 2008, Andi Kleen wrote: > Add information about the mapping state of the direct mapping to > /proc/meminfo.
Please use debugfs for this.
> This way we can see how many large pages are really used for it and how > many are split. > > Useful for debugging and general insight into the kernel. > > v2: Add hotplug locking to 64bit to plug a very obscure theoretical race. > 32bit doesn't need it because it doesn't support hotadd for lowmem. > Fix some typos > > Signed-off-by: Andi Kleen <ak@suse.de> > > --- > arch/x86/mm/init_32.c | 2 ++ > arch/x86/mm/init_64.c | 2 ++ > arch/x86/mm/pageattr.c | 24 ++++++++++++++++++++++++ > fs/proc/proc_misc.c | 7 +++++++ > include/asm-x86/pgtable.h | 3 +++ > 5 files changed, 38 insertions(+) > > Index: linux/arch/x86/mm/init_64.c > =================================================================== > --- linux.orig/arch/x86/mm/init_64.c > +++ linux/arch/x86/mm/init_64.c > @@ -319,6 +319,8 @@ __meminit void early_iounmap(void *addr, > static unsigned long __meminit > phys_pmd_init(pmd_t *pmd_page, unsigned long address, unsigned long end) > { > + unsigned long flags; > + unsigned pages = 0;
Can we use unsigned long for both please and safe one line ?
> int i = pmd_index(address); > > for (; i < PTRS_PER_PMD; i++, address += PMD_SIZE) { > @@ -335,9 +337,15 @@ phys_pmd_init(pmd_t *pmd_page, unsigned > if (pmd_val(*pmd)) > continue; > > + pages++; > set_pte((pte_t *)pmd, > pfn_pte(address >> PAGE_SHIFT, PAGE_KERNEL_LARGE)); > } > + > + /* Protect against CPA */ > + spin_lock_irqsave(&pgd_lock, flags); > + dpages_cnt[PG_LEVEL_2M] += pages; > + spin_unlock_irqrestore(&pgd_lock, flags);
Please make the update a debugfs conditional function in the CPA code. That way it can be compile out and the statistic internals are not scattered all over the place.
> @@ -356,6 +364,8 @@ phys_pmd_update(pud_t *pud, unsigned lon > static unsigned long __meminit > phys_pud_init(pud_t *pud_page, unsigned long addr, unsigned long end) > { > + unsigned long flags; > + unsigned pages = 0; > unsigned long true_end = end;
See above.
> int i = pud_index(addr); > > @@ -380,6 +390,7 @@ phys_pud_init(pud_t *pud_page, unsigned > } > > if (direct_gbpages) { > + dpages_cnt[PG_LEVEL_1G]++; > set_pte((pte_t *)pud, > pfn_pte(addr >> PAGE_SHIFT, PAGE_KERNEL_LARGE)); > true_end = (addr & PUD_MASK) + PUD_SIZE; > @@ -397,6 +408,11 @@ phys_pud_init(pud_t *pud_page, unsigned > } > __flush_tlb_all(); > > + /* Protect against CPA */ > + spin_lock_irqsave(&pgd_lock, flags); > + dpages_cnt[PG_LEVEL_1G] += pages; > + spin_unlock_irqrestore(&pgd_lock, flags); > +
See above
> return true_end >> PAGE_SHIFT; > } > > Index: linux/arch/x86/mm/pageattr.c > =================================================================== > --- linux.orig/arch/x86/mm/pageattr.c > +++ linux/arch/x86/mm/pageattr.c > @@ -18,6 +18,8 @@ > #include <asm/pgalloc.h> > #include <asm/proto.h> > > +unsigned long dpages_cnt[PG_LEVEL_NUM];
Can we have some intuitive name for that like direct_pages_stats, so it's clear that it is debug/statistics info ?
> /* > * The current flushing context - we pass it instead of 5 arguments: > */ > @@ -499,6 +501,12 @@ static int split_large_page(pte_t *kpte, > for (i = 0; i < PTRS_PER_PTE; i++, pfn += pfninc) > set_pte(&pbase[i], pfn_pte(pfn, ref_prot)); > > + if (address >= (unsigned long)__va(0) && > + address < (unsigned long)__va(end_pfn_map << PAGE_SHIFT)) { > + dpages_cnt[level]--; > + dpages_cnt[level - 1] += PTRS_PER_PTE;
inline conditional on DEBUGFS please
> + } > + > /* > * Install the new, split up pagetable. Important details here: > * > @@ -948,6 +956,22 @@ bool kernel_page_present(struct page *pa > > #endif /* CONFIG_DEBUG_PAGEALLOC */ > > +#ifdef CONFIG_PROC_FS
debugfs please
Thanks,
tglx
| |