On Tue, 2017-05-02 at 17:27 -0400, Josef Bacik wrote:

> +	/*
> +	 * If we don't have a lot of inactive or slab pages then
> there's no
> +	 * point in trying to free them exclusively, do the normal
> scan stuff.
> +	 */
> +	if (nr_inactive < total_high_wmark && nr_slab <
> total_high_wmark)
> +		sc->inactive_only = 0;

This part looks good. Below this point, there is obviously no
point in skipping the active list.

> +	if (!global_reclaim(sc))
> +		sc->inactive_only = 0;

Why the different behaviour with and without cgroups?

Have you tested both of these?

> +	/*
> +	 * We still want to slightly prefer slab over inactive, so
> if inactive
> +	 * is large enough just skip slab shrinking for now.  If we
> aren't able
> +	 * to reclaim enough exclusively from the inactive lists
> then we'll
> +	 * reset this on the first loop and dip into slab.
> +	 */
> +	if (nr_inactive > total_high_wmark && nr_inactive > nr_slab)
> +		skip_slab = true;

I worry that this may be a little too aggressive,
and result in the slab cache growing much larger
than it should be on some systems.

I wonder if it may make more sense to have the
aggressiveness of slab scanning depend on the
ratio of inactive to reclaimable slab pages, rather
than having a hard cut-off like this?
 
-- 
All rights reversed