| 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 |
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);
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
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
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 --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);
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(-)