lkml.org 
[lkml]   [2009]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH 2/3] check unevictable flag in lumy reclaim v2
    From
    2009/6/11 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>:
    > Minchan Kim さん wrote:
    >> On Thu, Jun 11, 2009 at 5:38 PM, KAMEZAWA
    >> Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> wrote:
    >>> How about this ?
    >>>
    >>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    >>>
    >>> Lumpy reclaim check pages from their pfn. Then, it can find unevictable
    >>> pages
    >>> in its loop.
    >>> Abort lumpy reclaim when we find Unevictable page, we never get a lump
    >>> of pages for requested order.
    >>>
    >>> Changelog: v1->v2
    >>> ?- rewrote commet.
    >>>
    >>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    >>> ---
    >>> ?mm/vmscan.c | ? ?9 +++++++++
    >>> ?1 file changed, 9 insertions(+)
    >>>
    >>> Index: lumpy-reclaim-trial/mm/vmscan.c
    >>> ===================================================================
    >>> --- lumpy-reclaim-trial.orig/mm/vmscan.c
    >>> +++ lumpy-reclaim-trial/mm/vmscan.c
    >>> @@ -936,6 +936,15 @@ static unsigned long isolate_lru_pages(u
    >>> ? ? ? ? ? ? ? ? ? ? ? ?/* Check that we have not crossed a zone
    >>> boundary. */
    >>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(page_zone_id(cursor_page) !=
    >>> zone_id))
    >>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
    >>> + ? ? ? ? ? ? ? ? ? ? ? /*
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?* We tries to free all pages in this range to
    >>> create
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?* a free large page. Then, if the range
    >>> includes a page
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?* never be reclaimed, we have no reason to do
    >>> more.
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?* PageUnevictable page is not a page which can
    >>> be
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?* easily freed. Abort this scan now.
    >>> + ? ? ? ? ? ? ? ? ? ? ? ?*/
    >>> + ? ? ? ? ? ? ? ? ? ? ? if (unlikely(PageUnevictable(cursor_page)))
    >>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
    >>
    >> __isolate_lru_pages already checked PageUnevictable to return error.
    >> I want to remove repeated check although it is trivial.
    >>
    >> By your patch, It seems to remove PageUnevictable check in
    >> __isolate_lru_pages.
    >>
    > yes.
    >
    >> But I know that. If we remove PageUnevictable check in
    >> __isolate_lru_pages, it can't go into BUG in non-lumpy case. ( I
    >> mentioned following as code)
    >>
    > In non-lumpy case, we'll never see Unevictable, maybe.

    I think so if it doesn't happen RAM failure.
    AFAIK, Unevictable check didn't related with RAM failure.

    >
    >>                 case -EBUSY:
    >>                         /* else it is being freed elsewhere */
    >>                         list_move(&page->lru, src);
    >>                         continue;
    >>
    >>                 default:
    >>                         BUG();
    >>                 }
    >>
    >>
    >> It means we can remove BUG in non-lumpy case and then add BUG into
    >> __isolate_lru_pages directly.
    >>
    >> If we can do it, we can remove unnecessary PageUnevictable check in
    >> __isolate_lru_page.
    >>
    > Hmm, but Unevicable check had tons of troubles at its implementation
    > and I don't want to do it at once.

    I think it's not a big problem.
    As comment said, the check's goal is to prevent in lumpy case.
    /*
    * When this function is being called for lumpy reclaim, we
    * initially look into all LRU pages, active, inactive and
    * unevictable; only give shrink_page_list evictable pages.
    */
    if (PageUnevictable(page))
    return ret;

    So I think we can remove this check.

    >> I am not sure this is right in case of memcg.
    >>
    > I think we don't see Unevictable in memcg's path if my memcg-lru code
    > works as designed.
    >
    > I'll postpone this patch for a while until my brain works well.

    If you have a concern about that, how about this ?
    (This code will be hunk since gmail webserver always mangle. Pz,forgive me)
    Also, we can CC original authors.

    --- a/mm/vmscan.c
    ++ b/mm/vmscan.c
    @@ -936,19 +936,20 @@ static unsigned long isolate_lru_pages(unsigned
    long nr_to_scan,
    /* Check that we have not crossed a zone boundary. */
    if (unlikely(page_zone_id(cursor_page) != zone_id))
    continue;
    - switch (__isolate_lru_page(cursor_page, mode, file)) {
    - case 0:
    + if (__isolate_lru_page(cursor_page, mode, file) == 0) {
    list_move(&cursor_page->lru, dst);
    nr_taken++;
    scan++;
    - break;
    -
    - case -EBUSY:
    - /* else it is being freed elsewhere */
    - list_move(&cursor_page->lru, src);
    - default:
    - break; /* ! on LRU or wrong list */
    }
    + else if (PageUnevictable(cursor_page))
    + /*
    + * We tries to free all pages in this
    range to create
    + * a free large page. Then, if the
    range includes a page
    + * never be reclaimed, we have no
    reason to do more.
    + * PageUnevictable page is not a page
    which can be
    + * easily freed. Abort this scan now.
    + */
    + break
    }
    }

    > Thanks,
    > -Kame
    >
    >



    --
    Kinds regards,
    Minchan Kim
    --
    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: 2009-06-11 14:01    [W:0.033 / U:90.280 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site