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

Johannes Weiner
2014-04-30T22:56:00Z

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.