diff mbox series

[06/14] mm, migrate: Immediately fail migration of a page with no migration handler

Message ID 20181214230310.572-7-mgorman@techsingularity.net (mailing list archive)
State New, archived
Headers show
Series Increase success rates and reduce latency of compaction v1 | expand

Commit Message

Mel Gorman Dec. 14, 2018, 11:03 p.m. UTC
Pages with no migration handler use a fallback hander which sometimes
works and sometimes persistently fails such as blockdev pages. Migration
will retry a number of times on these persistent pages which is wasteful
during compaction. This patch will fail migration immediately unless the
caller is in MIGRATE_SYNC mode which indicates the caller is willing to
wait while being persistent.

This is not expected to help THP allocation success rates but it does
reduce latencies slightly.

1-socket thpfioscale
                                    4.20.0-rc6             4.20.0-rc6
                               noreserved-v1r4          failfast-v1r4
Amean     fault-both-1         0.00 (   0.00%)        0.00 *   0.00%*
Amean     fault-both-3      2276.15 (   0.00%)     3867.54 * -69.92%*
Amean     fault-both-5      4992.20 (   0.00%)     5313.20 (  -6.43%)
Amean     fault-both-7      7373.30 (   0.00%)     7039.11 (   4.53%)
Amean     fault-both-12    11911.52 (   0.00%)    11328.29 (   4.90%)
Amean     fault-both-18    17209.42 (   0.00%)    16455.34 (   4.38%)
Amean     fault-both-24    20943.71 (   0.00%)    20448.94 (   2.36%)
Amean     fault-both-30    22703.00 (   0.00%)    21655.07 (   4.62%)
Amean     fault-both-32    22461.41 (   0.00%)    21415.35 (   4.66%)

The 2-socket results are not materially different. Scan rates are
similar as expected.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 mm/migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vlastimil Babka Dec. 18, 2018, 9:06 a.m. UTC | #1
On 12/15/18 12:03 AM, Mel Gorman wrote:
> Pages with no migration handler use a fallback hander which sometimes
> works and sometimes persistently fails such as blockdev pages. Migration
> will retry a number of times on these persistent pages which is wasteful
> during compaction. This patch will fail migration immediately unless the
> caller is in MIGRATE_SYNC mode which indicates the caller is willing to
> wait while being persistent.

Right.

> This is not expected to help THP allocation success rates but it does
> reduce latencies slightly.
> 
> 1-socket thpfioscale
>                                     4.20.0-rc6             4.20.0-rc6
>                                noreserved-v1r4          failfast-v1r4
> Amean     fault-both-1         0.00 (   0.00%)        0.00 *   0.00%*
> Amean     fault-both-3      2276.15 (   0.00%)     3867.54 * -69.92%*

This is rather weird.

> Amean     fault-both-5      4992.20 (   0.00%)     5313.20 (  -6.43%)
> Amean     fault-both-7      7373.30 (   0.00%)     7039.11 (   4.53%)
> Amean     fault-both-12    11911.52 (   0.00%)    11328.29 (   4.90%)
> Amean     fault-both-18    17209.42 (   0.00%)    16455.34 (   4.38%)
> Amean     fault-both-24    20943.71 (   0.00%)    20448.94 (   2.36%)
> Amean     fault-both-30    22703.00 (   0.00%)    21655.07 (   4.62%)
> Amean     fault-both-32    22461.41 (   0.00%)    21415.35 (   4.66%)
> 
> The 2-socket results are not materially different. Scan rates are
> similar as expected.
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/migrate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index df17a710e2c7..0e27a10429e2 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -885,7 +885,7 @@ static int fallback_migrate_page(struct address_space *mapping,
>  	 */
>  	if (page_has_private(page) &&
>  	    !try_to_release_page(page, GFP_KERNEL))
> -		return -EAGAIN;
> +		return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY;
>  
>  	return migrate_page(mapping, newpage, page, mode);
>  }
>
Mel Gorman Dec. 18, 2018, 9:55 a.m. UTC | #2
On Tue, Dec 18, 2018 at 10:06:31AM +0100, Vlastimil Babka wrote:
> On 12/15/18 12:03 AM, Mel Gorman wrote:
> > Pages with no migration handler use a fallback hander which sometimes
> > works and sometimes persistently fails such as blockdev pages. Migration
> > will retry a number of times on these persistent pages which is wasteful
> > during compaction. This patch will fail migration immediately unless the
> > caller is in MIGRATE_SYNC mode which indicates the caller is willing to
> > wait while being persistent.
> 
> Right.
> 
> > This is not expected to help THP allocation success rates but it does
> > reduce latencies slightly.
> > 
> > 1-socket thpfioscale
> >                                     4.20.0-rc6             4.20.0-rc6
> >                                noreserved-v1r4          failfast-v1r4
> > Amean     fault-both-1         0.00 (   0.00%)        0.00 *   0.00%*
> > Amean     fault-both-3      2276.15 (   0.00%)     3867.54 * -69.92%*
> 
> This is rather weird.
> 

