lkml.org 
[lkml]   [2005]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] Add pagetable allocation notifiers
* Zachary Amsden (zach@vmware.com) wrote:
> Chris Wright wrote:
>
> >* Zachary Amsden (zach@vmware.com) wrote:
> >
> >
> >>--- linux-2.6.13.orig/arch/i386/mm/init.c 2005-08-24
> >>09:31:05.000000000 -0700
> >>+++ linux-2.6.13/arch/i386/mm/init.c 2005-08-24 09:31:31.000000000 -0700
> >>@@ -79,6 +79,7 @@ static pte_t * __init one_page_table_ini
> >>{
> >> if (pmd_none(*pmd)) {
> >> pte_t *page_table = (pte_t *)
> >> alloc_bootmem_low_pages(PAGE_SIZE);
> >>+ SetPagePTE(virt_to_page(page_table));
> >>
> >>
> >
> >Xen has this on one_md_table_init() as well for pmd.
>
> I'll add that in another patch. It's easy to miss some of the init time
> call sites (we don't actually depend on them for correctness).

You could just respin this patch.

> >> spin_lock_irqsave(&pgd_lock, flags);
> >> pgd_list_del(pgd);
> >> spin_unlock_irqrestore(&pgd_lock, flags);
> >>@@ -244,13 +246,16 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
> >> pmd_t *pmd = kmem_cache_alloc(pmd_cache, GFP_KERNEL);
> >> if (!pmd)
> >> goto out_oom;
> >>+ SetPagePDE(virt_to_page(pmd));
> >> set_pgd(&pgd[i], __pgd(1 + __pa(pmd)));
> >> }
> >> return pgd;
> >>
> >>out_oom:
> >>- for (i--; i >= 0; i--)
> >>+ for (i--; i >= 0; i--) {
> >>+ ClearPagePDE(pfn_to_page(pgd_val(pgd[i]) >> PAGE_SHIFT));
> >>
> >>
> >
> >Is that the right pfn? That -1 throws me off.
> >
> >
> >
> >> kmem_cache_free(pmd_cache, (void *)__va(pgd_val(pgd[i])-1));
> >>+ }
> >> kmem_cache_free(pgd_cache, pgd);
> >> return NULL;
> >>}
> >>@@ -261,8 +266,10 @@ void pgd_free(pgd_t *pgd)
> >>
> >> /* in the PAE case user pgd entries are overwritten before usage */
> >> if (PTRS_PER_PMD > 1)
> >>- for (i = 0; i < USER_PTRS_PER_PGD; ++i)
> >>- kmem_cache_free(pmd_cache, (void
> >>*)__va(pgd_val(pgd[i])-1));
> >>+ for (i = 0; i < USER_PTRS_PER_PGD; ++i) {
> >>+ ClearPagePDE(pfn_to_page(pgd_val(pgd[i]) >>
> >>PAGE_SHIFT));
> >>+ kmem_cache_free(pmd_cache, (void
> >>*)__va(pgd_val(pgd[i]) & PAGE_MASK));
> >>
> >>
> >
> >Why the switch of kmem_cache_free call?
>
> Because pgd_val(pgd[i])-1 is confusing.

Yes, I agree it is. That's why I was wondering about the ClearPagePDE calls
which don't use them.

> Using (pgd_val(pgd[i]) -
> _PAGE_PRESENT) would be better, but the +/- 1s all over the place here
> could use some general cleanup as well. I smell a cleanup fit coming
> on. Using (pgd_val(pgd[i]) & PAGE_MASK) is a less error prone way to
> get the physical frame bits, since it is not wrong if you turn on PCD or
> PWD.

Please save all that for later cleanup. As it stands it's just a conversion
of one random call site, which should be dropped from the patch.

thanks,
-chris
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-08-24 23:21    [W:0.056 / U:0.364 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site