lkml.org 
[lkml]   [2006]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch] i386: pageattr remove __put_page
    On Wed, Jan 18, 2006 at 07:00:28PM -0800, Andrew Morton wrote:
    > Nick Piggin <npiggin@suse.de> wrote:
    > >
    > > Stop using __put_page and page_count in i386 pageattr.c
    > >
    >
    > who, where, what, why, when?? The patch appears to ascribe some special
    > significance to page->private, but you don't tell us what it is. And if
    > that's not obvious from reading the patch, it won't be obvious to people
    > who are later reading the code.
    >
    > iow: you owe us a nice comment, please.
    >

    OK sure, I'll redo it.

    > >
    > > Index: linux-2.6/arch/i386/mm/pageattr.c
    > > ===================================================================
    > > --- linux-2.6.orig/arch/i386/mm/pageattr.c
    > > +++ linux-2.6/arch/i386/mm/pageattr.c
    > > @@ -51,6 +51,9 @@ static struct page *split_large_page(uns
    > > if (!base)
    > > return NULL;
    > >
    > > + SetPagePrivate(base);
    > > + page_private(base) = 0;
    >
    > A "function call" as an lval give me hiccups. Use set_page_private(p, v),
    > please.
    >

    Hmm yes of course. That makes it pretty ugly.

    > > address = __pa(address);
    > > addr = address & LARGE_PAGE_MASK;
    > > pbase = (pte_t *)page_address(base);
    > > @@ -143,11 +146,12 @@ __change_page_attr(struct page *page, pg
    > > return -ENOMEM;
    > > set_pmd_pte(kpte,address,mk_pte(split, ref_prot));
    > > kpte_page = split;
    > > - }
    > > - get_page(kpte_page);
    > > + }
    > > + page_private(kpte_page)++;
    >
    > Ditto, really. If we're going to be nice about this it should be
    >
    > set_page_private(page, page_private(page) + 1);
    >
    > > + page_private(kpte_page)--;
    >
    > Ditto.
    >
    >
    > Or we just forget about page_private() and go back to using page->private -
    > page_private() was rather a stopgap thing.
    >

    Could we? We can do anonymous unions now, right?

    -
    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: 2006-01-19 15:15    [W:0.026 / U:0.304 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site