Re: [PATCH 1/4] memcg, mm: introduce lowlimit reclaim

[Johannes Weiner]

On Mon, Apr 28, 2014 at 02:26:42PM +0200, Michal Hocko wrote:
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 19d620b3d69c..40e517630138 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2808,6 +2808,29 @@ static struct mem_cgroup *mem_cgroup_lookup(unsigned short id)
>  	return mem_cgroup_from_id(id);
>  }
>  
> +/**
> + * mem_cgroup_reclaim_eligible - checks whether given memcg is eligible for the
> + * reclaim
> + * @memcg: target memcg for the reclaim
> + * @root: root of the reclaim hierarchy (null for the global reclaim)
> + *
> + * The given group is reclaimable if it is above its low limit and the same
> + * applies for all parents up the hierarchy until root (including).
> + */
> +bool mem_cgroup_reclaim_eligible(struct mem_cgroup *memcg,
> +		struct mem_cgroup *root)

Could you please rename this to something that is more descriptive in
the reclaim callsite?  How about mem_cgroup_within_low_limit()?

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c1cd99a5074b..0f428158254e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2215,9 +2215,11 @@ static inline bool should_continue_reclaim(struct zone *zone,
>  	}
>  }
>  
> -static void shrink_zone(struct zone *zone, struct scan_control *sc)
> +static unsigned __shrink_zone(struct zone *zone, struct scan_control *sc,
> +		bool follow_low_limit)
>  {
>  	unsigned long nr_reclaimed, nr_scanned;
> +	unsigned nr_scanned_groups = 0;
>  
>  	do {
>  		struct mem_cgroup *root = sc->target_mem_cgroup;
> @@ -2234,7 +2236,23 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
>  		do {
>  			struct lruvec *lruvec;
>  
> +			/*
> +			 * Memcg might be under its low limit so we have to
> +			 * skip it during the first reclaim round
> +			 */
> +			if (follow_low_limit &&
> +					!mem_cgroup_reclaim_eligible(memcg, root)) {
> +				/*
> +				 * It would be more optimal to skip the memcg
> +				 * subtree now but we do not have a memcg iter
> +				 * helper for that. Anyone?
> +				 */
> +				memcg = mem_cgroup_iter(root, memcg, &reclaim);
> +				continue;
> +			}
> +
>  			lruvec = mem_cgroup_zone_lruvec(zone, memcg);
> +			nr_scanned_groups++;
>  
>  			shrink_lruvec(lruvec, sc);
>  
> @@ -2262,6 +2280,20 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
>  
>  	} while (should_continue_reclaim(zone, sc->nr_reclaimed - nr_reclaimed,
>  					 sc->nr_scanned - nr_scanned, sc));
> +
> +	return nr_scanned_groups;
> +}
> +
> +static void shrink_zone(struct zone *zone, struct scan_control *sc)
> +{
> +	if (!__shrink_zone(zone, sc, true)) {
> +		/*
> +		 * First round of reclaim didn't find anything to reclaim
> +		 * because of low limit protection so try again and ignore
> +		 * the low limit this time.
> +		 */
> +		__shrink_zone(zone, sc, false);
> +	}
>  }
>  
>  /* Returns true if compaction should go ahead for a high-order request */

I would actually prefer not having a second round here, and make the
low limit behave more like mlock memory.  If there is no reclaimable
memory, go OOM.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/