| Message ID | 1447371693-25143-3-git-send-email-hannes@cmpxchg.org |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Johannes Weiner <hannes@cmpxchg.org> Date: Thu, 12 Nov 2015 18:41:21 -0500 > Letting shrink_slab() handle the root_mem_cgroup, and implicitely the > !CONFIG_MEMCG case, allows shrink_zone() to invoke the shrinkers > unconditionally from within the memcg iteration loop. > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> > Acked-by: Michal Hocko <mhocko@suse.com> Acked-by: David S. Miller <davem@davemloft.net> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 12, 2015 at 06:41:21PM -0500, Johannes Weiner wrote: ... > diff --git a/mm/vmscan.c b/mm/vmscan.c > index a4507ec..e4f5b3c 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -411,6 +411,10 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, > struct shrinker *shrinker; > unsigned long freed = 0; > > + /* Global shrinker mode */ > + if (memcg == root_mem_cgroup) > + memcg = NULL; > + > if (memcg && !memcg_kmem_is_active(memcg)) > return 0; > > @@ -2410,11 +2414,22 @@ static bool shrink_zone(struct zone *zone, struct scan_control *sc, > shrink_lruvec(lruvec, swappiness, sc, &lru_pages); > zone_lru_pages += lru_pages; > > - if (memcg && is_classzone) > + /* > + * Shrink the slab caches in the same proportion that > + * the eligible LRU pages were scanned. > + */ > + if (is_classzone) { > shrink_slab(sc->gfp_mask, zone_to_nid(zone), > memcg, sc->nr_scanned - scanned, > lru_pages); > > + if (reclaim_state) { > + sc->nr_reclaimed += > + reclaim_state->reclaimed_slab; > + reclaim_state->reclaimed_slab = 0; > + } > + } > + > /* > * Direct reclaim and kswapd have to scan all memory > * cgroups to fulfill the overall scan target for the > @@ -2432,20 +2447,6 @@ static bool shrink_zone(struct zone *zone, struct scan_control *sc, > } > } while ((memcg = mem_cgroup_iter(root, memcg, &reclaim))); > > - /* > - * Shrink the slab caches in the same proportion that > - * the eligible LRU pages were scanned. > - */ > - if (global_reclaim(sc) && is_classzone) > - shrink_slab(sc->gfp_mask, zone_to_nid(zone), NULL, > - sc->nr_scanned - nr_scanned, > - zone_lru_pages); > - > - if (reclaim_state) { > - sc->nr_reclaimed += reclaim_state->reclaimed_slab; > - reclaim_state->reclaimed_slab = 0; > - } > - AFAICS this patch deadly breaks memcg-unaware shrinkers vs LRU balance: currently we scan (*total* LRU scanned / *total* LRU pages) of all such objects; with this patch we'd use the numbers from the root cgroup instead. If most processes reside in memory cgroups, the root cgroup will have only a few LRU pages and hence the pressure exerted upon such objects will be unfairly severe. Thanks, Vladimir > vmpressure(sc->gfp_mask, sc->target_mem_cgroup, > sc->nr_scanned - nr_scanned, > sc->nr_reclaimed - nr_reclaimed); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Nov 14, 2015 at 03:36:50PM +0300, Vladimir Davydov wrote: > On Thu, Nov 12, 2015 at 06:41:21PM -0500, Johannes Weiner wrote: > > @@ -2432,20 +2447,6 @@ static bool shrink_zone(struct zone *zone, struct scan_control *sc, > > } > > } while ((memcg = mem_cgroup_iter(root, memcg, &reclaim))); > > > > - /* > > - * Shrink the slab caches in the same proportion that > > - * the eligible LRU pages were scanned. > > - */ > > - if (global_reclaim(sc) && is_classzone) > > - shrink_slab(sc->gfp_mask, zone_to_nid(zone), NULL, > > - sc->nr_scanned - nr_scanned, > > - zone_lru_pages); > > - > > - if (reclaim_state) { > > - sc->nr_reclaimed += reclaim_state->reclaimed_slab; > > - reclaim_state->reclaimed_slab = 0; > > - } > > - > > AFAICS this patch deadly breaks memcg-unaware shrinkers vs LRU balance: > currently we scan (*total* LRU scanned / *total* LRU pages) of all such > objects; with this patch we'd use the numbers from the root cgroup > instead. If most processes reside in memory cgroups, the root cgroup > will have only a few LRU pages and hence the pressure exerted upon such > objects will be unfairly severe. You're absolutely right, good catch. Please disregard this patch. It's not necessary for this series after v2, I just kept it because I thought it's a nice simplification that's possible after making root_mem_cgroup public. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 9a7a24a..251bb51 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -502,6 +502,8 @@ void mem_cgroup_split_huge_fixup(struct page *head); #else /* CONFIG_MEMCG */ struct mem_cgroup; +#define root_mem_cgroup NULL + static inline void mem_cgroup_events(struct mem_cgroup *memcg, enum mem_cgroup_events_index idx, unsigned int nr) diff --git a/mm/vmscan.c b/mm/vmscan.c index a4507ec..e4f5b3c 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -411,6 +411,10 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct shrinker *shrinker; unsigned long freed = 0; + /* Global shrinker mode */ + if (memcg == root_mem_cgroup) + memcg = NULL; + if (memcg && !memcg_kmem_is_active(memcg)) return 0; @@ -2410,11 +2414,22 @@ static bool shrink_zone(struct zone *zone, struct scan_control *sc, shrink_lruvec(lruvec, swappiness, sc, &lru_pages); zone_lru_pages += lru_pages; - if (memcg && is_classzone) + /* + * Shrink the slab caches in the same proportion that + * the eligible LRU pages were scanned. + */ + if (is_classzone) { shrink_slab(sc->gfp_mask, zone_to_nid(zone), memcg, sc->nr_scanned - scanned, lru_pages); + if (reclaim_state) { + sc->nr_reclaimed += + reclaim_state->reclaimed_slab; + reclaim_state->reclaimed_slab = 0; + } + } + /* * Direct reclaim and kswapd have to scan all memory * cgroups to fulfill the overall scan target for the @@ -2432,20 +2447,6 @@ static bool shrink_zone(struct zone *zone, struct scan_control *sc, } } while ((memcg = mem_cgroup_iter(root, memcg, &reclaim))); - /* - * Shrink the slab caches in the same proportion that - * the eligible LRU pages were scanned. - */ - if (global_reclaim(sc) && is_classzone) - shrink_slab(sc->gfp_mask, zone_to_nid(zone), NULL, - sc->nr_scanned - nr_scanned, - zone_lru_pages); - - if (reclaim_state) { - sc->nr_reclaimed += reclaim_state->reclaimed_slab; - reclaim_state->reclaimed_slab = 0; - } - vmpressure(sc->gfp_mask, sc->target_mem_cgroup, sc->nr_scanned - nr_scanned, sc->nr_reclaimed - nr_reclaimed);