Re: OOM problems with 2.6.11-rc4

Andrew Morton
2005-03-16T12:05:14Z

Andrea Arcangeli <andrea@suse.de> wrote:
>
> the VM is setting all_unreclaimable on the
> normal zone without any care about the progress we're making at freeing
> the slab.

Urgh, I didn't notice that all_unreclaimable is set.

> Beware this absolutely untested and it may not be enough. Perhaps there
> are more bugs in the same area (the shrink_slab itself seems overkill
> complicated for no good reason and different methods returns random
> stuff, dcache returns a percentage of the free entries, dquot instead
> returns the allocated inuse entries too which makes the whole API
> looking unreliable).

No, the two functions are equivalent for the default value of
vfs_cache_pressure (100) - it's not a percentage. It's just that we forgot
about the quota cache when adding the tunable. And mbcache, come to that.

> Signed-off-by: Andrea Arcangeli <andrea@suse.de>
>
> --- x/mm/vmscan.c.~1~ 2005-03-14 05:02:17.000000000 +0100
> +++ x/mm/vmscan.c 2005-03-16 01:28:16.000000000 +0100
> @@ -1074,8 +1074,9 @@ scan:
> total_scanned += sc.nr_scanned;
> if (zone->all_unreclaimable)
> continue;
> - if (zone->pages_scanned >= (zone->nr_active +
> - zone->nr_inactive) * 4)

A change we made a while back effectively doubles the rate at which
pages_scanned gets incremented here (we now account for the active list as
well as the inactive list). So this should be *8 to make it more
equivalent to the old code. Not that this is likely to make much
difference.

> + if (!reclaim_state->reclaimed_slab &&
> + zone->pages_scanned >= (zone->nr_active +
> + zone->nr_inactive) * 4)
> zone->all_unreclaimable = 1;

That might not change anything because we clear ->all_unreclaimable in
free_page_bulk(). Although that is behind the per-cpu-pages, so there will
be some lag. And this change will cause us to not bale out of reclaim..

Still, I think it would make more sense to return a success indication from
shrink_slab() if we actually freed any slab objects. That will prevent us
from incorrectly going all_unreclaimable if all we happen to be doing is
increasing slab internal fragmentation.

We could do that kludgily by re-polling the shrinker but it would be better
to return a second value from all the shrinkers.

> --- x/fs/dquot.c.~1~ 2005-03-08 01:02:13.000000000 +0100
> +++ x/fs/dquot.c 2005-03-16 01:18:19.000000000 +0100
> @@ -510,7 +510,7 @@ static int shrink_dqcache_memory(int nr,
> spin_lock(&dq_list_lock);
> if (nr)
> prune_dqcache(nr);
> - ret = dqstats.allocated_dquots;
> + ret = (dqstats.free_dquots / 100) * sysctl_vfs_cache_pressure;
> spin_unlock(&dq_list_lock);
> return ret;
> }

yup.