lkml.org 
[lkml]   [2017]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: + mm-vmscan-add-mm_vmscan_inactive_list_is_low-tracepoint.patch added to -mm tree
    Hello,

    On Wed, Jan 11, 2017 at 04:52:39PM +0100, Michal Hocko wrote:
    > On Wed 11-01-17 08:52:50, Minchan Kim wrote:
    > [...]
    > > > @@ -2055,8 +2055,8 @@ static bool inactive_list_is_low(struct
    > > > if (!file && !total_swap_pages)
    > > > return false;
    > > >
    > > > - inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
    > > > - active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
    > > > + total_inactive = inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
    > > > + total_active = active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
    > > >
    > >
    > > the decision of deactivating is based on eligible zone's LRU size,
    > > not whole zone so why should we need to get a trace of all zones's LRU?
    >
    > Strictly speaking, the total_ counters are not necessary for making the
    > decision. I found reporting those numbers useful regardless because this
    > will give us also an information how large is the eligible portion of
    > the LRU list. We do not have any other tracepoint which would report
    > that.

    The patch doesn't say anything why it's useful. Could you tell why it's
    useful and inactive_list_is_low should be right place?

    Don't get me wrong, please. I don't want to bother you.
    I really don't want to add random stuff although it's tracepoint for
    debugging.

    >
    > [...]
    > > > @@ -2223,7 +2228,7 @@ static void get_scan_count(struct lruvec
    > > > * lruvec even if it has plenty of old anonymous pages unless the
    > > > * system is under heavy pressure.
    > > > */
    > > > - if (!inactive_list_is_low(lruvec, true, sc) &&
    > > > + if (!inactive_list_is_low(lruvec, true, sc, false) &&
    > >
    > > Hmm, I was curious why you added trace boolean arguement and found it here.
    > > Yes, here is not related to deactivation directly but couldn't we help to
    > > trace it unconditionally?
    >
    > I've had it like that when I was debugging the mentioned bug and found
    > it a bit disturbing. It generated more output than I would like and it
    > wasn't really clear from which code path this has been called from.

    Indeed.

    Personally, I want to move inactive_list_is_low in shrink_active_list
    and shrink_active_list calls inactive_list_is_low(...., true),
    unconditionally so that it can make code simple/clear but cannot remove
    trace boolean variable , which what I want. So, it's okay if you love
    your version.

    >
    > > With that, we can know why VM reclaim only
    > > file-backed page on slow device although enough anonymous pages on fast
    > > swap like zram are enough.
    >
    > That would be something for a separate tracepoint in g_s_c

    Agree.

    \
     
     \ /
      Last update: 2017-01-12 06:13    [W:2.262 / U:0.204 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site