diff mbox series

mm/compaction: clear total_{migrate,free}_scanned before scanning a new zone

Message ID 1563789275-9639-1-git-send-email-laoar.shao@gmail.com (mailing list archive)
State New, archived
Headers show
Series mm/compaction: clear total_{migrate,free}_scanned before scanning a new zone | expand

Commit Message

Yafang Shao July 22, 2019, 9:54 a.m. UTC
total_{migrate,free}_scanned will be added to COMPACTMIGRATE_SCANNED and
COMPACTFREE_SCANNED in compact_zone(). We should clear them before scanning
a new zone.
In the proc triggered compaction, we forgot clearing them.

Fixes: 7f354a548d1c ("mm, compaction: add vmstats for kcompactd work")
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Yafang Shao <shaoyafang@didiglobal.com>
---
 mm/compaction.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andrew Morton July 23, 2019, 12:17 a.m. UTC | #1
On Mon, 22 Jul 2019 05:54:35 -0400 Yafang Shao <laoar.shao@gmail.com> wrote:

> total_{migrate,free}_scanned will be added to COMPACTMIGRATE_SCANNED and
> COMPACTFREE_SCANNED in compact_zone(). We should clear them before scanning
> a new zone.
> In the proc triggered compaction, we forgot clearing them.

It isn't the worst bug we've ever had, but I'm thinking we should
backport the fix into -stable kernels?

> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2405,8 +2405,6 @@ static void compact_node(int nid)
>  	struct zone *zone;
>  	struct compact_control cc = {
>  		.order = -1,
> -		.total_migrate_scanned = 0,
> -		.total_free_scanned = 0,
>  		.mode = MIGRATE_SYNC,
>  		.ignore_skip_hint = true,
>  		.whole_zone = true,
> @@ -2422,6 +2420,8 @@ static void compact_node(int nid)
>  
>  		cc.nr_freepages = 0;
>  		cc.nr_migratepages = 0;
> +		cc.total_migrate_scanned = 0;
> +		cc.total_free_scanned = 0;
>  		cc.zone = zone;
>  		INIT_LIST_HEAD(&cc.freepages);
>  		INIT_LIST_HEAD(&cc.migratepages);
Michal Hocko July 23, 2019, 5:36 a.m. UTC | #2
On Mon 22-07-19 05:54:35, Yafang Shao wrote:
> total_{migrate,free}_scanned will be added to COMPACTMIGRATE_SCANNED and
> COMPACTFREE_SCANNED in compact_zone(). We should clear them before scanning
> a new zone.
> In the proc triggered compaction, we forgot clearing them.

Wouldn't it be more robust to move zeroying to compact_zone so that none
of the three current callers has to duplicate (and forget) to do that?

> Fixes: 7f354a548d1c ("mm, compaction: add vmstats for kcompactd work")
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Yafang Shao <shaoyafang@didiglobal.com>
> ---
>  mm/compaction.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 9e1b9ac..a109b45 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -2405,8 +2405,6 @@ static void compact_node(int nid)
>  	struct zone *zone;
>  	struct compact_control cc = {
>  		.order = -1,
> -		.total_migrate_scanned = 0,
> -		.total_free_scanned = 0,
>  		.mode = MIGRATE_SYNC,
>  		.ignore_skip_hint = true,
>  		.whole_zone = true,
> @@ -2422,6 +2420,8 @@ static void compact_node(int nid)
>  
>  		cc.nr_freepages = 0;
>  		cc.nr_migratepages = 0;
> +		cc.total_migrate_scanned = 0;
> +		cc.total_free_scanned = 0;
>  		cc.zone = zone;
>  		INIT_LIST_HEAD(&cc.freepages);
>  		INIT_LIST_HEAD(&cc.migratepages);
> -- 
> 1.8.3.1
Yafang Shao July 23, 2019, 5:55 a.m. UTC | #3
On Tue, Jul 23, 2019 at 1:36 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Mon 22-07-19 05:54:35, Yafang Shao wrote:
> > total_{migrate,free}_scanned will be added to COMPACTMIGRATE_SCANNED and
> > COMPACTFREE_SCANNED in compact_zone(). We should clear them before scanning
> > a new zone.
> > In the proc triggered compaction, we forgot clearing them.
>
> Wouldn't it be more robust to move zeroying to compact_zone so that none
> of the three current callers has to duplicate (and forget) to do that?
>

Seems that is better.
I will post an update.

Thanks
Yafang

> > Fixes: 7f354a548d1c ("mm, compaction: add vmstats for kcompactd work")
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Yafang Shao <shaoyafang@didiglobal.com>
> > ---
> >  mm/compaction.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 9e1b9ac..a109b45 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -2405,8 +2405,6 @@ static void compact_node(int nid)
> >       struct zone *zone;
> >       struct compact_control cc = {
> >               .order = -1,
> > -             .total_migrate_scanned = 0,
> > -             .total_free_scanned = 0,
> >               .mode = MIGRATE_SYNC,
> >               .ignore_skip_hint = true,
> >               .whole_zone = true,
> > @@ -2422,6 +2420,8 @@ static void compact_node(int nid)
> >
> >               cc.nr_freepages = 0;
> >               cc.nr_migratepages = 0;
> > +             cc.total_migrate_scanned = 0;
> > +             cc.total_free_scanned = 0;
> >               cc.zone = zone;
> >               INIT_LIST_HEAD(&cc.freepages);
> >               INIT_LIST_HEAD(&cc.migratepages);
> > --
> > 1.8.3.1
>
> --
> Michal Hocko
> SUSE Labs
Mel Gorman July 23, 2019, 6:59 a.m. UTC | #4
On Mon, Jul 22, 2019 at 05:17:00PM -0700, Andrew Morton wrote:
> On Mon, 22 Jul 2019 05:54:35 -0400 Yafang Shao <laoar.shao@gmail.com> wrote:
> 
> > total_{migrate,free}_scanned will be added to COMPACTMIGRATE_SCANNED and
> > COMPACTFREE_SCANNED in compact_zone(). We should clear them before scanning
> > a new zone.
> > In the proc triggered compaction, we forgot clearing them.
> 
> It isn't the worst bug we've ever had, but I'm thinking we should
> backport the fix into -stable kernels?
> 

There is no harm in having it in -stable. It may matter for those trying
to debug excessive compaction activity and getting misleading stats.
diff mbox series

Patch

diff --git a/mm/compaction.c b/mm/compaction.c
index 9e1b9ac..a109b45 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2405,8 +2405,6 @@  static void compact_node(int nid)
 	struct zone *zone;
 	struct compact_control cc = {
 		.order = -1,
-		.total_migrate_scanned = 0,
-		.total_free_scanned = 0,
 		.mode = MIGRATE_SYNC,
 		.ignore_skip_hint = true,
 		.whole_zone = true,
@@ -2422,6 +2420,8 @@  static void compact_node(int nid)
 
 		cc.nr_freepages = 0;
 		cc.nr_migratepages = 0;
+		cc.total_migrate_scanned = 0;
+		cc.total_free_scanned = 0;
 		cc.zone = zone;
 		INIT_LIST_HEAD(&cc.freepages);
 		INIT_LIST_HEAD(&cc.migratepages);