* [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-20 16:33 ` Johannes Weiner
0 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-20 16:33 UTC (permalink / raw)
To: Andrew Morton
Cc: Mel Gorman, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
Page reclaim for a higher-order page runs until compaction is ready,
then aborts and signals this situation through the return value of
shrink_zones(). This is an oddly specific signal to encode in the
return value of shrink_zones(), though, and can be quite confusing.
Introduce sc->compaction_ready and signal the compactability of the
zones out-of-band to free up the return value of shrink_zones() for
actual zone reclaimability.
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 19b5b8016209..ed1efb84c542 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -65,6 +65,9 @@ struct scan_control {
/* Number of pages freed so far during a call to shrink_zones() */
unsigned long nr_reclaimed;
+ /* One of the zones is ready for compaction */
+ int compaction_ready;
+
/* How many pages shrink_list() should reclaim */
unsigned long nr_to_reclaim;
@@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
}
/* Returns true if compaction should go ahead for a high-order request */
-static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
+static inline bool compaction_ready(struct zone *zone, int order)
{
unsigned long balance_gap, watermark;
bool watermark_ok;
- /* Do not consider compaction for orders reclaim is meant to satisfy */
- if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
- return false;
-
/*
* Compaction takes time to run and there are potentially other
* callers using the pages just freed. Continue reclaiming until
@@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
*/
balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
- watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
+ watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
/*
* If compaction is deferred, reclaim up to a point where
* compaction will have a chance of success when re-enabled
*/
- if (compaction_deferred(zone, sc->order))
+ if (compaction_deferred(zone, order))
return watermark_ok;
/* If compaction is not ready to start, keep reclaiming */
- if (!compaction_suitable(zone, sc->order))
+ if (!compaction_suitable(zone, order))
return false;
return watermark_ok;
@@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
*
* If a zone is deemed to be full of pinned pages then just give it a light
* scan then give up on it.
- *
- * This function returns true if a zone is being reclaimed for a costly
- * high-order allocation and compaction is ready to begin. This indicates to
- * the caller that it should consider retrying the allocation instead of
- * further reclaim.
*/
-static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
+static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
{
struct zoneref *z;
struct zone *zone;
unsigned long nr_soft_reclaimed;
unsigned long nr_soft_scanned;
unsigned long lru_pages = 0;
- bool aborted_reclaim = false;
struct reclaim_state *reclaim_state = current->reclaim_state;
gfp_t orig_mask;
struct shrink_control shrink = {
@@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
if (sc->priority != DEF_PRIORITY &&
!zone_reclaimable(zone))
continue; /* Let kswapd poll it */
- if (IS_ENABLED(CONFIG_COMPACTION)) {
- /*
- * If we already have plenty of memory free for
- * compaction in this zone, don't free any more.
- * Even though compaction is invoked for any
- * non-zero order, only frequent costly order
- * reclamation is disruptive enough to become a
- * noticeable problem, like transparent huge
- * page allocations.
- */
- if ((zonelist_zone_idx(z) <= requested_highidx)
- && compaction_ready(zone, sc)) {
- aborted_reclaim = true;
- continue;
- }
+
+ /*
+ * If we already have plenty of memory free
+ * for compaction in this zone, don't free any
+ * more. Even though compaction is invoked
+ * for any non-zero order, only frequent
+ * costly order reclamation is disruptive
+ * enough to become a noticeable problem, like
+ * transparent huge page allocations.
+ */
+ if (IS_ENABLED(CONFIG_COMPACTION) &&
+ sc->order > PAGE_ALLOC_COSTLY_ORDER &&
+ zonelist_zone_idx(z) <= requested_highidx &&
+ compaction_ready(zone, sc->order)) {
+ sc->compaction_ready = true;
+ continue;
}
+
/*
* This steals pages from memory cgroups over softlimit
* and returns the number of reclaimed pages and
@@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
* promoted it to __GFP_HIGHMEM.
*/
sc->gfp_mask = orig_mask;
-
- return aborted_reclaim;
}
/* All zones in zonelist are unreclaimable? */
@@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
{
unsigned long total_scanned = 0;
unsigned long writeback_threshold;
- bool aborted_reclaim;
delayacct_freepages_start();
@@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
sc->priority);
sc->nr_scanned = 0;
- aborted_reclaim = shrink_zones(zonelist, sc);
+ shrink_zones(zonelist, sc);
total_scanned += sc->nr_scanned;
if (sc->nr_reclaimed >= sc->nr_to_reclaim)
goto out;
+ if (sc->compaction_ready)
+ goto out;
+
/*
* If we're getting trouble reclaiming, start doing
* writepage even in laptop mode.
@@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
WB_REASON_TRY_TO_FREE_PAGES);
sc->may_writepage = 1;
}
- } while (--sc->priority >= 0 && !aborted_reclaim);
+ } while (--sc->priority >= 0);
out:
delayacct_freepages_end();
@@ -2535,7 +2530,7 @@ out:
return sc->nr_reclaimed;
/* Aborted reclaim to try compaction? don't OOM, then */
- if (aborted_reclaim)
+ if (sc->compaction_ready)
return 1;
/* top priority shrink_zones still had more to do? don't OOM, then */
--
2.0.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-20 16:33 ` Johannes Weiner
@ 2014-06-20 16:56 ` Vlastimil Babka
-1 siblings, 0 replies; 42+ messages in thread
From: Vlastimil Babka @ 2014-06-20 16:56 UTC (permalink / raw)
To: Johannes Weiner, Andrew Morton
Cc: Mel Gorman, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On 06/20/2014 06:33 PM, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
(with a nitpick below)
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
You moved this comment block left, yet you further shortened the individual lines, despite
there is now more space to prolong them.
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-20 16:56 ` Vlastimil Babka
0 siblings, 0 replies; 42+ messages in thread
From: Vlastimil Babka @ 2014-06-20 16:56 UTC (permalink / raw)
To: Johannes Weiner, Andrew Morton
Cc: Mel Gorman, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On 06/20/2014 06:33 PM, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
(with a nitpick below)
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
You moved this comment block left, yet you further shortened the individual lines, despite
there is now more space to prolong them.
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-20 16:56 ` Vlastimil Babka
@ 2014-06-20 20:24 ` Johannes Weiner
-1 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-20 20:24 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Fri, Jun 20, 2014 at 06:56:03PM +0200, Vlastimil Babka wrote:
> On 06/20/2014 06:33 PM, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks, Vlastimil!
> > @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> > if (sc->priority != DEF_PRIORITY &&
> > !zone_reclaimable(zone))
> > continue; /* Let kswapd poll it */
> > - if (IS_ENABLED(CONFIG_COMPACTION)) {
> > - /*
> > - * If we already have plenty of memory free for
> > - * compaction in this zone, don't free any more.
> > - * Even though compaction is invoked for any
> > - * non-zero order, only frequent costly order
> > - * reclamation is disruptive enough to become a
> > - * noticeable problem, like transparent huge
> > - * page allocations.
> > - */
> > - if ((zonelist_zone_idx(z) <= requested_highidx)
> > - && compaction_ready(zone, sc)) {
> > - aborted_reclaim = true;
> > - continue;
> > - }
> > +
> > + /*
> > + * If we already have plenty of memory free
> > + * for compaction in this zone, don't free any
> > + * more. Even though compaction is invoked
> > + * for any non-zero order, only frequent
> > + * costly order reclamation is disruptive
> > + * enough to become a noticeable problem, like
> > + * transparent huge page allocations.
> > + */
>
> You moved this comment block left, yet you further shortened the individual lines, despite
> there is now more space to prolong them.
This is a result of using emacs' auto-fill all the time when writing
comments, I have to watch my reflexes while moving stuff around :-)
Updated patch:
---
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-20 20:24 ` Johannes Weiner
0 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-20 20:24 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Fri, Jun 20, 2014 at 06:56:03PM +0200, Vlastimil Babka wrote:
> On 06/20/2014 06:33 PM, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks, Vlastimil!
> > @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> > if (sc->priority != DEF_PRIORITY &&
> > !zone_reclaimable(zone))
> > continue; /* Let kswapd poll it */
> > - if (IS_ENABLED(CONFIG_COMPACTION)) {
> > - /*
> > - * If we already have plenty of memory free for
> > - * compaction in this zone, don't free any more.
> > - * Even though compaction is invoked for any
> > - * non-zero order, only frequent costly order
> > - * reclamation is disruptive enough to become a
> > - * noticeable problem, like transparent huge
> > - * page allocations.
> > - */
> > - if ((zonelist_zone_idx(z) <= requested_highidx)
> > - && compaction_ready(zone, sc)) {
> > - aborted_reclaim = true;
> > - continue;
> > - }
> > +
> > + /*
> > + * If we already have plenty of memory free
> > + * for compaction in this zone, don't free any
> > + * more. Even though compaction is invoked
> > + * for any non-zero order, only frequent
> > + * costly order reclamation is disruptive
> > + * enough to become a noticeable problem, like
> > + * transparent huge page allocations.
> > + */
>
> You moved this comment block left, yet you further shortened the individual lines, despite
> there is now more space to prolong them.
This is a result of using emacs' auto-fill all the time when writing
comments, I have to watch my reflexes while moving stuff around :-)
Updated patch:
---
>From cd48b73fdca9e23aa21f65e9af1f850dbac5ab8e Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Wed, 11 Jun 2014 12:53:59 -0400
Subject: [patch] mm: vmscan: rework compaction-ready signaling in direct
reclaim
Page reclaim for a higher-order page runs until compaction is ready,
then aborts and signals this situation through the return value of
shrink_zones(). This is an oddly specific signal to encode in the
return value of shrink_zones(), though, and can be quite confusing.
Introduce sc->compaction_ready and signal the compactability of the
zones out-of-band to free up the return value of shrink_zones() for
actual zone reclaimability.
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 19b5b8016209..35747a75bf08 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -65,6 +65,9 @@ struct scan_control {
/* Number of pages freed so far during a call to shrink_zones() */
unsigned long nr_reclaimed;
+ /* One of the zones is ready for compaction */
+ int compaction_ready;
+
/* How many pages shrink_list() should reclaim */
unsigned long nr_to_reclaim;
@@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
}
/* Returns true if compaction should go ahead for a high-order request */
-static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
+static inline bool compaction_ready(struct zone *zone, int order)
{
unsigned long balance_gap, watermark;
bool watermark_ok;
- /* Do not consider compaction for orders reclaim is meant to satisfy */
- if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
- return false;
-
/*
* Compaction takes time to run and there are potentially other
* callers using the pages just freed. Continue reclaiming until
@@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
*/
balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
- watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
+ watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
/*
* If compaction is deferred, reclaim up to a point where
* compaction will have a chance of success when re-enabled
*/
- if (compaction_deferred(zone, sc->order))
+ if (compaction_deferred(zone, order))
return watermark_ok;
/* If compaction is not ready to start, keep reclaiming */
- if (!compaction_suitable(zone, sc->order))
+ if (!compaction_suitable(zone, order))
return false;
return watermark_ok;
@@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
*
* If a zone is deemed to be full of pinned pages then just give it a light
* scan then give up on it.
- *
- * This function returns true if a zone is being reclaimed for a costly
- * high-order allocation and compaction is ready to begin. This indicates to
- * the caller that it should consider retrying the allocation instead of
- * further reclaim.
*/
-static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
+static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
{
struct zoneref *z;
struct zone *zone;
unsigned long nr_soft_reclaimed;
unsigned long nr_soft_scanned;
unsigned long lru_pages = 0;
- bool aborted_reclaim = false;
struct reclaim_state *reclaim_state = current->reclaim_state;
gfp_t orig_mask;
struct shrink_control shrink = {
@@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
if (sc->priority != DEF_PRIORITY &&
!zone_reclaimable(zone))
continue; /* Let kswapd poll it */
- if (IS_ENABLED(CONFIG_COMPACTION)) {
- /*
- * If we already have plenty of memory free for
- * compaction in this zone, don't free any more.
- * Even though compaction is invoked for any
- * non-zero order, only frequent costly order
- * reclamation is disruptive enough to become a
- * noticeable problem, like transparent huge
- * page allocations.
- */
- if ((zonelist_zone_idx(z) <= requested_highidx)
- && compaction_ready(zone, sc)) {
- aborted_reclaim = true;
- continue;
- }
+
+ /*
+ * If we already have plenty of memory free for
+ * compaction in this zone, don't free any more.
+ * Even though compaction is invoked for any
+ * non-zero order, only frequent costly order
+ * reclamation is disruptive enough to become a
+ * noticeable problem, like transparent huge
+ * page allocations.
+ */
+ if (IS_ENABLED(CONFIG_COMPACTION) &&
+ sc->order > PAGE_ALLOC_COSTLY_ORDER &&
+ zonelist_zone_idx(z) <= requested_highidx &&
+ compaction_ready(zone, sc->order)) {
+ sc->compaction_ready = true;
+ continue;
}
+
/*
* This steals pages from memory cgroups over softlimit
* and returns the number of reclaimed pages and
@@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
* promoted it to __GFP_HIGHMEM.
*/
sc->gfp_mask = orig_mask;
-
- return aborted_reclaim;
}
/* All zones in zonelist are unreclaimable? */
@@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
{
unsigned long total_scanned = 0;
unsigned long writeback_threshold;
- bool aborted_reclaim;
delayacct_freepages_start();
@@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
sc->priority);
sc->nr_scanned = 0;
- aborted_reclaim = shrink_zones(zonelist, sc);
+ shrink_zones(zonelist, sc);
total_scanned += sc->nr_scanned;
if (sc->nr_reclaimed >= sc->nr_to_reclaim)
goto out;
+ if (sc->compaction_ready)
+ goto out;
+
/*
* If we're getting trouble reclaiming, start doing
* writepage even in laptop mode.
@@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
WB_REASON_TRY_TO_FREE_PAGES);
sc->may_writepage = 1;
}
- } while (--sc->priority >= 0 && !aborted_reclaim);
+ } while (--sc->priority >= 0);
out:
delayacct_freepages_end();
@@ -2535,7 +2530,7 @@ out:
return sc->nr_reclaimed;
/* Aborted reclaim to try compaction? don't OOM, then */
- if (aborted_reclaim)
+ if (sc->compaction_ready)
return 1;
/* top priority shrink_zones still had more to do? don't OOM, then */
--
2.0.0
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-20 20:24 ` Johannes Weiner
@ 2014-06-23 7:28 ` Michal Hocko
-1 siblings, 0 replies; 42+ messages in thread
From: Michal Hocko @ 2014-06-23 7:28 UTC (permalink / raw)
To: Johannes Weiner
Cc: Vlastimil Babka, Andrew Morton, Mel Gorman, Rik van Riel,
linux-mm, linux-kernel
On Fri 20-06-14 16:24:49, Johannes Weiner wrote:
[...]
> From cd48b73fdca9e23aa21f65e9af1f850dbac5ab8e Mon Sep 17 00:00:00 2001
> From: Johannes Weiner <hannes@cmpxchg.org>
> Date: Wed, 11 Jun 2014 12:53:59 -0400
> Subject: [patch] mm: vmscan: rework compaction-ready signaling in direct
> reclaim
>
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Very nice. It will help me to get rid off additional hacks for the
min_limit for memcg. Thanks!
One question below
[...]
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
It is not entirely clear to me why we do not need to check and wake up
flusher threads anymore?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-23 7:28 ` Michal Hocko
0 siblings, 0 replies; 42+ messages in thread
From: Michal Hocko @ 2014-06-23 7:28 UTC (permalink / raw)
To: Johannes Weiner
Cc: Vlastimil Babka, Andrew Morton, Mel Gorman, Rik van Riel,
linux-mm, linux-kernel
On Fri 20-06-14 16:24:49, Johannes Weiner wrote:
[...]
> From cd48b73fdca9e23aa21f65e9af1f850dbac5ab8e Mon Sep 17 00:00:00 2001
> From: Johannes Weiner <hannes@cmpxchg.org>
> Date: Wed, 11 Jun 2014 12:53:59 -0400
> Subject: [patch] mm: vmscan: rework compaction-ready signaling in direct
> reclaim
>
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Very nice. It will help me to get rid off additional hacks for the
min_limit for memcg. Thanks!
One question below
[...]
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
It is not entirely clear to me why we do not need to check and wake up
flusher threads anymore?
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-20 16:33 ` Johannes Weiner
@ 2014-06-23 6:36 ` Minchan Kim
-1 siblings, 0 replies; 42+ messages in thread
From: Minchan Kim @ 2014-06-23 6:36 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Minchan Kim <minchan@kernel.org>
Below just one nitpick.
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
You are deleting comment sc->order <= PAGE_ALLOC_COSTLY_ORDER which was
in compaction_ready. At least, that comment was useful for me to guess
the intention. So if you have strong reason to remove that, I'd like to
remain it.
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
> --
> 2.0.0
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-23 6:36 ` Minchan Kim
0 siblings, 0 replies; 42+ messages in thread
From: Minchan Kim @ 2014-06-23 6:36 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Minchan Kim <minchan@kernel.org>
Below just one nitpick.
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
You are deleting comment sc->order <= PAGE_ALLOC_COSTLY_ORDER which was
in compaction_ready. At least, that comment was useful for me to guess
the intention. So if you have strong reason to remove that, I'd like to
remain it.
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
> --
> 2.0.0
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kind regards,
Minchan Kim
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-23 6:36 ` Minchan Kim
@ 2014-06-23 18:20 ` Johannes Weiner
-1 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-23 18:20 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Mon, Jun 23, 2014 at 03:36:37PM +0900, Minchan Kim wrote:
> On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Minchan Kim <minchan@kernel.org>
Thanks!
> > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > }
> >
> > /* Returns true if compaction should go ahead for a high-order request */
> > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > +static inline bool compaction_ready(struct zone *zone, int order)
> > {
> > unsigned long balance_gap, watermark;
> > bool watermark_ok;
> >
> > - /* Do not consider compaction for orders reclaim is meant to satisfy */
> > - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> > - return false;
> > -
> > /*
> > * Compaction takes time to run and there are potentially other
> > * callers using the pages just freed. Continue reclaiming until
> > @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> > if (sc->priority != DEF_PRIORITY &&
> > !zone_reclaimable(zone))
> > continue; /* Let kswapd poll it */
> > - if (IS_ENABLED(CONFIG_COMPACTION)) {
> > - /*
> > - * If we already have plenty of memory free for
> > - * compaction in this zone, don't free any more.
> > - * Even though compaction is invoked for any
> > - * non-zero order, only frequent costly order
> > - * reclamation is disruptive enough to become a
> > - * noticeable problem, like transparent huge
> > - * page allocations.
> > - */
> > - if ((zonelist_zone_idx(z) <= requested_highidx)
> > - && compaction_ready(zone, sc)) {
> > - aborted_reclaim = true;
> > - continue;
> > - }
> > +
> > + /*
> > + * If we already have plenty of memory free
> > + * for compaction in this zone, don't free any
> > + * more. Even though compaction is invoked
> > + * for any non-zero order, only frequent
> > + * costly order reclamation is disruptive
> > + * enough to become a noticeable problem, like
> > + * transparent huge page allocations.
> > + */
> > + if (IS_ENABLED(CONFIG_COMPACTION) &&
> > + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
>
> You are deleting comment sc->order <= PAGE_ALLOC_COSTLY_ORDER which was
> in compaction_ready. At least, that comment was useful for me to guess
> the intention. So if you have strong reason to remove that, I'd like to
> remain it.
There are two separate explanations for aborting reclaim early for
costly orders:
1. /* Do not consider compaction for orders reclaim is meant to satisfy */
2. /*
* Even though compaction is invoked
* for any non-zero order, only frequent
* costly order reclamation is disruptive
* enough to become a noticeable problem, like
* transparent huge page allocations.
*/
I thought it makes sense to pick one and go with that, so I went with
2. and moved the order check out there as well.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-23 18:20 ` Johannes Weiner
0 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-23 18:20 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Mel Gorman, Rik van Riel, Michal Hocko, linux-mm,
linux-kernel
On Mon, Jun 23, 2014 at 03:36:37PM +0900, Minchan Kim wrote:
> On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Minchan Kim <minchan@kernel.org>
Thanks!
> > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > }
> >
> > /* Returns true if compaction should go ahead for a high-order request */
> > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > +static inline bool compaction_ready(struct zone *zone, int order)
> > {
> > unsigned long balance_gap, watermark;
> > bool watermark_ok;
> >
> > - /* Do not consider compaction for orders reclaim is meant to satisfy */
> > - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> > - return false;
> > -
> > /*
> > * Compaction takes time to run and there are potentially other
> > * callers using the pages just freed. Continue reclaiming until
> > @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> > if (sc->priority != DEF_PRIORITY &&
> > !zone_reclaimable(zone))
> > continue; /* Let kswapd poll it */
> > - if (IS_ENABLED(CONFIG_COMPACTION)) {
> > - /*
> > - * If we already have plenty of memory free for
> > - * compaction in this zone, don't free any more.
> > - * Even though compaction is invoked for any
> > - * non-zero order, only frequent costly order
> > - * reclamation is disruptive enough to become a
> > - * noticeable problem, like transparent huge
> > - * page allocations.
> > - */
> > - if ((zonelist_zone_idx(z) <= requested_highidx)
> > - && compaction_ready(zone, sc)) {
> > - aborted_reclaim = true;
> > - continue;
> > - }
> > +
> > + /*
> > + * If we already have plenty of memory free
> > + * for compaction in this zone, don't free any
> > + * more. Even though compaction is invoked
> > + * for any non-zero order, only frequent
> > + * costly order reclamation is disruptive
> > + * enough to become a noticeable problem, like
> > + * transparent huge page allocations.
> > + */
> > + if (IS_ENABLED(CONFIG_COMPACTION) &&
> > + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
>
> You are deleting comment sc->order <= PAGE_ALLOC_COSTLY_ORDER which was
> in compaction_ready. At least, that comment was useful for me to guess
> the intention. So if you have strong reason to remove that, I'd like to
> remain it.
There are two separate explanations for aborting reclaim early for
costly orders:
1. /* Do not consider compaction for orders reclaim is meant to satisfy */
2. /*
* Even though compaction is invoked
* for any non-zero order, only frequent
* costly order reclamation is disruptive
* enough to become a noticeable problem, like
* transparent huge page allocations.
*/
I thought it makes sense to pick one and go with that, so I went with
2. and moved the order check out there as well.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-20 16:33 ` Johannes Weiner
@ 2014-06-23 13:07 ` Mel Gorman
-1 siblings, 0 replies; 42+ messages in thread
From: Mel Gorman @ 2014-06-23 13:07 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
You are not the criminal here but scan_control is larger than it needs
to be and the stack usage of reclaim has reared its head again.
Add a preparation patch that convert sc->may* and sc->hibernation_mode
to bool and moves them towards the end of the struct. Then add
compaction_ready as a bool.
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
Why did you remove the use of sc->order? In this patch there is only one
called of compaction_ready and it looks like
if (IS_ENABLED(CONFIG_COMPACTION) &&
sc->order > PAGE_ALLOC_COSTLY_ORDER &&
zonelist_zone_idx(z) <= requested_highidx &&
compaction_ready(zone, sc->order)) {
So it's unclear why you changed the signature.
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
break?
Convert the other one to break as well. out label seems unnecessary in
this context.
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
> --
> 2.0.0
>
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-23 13:07 ` Mel Gorman
0 siblings, 0 replies; 42+ messages in thread
From: Mel Gorman @ 2014-06-23 13:07 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> Page reclaim for a higher-order page runs until compaction is ready,
> then aborts and signals this situation through the return value of
> shrink_zones(). This is an oddly specific signal to encode in the
> return value of shrink_zones(), though, and can be quite confusing.
>
> Introduce sc->compaction_ready and signal the compactability of the
> zones out-of-band to free up the return value of shrink_zones() for
> actual zone reclaimability.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
> mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 19b5b8016209..ed1efb84c542 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -65,6 +65,9 @@ struct scan_control {
> /* Number of pages freed so far during a call to shrink_zones() */
> unsigned long nr_reclaimed;
>
> + /* One of the zones is ready for compaction */
> + int compaction_ready;
> +
> /* How many pages shrink_list() should reclaim */
> unsigned long nr_to_reclaim;
>
You are not the criminal here but scan_control is larger than it needs
to be and the stack usage of reclaim has reared its head again.
Add a preparation patch that convert sc->may* and sc->hibernation_mode
to bool and moves them towards the end of the struct. Then add
compaction_ready as a bool.
> @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> }
>
> /* Returns true if compaction should go ahead for a high-order request */
> -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> +static inline bool compaction_ready(struct zone *zone, int order)
> {
Why did you remove the use of sc->order? In this patch there is only one
called of compaction_ready and it looks like
if (IS_ENABLED(CONFIG_COMPACTION) &&
sc->order > PAGE_ALLOC_COSTLY_ORDER &&
zonelist_zone_idx(z) <= requested_highidx &&
compaction_ready(zone, sc->order)) {
So it's unclear why you changed the signature.
> unsigned long balance_gap, watermark;
> bool watermark_ok;
>
> - /* Do not consider compaction for orders reclaim is meant to satisfy */
> - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER)
> - return false;
> -
> /*
> * Compaction takes time to run and there are potentially other
> * callers using the pages just freed. Continue reclaiming until
> @@ -2309,18 +2308,18 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> */
> balance_gap = min(low_wmark_pages(zone), DIV_ROUND_UP(
> zone->managed_pages, KSWAPD_ZONE_BALANCE_GAP_RATIO));
> - watermark = high_wmark_pages(zone) + balance_gap + (2UL << sc->order);
> + watermark = high_wmark_pages(zone) + balance_gap + (2UL << order);
> watermark_ok = zone_watermark_ok_safe(zone, 0, watermark, 0, 0);
>
> /*
> * If compaction is deferred, reclaim up to a point where
> * compaction will have a chance of success when re-enabled
> */
> - if (compaction_deferred(zone, sc->order))
> + if (compaction_deferred(zone, order))
> return watermark_ok;
>
> /* If compaction is not ready to start, keep reclaiming */
> - if (!compaction_suitable(zone, sc->order))
> + if (!compaction_suitable(zone, order))
> return false;
>
> return watermark_ok;
> @@ -2341,20 +2340,14 @@ static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> *
> * If a zone is deemed to be full of pinned pages then just give it a light
> * scan then give up on it.
> - *
> - * This function returns true if a zone is being reclaimed for a costly
> - * high-order allocation and compaction is ready to begin. This indicates to
> - * the caller that it should consider retrying the allocation instead of
> - * further reclaim.
> */
> -static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> +static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> {
> struct zoneref *z;
> struct zone *zone;
> unsigned long nr_soft_reclaimed;
> unsigned long nr_soft_scanned;
> unsigned long lru_pages = 0;
> - bool aborted_reclaim = false;
> struct reclaim_state *reclaim_state = current->reclaim_state;
> gfp_t orig_mask;
> struct shrink_control shrink = {
> @@ -2391,22 +2384,24 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> if (sc->priority != DEF_PRIORITY &&
> !zone_reclaimable(zone))
> continue; /* Let kswapd poll it */
> - if (IS_ENABLED(CONFIG_COMPACTION)) {
> - /*
> - * If we already have plenty of memory free for
> - * compaction in this zone, don't free any more.
> - * Even though compaction is invoked for any
> - * non-zero order, only frequent costly order
> - * reclamation is disruptive enough to become a
> - * noticeable problem, like transparent huge
> - * page allocations.
> - */
> - if ((zonelist_zone_idx(z) <= requested_highidx)
> - && compaction_ready(zone, sc)) {
> - aborted_reclaim = true;
> - continue;
> - }
> +
> + /*
> + * If we already have plenty of memory free
> + * for compaction in this zone, don't free any
> + * more. Even though compaction is invoked
> + * for any non-zero order, only frequent
> + * costly order reclamation is disruptive
> + * enough to become a noticeable problem, like
> + * transparent huge page allocations.
> + */
> + if (IS_ENABLED(CONFIG_COMPACTION) &&
> + sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> + zonelist_zone_idx(z) <= requested_highidx &&
> + compaction_ready(zone, sc->order)) {
> + sc->compaction_ready = true;
> + continue;
> }
> +
> /*
> * This steals pages from memory cgroups over softlimit
> * and returns the number of reclaimed pages and
> @@ -2444,8 +2439,6 @@ static bool shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
> * promoted it to __GFP_HIGHMEM.
> */
> sc->gfp_mask = orig_mask;
> -
> - return aborted_reclaim;
> }
>
> /* All zones in zonelist are unreclaimable? */
> @@ -2489,7 +2482,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> {
> unsigned long total_scanned = 0;
> unsigned long writeback_threshold;
> - bool aborted_reclaim;
>
> delayacct_freepages_start();
>
> @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> sc->priority);
> sc->nr_scanned = 0;
> - aborted_reclaim = shrink_zones(zonelist, sc);
> + shrink_zones(zonelist, sc);
>
> total_scanned += sc->nr_scanned;
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> goto out;
>
> + if (sc->compaction_ready)
> + goto out;
> +
break?
Convert the other one to break as well. out label seems unnecessary in
this context.
> /*
> * If we're getting trouble reclaiming, start doing
> * writepage even in laptop mode.
> @@ -2526,7 +2521,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> WB_REASON_TRY_TO_FREE_PAGES);
> sc->may_writepage = 1;
> }
> - } while (--sc->priority >= 0 && !aborted_reclaim);
> + } while (--sc->priority >= 0);
>
> out:
> delayacct_freepages_end();
> @@ -2535,7 +2530,7 @@ out:
> return sc->nr_reclaimed;
>
> /* Aborted reclaim to try compaction? don't OOM, then */
> - if (aborted_reclaim)
> + if (sc->compaction_ready)
> return 1;
>
> /* top priority shrink_zones still had more to do? don't OOM, then */
> --
> 2.0.0
>
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-23 13:07 ` Mel Gorman
@ 2014-06-23 17:20 ` Johannes Weiner
-1 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-23 17:20 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
Hi Mel,
On Mon, Jun 23, 2014 at 02:07:05PM +0100, Mel Gorman wrote:
> On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > ---
> > mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> > 1 file changed, 31 insertions(+), 36 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 19b5b8016209..ed1efb84c542 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -65,6 +65,9 @@ struct scan_control {
> > /* Number of pages freed so far during a call to shrink_zones() */
> > unsigned long nr_reclaimed;
> >
> > + /* One of the zones is ready for compaction */
> > + int compaction_ready;
> > +
> > /* How many pages shrink_list() should reclaim */
> > unsigned long nr_to_reclaim;
> >
>
> You are not the criminal here but scan_control is larger than it needs
> to be and the stack usage of reclaim has reared its head again.
>
> Add a preparation patch that convert sc->may* and sc->hibernation_mode
> to bool and moves them towards the end of the struct. Then add
> compaction_ready as a bool.
Good idea, I'll do that.
> > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > }
> >
> > /* Returns true if compaction should go ahead for a high-order request */
> > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > +static inline bool compaction_ready(struct zone *zone, int order)
> >
> > {
>
> Why did you remove the use of sc->order? In this patch there is only one
> called of compaction_ready and it looks like
>
> if (IS_ENABLED(CONFIG_COMPACTION) &&
> sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> zonelist_zone_idx(z) <= requested_highidx &&
> compaction_ready(zone, sc->order)) {
>
> So it's unclear why you changed the signature.
Everything else in compaction_ready() is about internal compaction
requirements, like checking for free pages and deferred compaction,
whereas this order check is more of a reclaim policy rule according to
the comment in the caller:
...
* Even though compaction is invoked for any
* non-zero order, only frequent costly order
* reclamation is disruptive enough to become a
* noticeable problem, like transparent huge
* page allocations.
*/
But it's an unrelated in-the-area-anyway change, I can split it out -
or drop it entirely - if you prefer.
> > @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> > vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> > sc->priority);
> > sc->nr_scanned = 0;
> > - aborted_reclaim = shrink_zones(zonelist, sc);
> > + shrink_zones(zonelist, sc);
> >
> > total_scanned += sc->nr_scanned;
> > if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> > goto out;
> >
> > + if (sc->compaction_ready)
> > + goto out;
> > +
>
> break?
>
> Convert the other one to break as well. out label seems unnecessary in
> this context.
Makes sense, I'll include this in v2.
Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-23 17:20 ` Johannes Weiner
0 siblings, 0 replies; 42+ messages in thread
From: Johannes Weiner @ 2014-06-23 17:20 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
Hi Mel,
On Mon, Jun 23, 2014 at 02:07:05PM +0100, Mel Gorman wrote:
> On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > Page reclaim for a higher-order page runs until compaction is ready,
> > then aborts and signals this situation through the return value of
> > shrink_zones(). This is an oddly specific signal to encode in the
> > return value of shrink_zones(), though, and can be quite confusing.
> >
> > Introduce sc->compaction_ready and signal the compactability of the
> > zones out-of-band to free up the return value of shrink_zones() for
> > actual zone reclaimability.
> >
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > ---
> > mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> > 1 file changed, 31 insertions(+), 36 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 19b5b8016209..ed1efb84c542 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -65,6 +65,9 @@ struct scan_control {
> > /* Number of pages freed so far during a call to shrink_zones() */
> > unsigned long nr_reclaimed;
> >
> > + /* One of the zones is ready for compaction */
> > + int compaction_ready;
> > +
> > /* How many pages shrink_list() should reclaim */
> > unsigned long nr_to_reclaim;
> >
>
> You are not the criminal here but scan_control is larger than it needs
> to be and the stack usage of reclaim has reared its head again.
>
> Add a preparation patch that convert sc->may* and sc->hibernation_mode
> to bool and moves them towards the end of the struct. Then add
> compaction_ready as a bool.
Good idea, I'll do that.
> > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > }
> >
> > /* Returns true if compaction should go ahead for a high-order request */
> > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > +static inline bool compaction_ready(struct zone *zone, int order)
> >
> > {
>
> Why did you remove the use of sc->order? In this patch there is only one
> called of compaction_ready and it looks like
>
> if (IS_ENABLED(CONFIG_COMPACTION) &&
> sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> zonelist_zone_idx(z) <= requested_highidx &&
> compaction_ready(zone, sc->order)) {
>
> So it's unclear why you changed the signature.
Everything else in compaction_ready() is about internal compaction
requirements, like checking for free pages and deferred compaction,
whereas this order check is more of a reclaim policy rule according to
the comment in the caller:
...
* Even though compaction is invoked for any
* non-zero order, only frequent costly order
* reclamation is disruptive enough to become a
* noticeable problem, like transparent huge
* page allocations.
*/
But it's an unrelated in-the-area-anyway change, I can split it out -
or drop it entirely - if you prefer.
> > @@ -2500,12 +2492,15 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> > vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup,
> > sc->priority);
> > sc->nr_scanned = 0;
> > - aborted_reclaim = shrink_zones(zonelist, sc);
> > + shrink_zones(zonelist, sc);
> >
> > total_scanned += sc->nr_scanned;
> > if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> > goto out;
> >
> > + if (sc->compaction_ready)
> > + goto out;
> > +
>
> break?
>
> Convert the other one to break as well. out label seems unnecessary in
> this context.
Makes sense, I'll include this in v2.
Thanks!
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
2014-06-23 17:20 ` Johannes Weiner
@ 2014-06-25 9:55 ` Mel Gorman
-1 siblings, 0 replies; 42+ messages in thread
From: Mel Gorman @ 2014-06-25 9:55 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On Mon, Jun 23, 2014 at 01:20:56PM -0400, Johannes Weiner wrote:
> Hi Mel,
>
> On Mon, Jun 23, 2014 at 02:07:05PM +0100, Mel Gorman wrote:
> > On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > > Page reclaim for a higher-order page runs until compaction is ready,
> > > then aborts and signals this situation through the return value of
> > > shrink_zones(). This is an oddly specific signal to encode in the
> > > return value of shrink_zones(), though, and can be quite confusing.
> > >
> > > Introduce sc->compaction_ready and signal the compactability of the
> > > zones out-of-band to free up the return value of shrink_zones() for
> > > actual zone reclaimability.
> > >
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > ---
> > > mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> > > 1 file changed, 31 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 19b5b8016209..ed1efb84c542 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -65,6 +65,9 @@ struct scan_control {
> > > /* Number of pages freed so far during a call to shrink_zones() */
> > > unsigned long nr_reclaimed;
> > >
> > > + /* One of the zones is ready for compaction */
> > > + int compaction_ready;
> > > +
> > > /* How many pages shrink_list() should reclaim */
> > > unsigned long nr_to_reclaim;
> > >
> >
> > You are not the criminal here but scan_control is larger than it needs
> > to be and the stack usage of reclaim has reared its head again.
> >
> > Add a preparation patch that convert sc->may* and sc->hibernation_mode
> > to bool and moves them towards the end of the struct. Then add
> > compaction_ready as a bool.
>
> Good idea, I'll do that.
>
Thanks.
> > > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > > }
> > >
> > > /* Returns true if compaction should go ahead for a high-order request */
> > > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > > +static inline bool compaction_ready(struct zone *zone, int order)
> > >
> > > {
> >
> > Why did you remove the use of sc->order? In this patch there is only one
> > called of compaction_ready and it looks like
> >
> > if (IS_ENABLED(CONFIG_COMPACTION) &&
> > sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> > zonelist_zone_idx(z) <= requested_highidx &&
> > compaction_ready(zone, sc->order)) {
> >
> > So it's unclear why you changed the signature.
>
> Everything else in compaction_ready() is about internal compaction
> requirements, like checking for free pages and deferred compaction,
> whereas this order check is more of a reclaim policy rule according to
> the comment in the caller:
>
> ...
> * Even though compaction is invoked for any
> * non-zero order, only frequent costly order
> * reclamation is disruptive enough to become a
> * noticeable problem, like transparent huge
> * page allocations.
> */
>
> But it's an unrelated in-the-area-anyway change, I can split it out -
> or drop it entirely - if you prefer.
>
It's ok as-is. It just seemed unrelated and seemed to do nothing. I was
wondering if this was a rebasing artifact and some other change that
required it got lost along the way by accident.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [patch 2/4] mm: vmscan: rework compaction-ready signaling in direct reclaim
@ 2014-06-25 9:55 ` Mel Gorman
0 siblings, 0 replies; 42+ messages in thread
From: Mel Gorman @ 2014-06-25 9:55 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Rik van Riel, Michal Hocko, linux-mm, linux-kernel
On Mon, Jun 23, 2014 at 01:20:56PM -0400, Johannes Weiner wrote:
> Hi Mel,
>
> On Mon, Jun 23, 2014 at 02:07:05PM +0100, Mel Gorman wrote:
> > On Fri, Jun 20, 2014 at 12:33:48PM -0400, Johannes Weiner wrote:
> > > Page reclaim for a higher-order page runs until compaction is ready,
> > > then aborts and signals this situation through the return value of
> > > shrink_zones(). This is an oddly specific signal to encode in the
> > > return value of shrink_zones(), though, and can be quite confusing.
> > >
> > > Introduce sc->compaction_ready and signal the compactability of the
> > > zones out-of-band to free up the return value of shrink_zones() for
> > > actual zone reclaimability.
> > >
> > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> > > ---
> > > mm/vmscan.c | 67 ++++++++++++++++++++++++++++---------------------------------
> > > 1 file changed, 31 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 19b5b8016209..ed1efb84c542 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -65,6 +65,9 @@ struct scan_control {
> > > /* Number of pages freed so far during a call to shrink_zones() */
> > > unsigned long nr_reclaimed;
> > >
> > > + /* One of the zones is ready for compaction */
> > > + int compaction_ready;
> > > +
> > > /* How many pages shrink_list() should reclaim */
> > > unsigned long nr_to_reclaim;
> > >
> >
> > You are not the criminal here but scan_control is larger than it needs
> > to be and the stack usage of reclaim has reared its head again.
> >
> > Add a preparation patch that convert sc->may* and sc->hibernation_mode
> > to bool and moves them towards the end of the struct. Then add
> > compaction_ready as a bool.
>
> Good idea, I'll do that.
>
Thanks.
> > > @@ -2292,15 +2295,11 @@ static void shrink_zone(struct zone *zone, struct scan_control *sc)
> > > }
> > >
> > > /* Returns true if compaction should go ahead for a high-order request */
> > > -static inline bool compaction_ready(struct zone *zone, struct scan_control *sc)
> > > +static inline bool compaction_ready(struct zone *zone, int order)
> > >
> > > {
> >
> > Why did you remove the use of sc->order? In this patch there is only one
> > called of compaction_ready and it looks like
> >
> > if (IS_ENABLED(CONFIG_COMPACTION) &&
> > sc->order > PAGE_ALLOC_COSTLY_ORDER &&
> > zonelist_zone_idx(z) <= requested_highidx &&
> > compaction_ready(zone, sc->order)) {
> >
> > So it's unclear why you changed the signature.
>
> Everything else in compaction_ready() is about internal compaction
> requirements, like checking for free pages and deferred compaction,
> whereas this order check is more of a reclaim policy rule according to
> the comment in the caller:
>
> ...
> * Even though compaction is invoked for any
> * non-zero order, only frequent costly order
> * reclamation is disruptive enough to become a
> * noticeable problem, like transparent huge
> * page allocations.
> */
>
> But it's an unrelated in-the-area-anyway change, I can split it out -
> or drop it entirely - if you prefer.
>
It's ok as-is. It just seemed unrelated and seemed to do nothing. I was
wondering if this was a rebasing artifact and some other change that
required it got lost along the way by accident.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 42+ messages in thread