[lkml]   [2011]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: Kernel falls apart under light memory pressure (i.e. linking vmlinux)
    On Mon, May 23, 2011 at 08:12:50AM +0900, Minchan Kim wrote:
    > diff --git a/mm/vmscan.c b/mm/vmscan.c
    > index 292582c..1663d24 100644
    > --- a/mm/vmscan.c
    > +++ b/mm/vmscan.c
    > @@ -231,8 +231,11 @@ unsigned long shrink_slab(struct shrink_control *shrink,
    > if (scanned == 0)
    > scanned = SWAP_CLUSTER_MAX;
    > - if (!down_read_trylock(&shrinker_rwsem))
    > - return 1; /* Assume we'll be able to shrink next time */
    > + if (!down_read_trylock(&shrinker_rwsem)) {
    > + /* Assume we'll be able to shrink next time */
    > + ret = 1;
    > + goto out;
    > + }

    It looks cleaner to return -1 here to differentiate the failure in
    taking the lock from when we take the lock and just 1 object is
    freed. Callers seems to be ok with -1 already and more intuitive for
    the while (nr > 10) loops too (those loops could be changed to "while
    (nr > 0)" if all shrinkers are accurate and not doing something
    inaccurate like the above code did, the shrinkers retvals I didn't
    check yet).

    > up_read(&shrinker_rwsem);
    > +out:
    > + cond_resched();
    > return ret;
    > }

    If we enter the loop some of the shrinkers will reschedule but it
    looks good for the last iteration that may have still run for some
    time before returning. The actual failure of shrinker_rwsem seems only
    theoretical though (but ok to cover it too with the cond_resched, but
    in practice this should be more for the case where shrinker_rwsem
    doesn't fail).

    > @@ -2331,7 +2336,7 @@ static bool sleeping_prematurely(pg_data_t
    > *pgdat, int order, long remaining,
    > * must be balanced
    > */
    > if (order)
    > - return pgdat_balanced(pgdat, balanced, classzone_idx);
    > + return !pgdat_balanced(pgdat, balanced, classzone_idx);
    > else
    > return !all_zones_ok;
    > }

    I now wonder if this is why compaction in kswapd didn't work out well
    and kswapd would spin at 100% load so much when compaction was added,
    plus with kswapd-compaction patch I think this code should be changed

    if (!COMPACTION_BUILD && order)
    return !pgdat_balanced();
    return !all_zones_ok;

    (but only with kswapd-compaction)
    I should probably give kswapd-compaction another spin after fixing
    this, because with compaction kswapd should be super successful at
    satisfying zone_watermark_ok_safe(zone, _order_...) in the
    sleeping_prematurely high watermark check, leading to pgdat_balanced
    returning true most of the time (which would make kswapd go crazy spin
    instead of stopping as it was supposed to). Mel, do you also think
    it's worth another try with a fixed sleeping_prematurely like above?

    Another thing, I'm not excited of the schedule_timeout(HZ/10) in
    kswapd_try_to_sleep(), it seems all for the statistics.


     \ /
      Last update: 2011-05-23 18:45    [W:0.033 / U:1.576 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site