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:3.175 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site