lkml.org 
[lkml]   [2010]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 6/7] hugetlb: hugepage migration core
    On Tue, Jul 06, 2010 at 11:00:27AM -0500, Christoph Lameter wrote:
    > On Fri, 2 Jul 2010, Naoya Horiguchi wrote:
    >
    > > --- v2.6.35-rc3-hwpoison/mm/migrate.c
    > > +++ v2.6.35-rc3-hwpoison/mm/migrate.c
    > > @@ -32,6 +32,7 @@
    > > #include <linux/security.h>
    > > #include <linux/memcontrol.h>
    > > #include <linux/syscalls.h>
    > > +#include <linux/hugetlb.h>
    > > #include <linux/gfp.h>
    > >
    > > #include "internal.h"
    > > @@ -74,6 +75,8 @@ void putback_lru_pages(struct list_head *l)
    > > struct page *page2;
    > >
    > > list_for_each_entry_safe(page, page2, l, lru) {
    > > + if (PageHuge(page))
    > > + break;
    > > list_del(&page->lru);
    >
    > Argh. Hugepages in putpack_lru_pages()? Huge pages are not on the lru.
    > Come up with something cleaner here.

    OK.

    > > @@ -267,7 +284,14 @@ static int migrate_page_move_mapping(struct address_space *mapping,
    > > * Note that anonymous pages are accounted for
    > > * via NR_FILE_PAGES and NR_ANON_PAGES if they
    > > * are mapped to swap space.
    > > + *
    > > + * Not account hugepage here for now because hugepage has
    > > + * separate accounting rule.
    > > */
    > > + if (PageHuge(newpage)) {
    > > + spin_unlock_irq(&mapping->tree_lock);
    > > + return 0;
    > > + }
    > > __dec_zone_page_state(page, NR_FILE_PAGES);
    > > __inc_zone_page_state(newpage, NR_FILE_PAGES);
    > > if (PageSwapBacked(page)) {
    >
    > This looks wrong here. Too many special casing added to basic migration
    > functionality.

    Agreed.
    As I replied in another email, I'll move to migrate_huge_page() and
    avoid adding ugly changes like this.

    > > @@ -284,7 +308,17 @@ static int migrate_page_move_mapping(struct address_space *mapping,
    > > */
    > > static void migrate_page_copy(struct page *newpage, struct page *page)
    > > {
    > > - copy_highpage(newpage, page);
    > > + int i;
    > > + struct hstate *h;
    > > + if (!PageHuge(newpage))
    > > + copy_highpage(newpage, page);
    > > + else {
    > > + h = page_hstate(newpage);
    > > + for (i = 0; i < pages_per_huge_page(h); i++) {
    > > + cond_resched();
    > > + copy_highpage(newpage + i, page + i);
    > > + }
    > > + }
    > >
    > > if (PageError(page))
    > > SetPageError(newpage);
    >
    > Could you generalize this for migrating an order N page?

    Yes.
    I'll define a helper function to handle order N case.

    > > @@ -718,6 +752,11 @@ unlock:
    > > put_page(page);
    > >
    > > if (rc != -EAGAIN) {
    > > + if (PageHuge(newpage)) {
    > > + put_page(newpage);
    > > + goto out;
    > > + }
    > > +
    >
    > I dont like this kind of inconsistency with the refcounting. Page
    > migration is complicated enough already.

    OK.

    Thanks,
    Naoya Horiguchi


    \
     
     \ /
      Last update: 2010-07-07 08:49    [W:0.044 / U:33.160 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site