Fault latency is extremely variable and there can be very large outliers
that skew the mean (the full report includes quartiles but it makes for an
excessive changelog). It can be down to luck about how often the migrate
scanner advances and how often it gets reset. For this series, it'll
not be unusual to see jitter in the latencies for individual patches
that will not get nailed down reliably until later in the series. The
alternative is massive patches that do multiple things which will look
nice in changelogs and be horrible to review.

> > Amean     fault-both-5      4992.20 (   0.00%)     5313.20 (  -6.43%)
> > Amean     fault-both-7      7373.30 (   0.00%)     7039.11 (   4.53%)
> > Amean     fault-both-12    11911.52 (   0.00%)    11328.29 (   4.90%)
> > Amean     fault-both-18    17209.42 (   0.00%)    16455.34 (   4.38%)
> > Amean     fault-both-24    20943.71 (   0.00%)    20448.94 (   2.36%)
> > Amean     fault-both-30    22703.00 (   0.00%)    21655.07 (   4.62%)
> > Amean     fault-both-32    22461.41 (   0.00%)    21415.35 (   4.66%)
> > 
> > The 2-socket results are not materially different. Scan rates are
> > similar as expected.
> > 
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 

Thanks.
Yang Shi Dec. 20, 2018, 7:44 p.m. UTC | #3
On Fri, Dec 14, 2018 at 3:03 PM Mel Gorman <mgorman@techsingularity.net> wrote:
>
> Pages with no migration handler use a fallback hander which sometimes
> works and sometimes persistently fails such as blockdev pages. Migration

A minor correction. The above statement sounds not accurate anymore
since Jan Kara had patch series (blkdev: avoid migration stalls for
blkdev pages) have blockdev use its own migration handler.

Thanks,
Yang

> will retry a number of times on these persistent pages which is wasteful
> during compaction. This patch will fail migration immediately unless the
> caller is in MIGRATE_SYNC mode which indicates the caller is willing to
> wait while being persistent.
>
> This is not expected to help THP allocation success rates but it does
> reduce latencies slightly.
>
> 1-socket thpfioscale
>                                     4.20.0-rc6             4.20.0-rc6
>                                noreserved-v1r4          failfast-v1r4
> Amean     fault-both-1         0.00 (   0.00%)        0.00 *   0.00%*
> Amean     fault-both-3      2276.15 (   0.00%)     3867.54 * -69.92%*
> Amean     fault-both-5      4992.20 (   0.00%)     5313.20 (  -6.43%)
> Amean     fault-both-7      7373.30 (   0.00%)     7039.11 (   4.53%)
> Amean     fault-both-12    11911.52 (   0.00%)    11328.29 (   4.90%)
> Amean     fault-both-18    17209.42 (   0.00%)    16455.34 (   4.38%)
> Amean     fault-both-24    20943.71 (   0.00%)    20448.94 (   2.36%)
> Amean     fault-both-30    22703.00 (   0.00%)    21655.07 (   4.62%)
> Amean     fault-both-32    22461.41 (   0.00%)    21415.35 (   4.66%)
>
> The 2-socket results are not materially different. Scan rates are
> similar as expected.
>
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> ---
>  mm/migrate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index df17a710e2c7..0e27a10429e2 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -885,7 +885,7 @@ static int fallback_migrate_page(struct address_space *mapping,
>          */
>         if (page_has_private(page) &&
>             !try_to_release_page(page, GFP_KERNEL))
> -               return -EAGAIN;
> +               return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY;
>
>         return migrate_page(mapping, newpage, page, mode);
>  }
> --
> 2.16.4
>
Mel Gorman Dec. 20, 2018, 8:31 p.m. UTC | #4
On Thu, Dec 20, 2018 at 11:44:57AM -0800, Yang Shi wrote:
> On Fri, Dec 14, 2018 at 3:03 PM Mel Gorman <mgorman@techsingularity.net> wrote:
> >
> > Pages with no migration handler use a fallback hander which sometimes
> > works and sometimes persistently fails such as blockdev pages. Migration
> 
> A minor correction. The above statement sounds not accurate anymore
> since Jan Kara had patch series (blkdev: avoid migration stalls for
> blkdev pages) have blockdev use its own migration handler.
> 

I'm aware given that I reviewed that series. The statement was correct
at the time of writing. I'll alter the example when rebased on top of
Jan's work.
diff mbox series

Patch

diff --git a/mm/migrate.c b/mm/migrate.c
index df17a710e2c7..0e27a10429e2 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -885,7 +885,7 @@  static int fallback_migrate_page(struct address_space *mapping,
 	 */
 	if (page_has_private(page) &&
 	    !try_to_release_page(page, GFP_KERNEL))
-		return -EAGAIN;
+		return mode == MIGRATE_SYNC ? -EAGAIN : -EBUSY;
 
 	return migrate_page(mapping, newpage, page, mode);
 }