Andrea Arcangeli <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
> 
> --- 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to