lkml.org 
[lkml]   [2009]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH -mm] vmscan: make mapped executable pages the first class citizen
    On Fri, 8 May 2009 16:16:08 +0800
    Wu Fengguang <fengguang.wu@intel.com> wrote:

    > vmscan: make mapped executable pages the first class citizen
    >
    > Protect referenced PROT_EXEC mapped pages from being deactivated.
    >
    > PROT_EXEC(or its internal presentation VM_EXEC) pages normally belong to some
    > currently running executables and their linked libraries, they shall really be
    > cached aggressively to provide good user experiences.
    >

    The patch seems reasonable but the changelog and the (non-existent)
    design documentation could do with a touch-up.

    >
    > --- linux.orig/mm/vmscan.c
    > +++ linux/mm/vmscan.c
    > @@ -1233,6 +1233,7 @@ static void shrink_active_list(unsigned
    > unsigned long pgscanned;
    > unsigned long vm_flags;
    > LIST_HEAD(l_hold); /* The pages which were snipped off */
    > + LIST_HEAD(l_active);
    > LIST_HEAD(l_inactive);
    > struct page *page;
    > struct pagevec pvec;
    > @@ -1272,8 +1273,13 @@ static void shrink_active_list(unsigned
    >
    > /* page_referenced clears PageReferenced */
    > if (page_mapping_inuse(page) &&
    > - page_referenced(page, 0, sc->mem_cgroup, &vm_flags))
    > + page_referenced(page, 0, sc->mem_cgroup, &vm_flags)) {
    > pgmoved++;
    > + if ((vm_flags & VM_EXEC) && !PageAnon(page)) {
    > + list_add(&page->lru, &l_active);
    > + continue;
    > + }
    > + }

    What we're doing here is to identify referenced, file-backed active
    pages. We clear their referenced bit and give than another trip around
    the active list. So if they aren't referenced during that additional
    pass, they will get deactivated next time they are scanned, yes? It's
    a fairly high-level design/heuristic thing which needs careful
    commenting, please.


    Also, the change makes this comment:

    spin_lock_irq(&zone->lru_lock);
    /*
    * Count referenced pages from currently used mappings as
    * rotated, even though they are moved to the inactive list.
    * This helps balance scan pressure between file and anonymous
    * pages in get_scan_ratio.
    */
    reclaim_stat->recent_rotated[!!file] += pgmoved;

    inaccurate.

    > list_add(&page->lru, &l_inactive);
    > }
    > @@ -1282,7 +1288,6 @@ static void shrink_active_list(unsigned
    > * Move the pages to the [file or anon] inactive list.
    > */
    > pagevec_init(&pvec, 1);
    > - lru = LRU_BASE + file * LRU_FILE;
    >
    > spin_lock_irq(&zone->lru_lock);
    > /*
    > @@ -1294,6 +1299,7 @@ static void shrink_active_list(unsigned
    > reclaim_stat->recent_rotated[!!file] += pgmoved;
    >
    > pgmoved = 0; /* count pages moved to inactive list */
    > + lru = LRU_BASE + file * LRU_FILE;
    > while (!list_empty(&l_inactive)) {
    > page = lru_to_page(&l_inactive);
    > prefetchw_prev_lru_page(page, &l_inactive, flags);
    > @@ -1316,6 +1322,29 @@ static void shrink_active_list(unsigned
    > __mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);
    > __count_zone_vm_events(PGREFILL, zone, pgscanned);
    > __count_vm_events(PGDEACTIVATE, pgmoved);
    > +
    > + pgmoved = 0; /* count pages moved back to active list */
    > + lru = LRU_ACTIVE + file * LRU_FILE;
    > + while (!list_empty(&l_active)) {
    > + page = lru_to_page(&l_active);
    > + prefetchw_prev_lru_page(page, &l_active, flags);
    > + VM_BUG_ON(PageLRU(page));
    > + SetPageLRU(page);
    > + VM_BUG_ON(!PageActive(page));
    > +
    > + list_move(&page->lru, &zone->lru[lru].list);
    > + mem_cgroup_add_lru_list(page, lru);
    > + pgmoved++;
    > + if (!pagevec_add(&pvec, page)) {
    > + spin_unlock_irq(&zone->lru_lock);
    > + if (buffer_heads_over_limit)
    > + pagevec_strip(&pvec);
    > + __pagevec_release(&pvec);
    > + spin_lock_irq(&zone->lru_lock);
    > + }
    > + }

    The copy-n-pasting here is unfortunate. But I expect that if we redid
    this as a loop, the result would be a bit ugly - the pageActive
    handling gets in the way.

    > + __mod_zone_page_state(zone, NR_LRU_BASE + lru, pgmoved);

    Is it just me, is is all this stuff:

    lru = LRU_ACTIVE + file * LRU_FILE;
    ...
    foo(NR_LRU_BASE + lru);

    really hard to read?


    Now. How do we know that this patch improves Linux?



    \
     
     \ /
      Last update: 2009-05-08 22:09    [W:4.057 / U:1.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site