| Message ID | 20210921064553.293905-3-o451686892@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Weizhao Ouyang <o451686892@gmail.com> writes: > Sync up MR_DEMOTION to migrate_reason_names and add a synch prompt. > > Fixes: 26aa2d199d6f ("mm/migrate: demote pages during reclaim") > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > Reviewed-by: "Huang, Ying" <ying.huang@intel.com> > --- > include/linux/migrate.h | 6 +++++- > mm/debug.c | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/linux/migrate.h b/include/linux/migrate.h > index 326250996b4e..c8077e936691 100644 > --- a/include/linux/migrate.h > +++ b/include/linux/migrate.h > @@ -19,6 +19,11 @@ struct migration_target_control; > */ > #define MIGRATEPAGE_SUCCESS 0 > > +/* > + * Keep sync with: > + * - macro MIGRATE_REASON in include/trace/events/migrate.h > + * - migrate_reason_names[MR_TYPES] in mm/debug.c > + */ > enum migrate_reason { > MR_COMPACTION, > MR_MEMORY_FAILURE, > @@ -32,7 +37,6 @@ enum migrate_reason { > MR_TYPES > }; > > -/* In mm/debug.c; also keep sync with include/trace/events/migrate.h */ > extern const char *migrate_reason_names[MR_TYPES]; > > #ifdef CONFIG_MIGRATION > diff --git a/mm/debug.c b/mm/debug.c > index e61037cded98..fae0f81ad831 100644 > --- a/mm/debug.c > +++ b/mm/debug.c > @@ -26,6 +26,7 @@ const char *migrate_reason_names[MR_TYPES] = { > "numa_misplaced", > "contig_range", > "longterm_pin", > + "demotion", > }; > > const struct trace_print_flags pageflag_names[] = { Can we add BUILD_BUG_ON() somewhere to capture at least some synchronization issue? Best Regards, Huang, Ying
On 2021/9/21 15:07, Huang, Ying wrote: > Weizhao Ouyang <o451686892@gmail.com> writes: > >> Sync up MR_DEMOTION to migrate_reason_names and add a synch prompt. >> >> Fixes: 26aa2d199d6f ("mm/migrate: demote pages during reclaim") >> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> >> Reviewed-by: "Huang, Ying" <ying.huang@intel.com> >> --- >> include/linux/migrate.h | 6 +++++- >> mm/debug.c | 1 + >> 2 files changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/migrate.h b/include/linux/migrate.h >> index 326250996b4e..c8077e936691 100644 >> --- a/include/linux/migrate.h >> +++ b/include/linux/migrate.h >> @@ -19,6 +19,11 @@ struct migration_target_control; >> */ >> #define MIGRATEPAGE_SUCCESS 0 >> >> +/* >> + * Keep sync with: >> + * - macro MIGRATE_REASON in include/trace/events/migrate.h >> + * - migrate_reason_names[MR_TYPES] in mm/debug.c >> + */ >> enum migrate_reason { >> MR_COMPACTION, >> MR_MEMORY_FAILURE, >> @@ -32,7 +37,6 @@ enum migrate_reason { >> MR_TYPES >> }; >> >> -/* In mm/debug.c; also keep sync with include/trace/events/migrate.h */ >> extern const char *migrate_reason_names[MR_TYPES]; >> >> #ifdef CONFIG_MIGRATION >> diff --git a/mm/debug.c b/mm/debug.c >> index e61037cded98..fae0f81ad831 100644 >> --- a/mm/debug.c >> +++ b/mm/debug.c >> @@ -26,6 +26,7 @@ const char *migrate_reason_names[MR_TYPES] = { >> "numa_misplaced", >> "contig_range", >> "longterm_pin", >> + "demotion", >> }; >> >> const struct trace_print_flags pageflag_names[] = { > Can we add BUILD_BUG_ON() somewhere to capture at least some > synchronization issue? Hi Huang, we discussed this in the v1 thread with you and John, seems you missed it. Now we just add a comment to do the synchronization, and we can figure out a more general way to use strings which in trace_events straight. Thanks, Weizhao
Weizhao Ouyang <o451686892@gmail.com> writes: > On 2021/9/21 15:07, Huang, Ying wrote: >> Weizhao Ouyang <o451686892@gmail.com> writes: >> >>> Sync up MR_DEMOTION to migrate_reason_names and add a synch prompt. >>> >>> Fixes: 26aa2d199d6f ("mm/migrate: demote pages during reclaim") >>> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> >>> Reviewed-by: "Huang, Ying" <ying.huang@intel.com> >>> --- >>> include/linux/migrate.h | 6 +++++- >>> mm/debug.c | 1 + >>> 2 files changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h >>> index 326250996b4e..c8077e936691 100644 >>> --- a/include/linux/migrate.h >>> +++ b/include/linux/migrate.h >>> @@ -19,6 +19,11 @@ struct migration_target_control; >>> */ >>> #define MIGRATEPAGE_SUCCESS 0 >>> >>> +/* >>> + * Keep sync with: >>> + * - macro MIGRATE_REASON in include/trace/events/migrate.h >>> + * - migrate_reason_names[MR_TYPES] in mm/debug.c >>> + */ >>> enum migrate_reason { >>> MR_COMPACTION, >>> MR_MEMORY_FAILURE, >>> @@ -32,7 +37,6 @@ enum migrate_reason { >>> MR_TYPES >>> }; >>> >>> -/* In mm/debug.c; also keep sync with include/trace/events/migrate.h */ >>> extern const char *migrate_reason_names[MR_TYPES]; >>> >>> #ifdef CONFIG_MIGRATION >>> diff --git a/mm/debug.c b/mm/debug.c >>> index e61037cded98..fae0f81ad831 100644 >>> --- a/mm/debug.c >>> +++ b/mm/debug.c >>> @@ -26,6 +26,7 @@ const char *migrate_reason_names[MR_TYPES] = { >>> "numa_misplaced", >>> "contig_range", >>> "longterm_pin", >>> + "demotion", >>> }; >>> >>> const struct trace_print_flags pageflag_names[] = { >> Can we add BUILD_BUG_ON() somewhere to capture at least some >> synchronization issue? > > Hi Huang, we discussed this in the v1 thread with you and John, seems you > missed it. Now we just add a comment to do the synchronization, and we can > figure out a more general way to use strings which in trace_events straight. Got it! And I think we can add the BUILD_BUG_ON() now and delete it when we have a better solution to deal with that. But if you can work out a better solution quickly, that's fine to ignore this. Best Regards, Huang, Ying
On 9/21/21 07:06, Huang, Ying wrote: ... >>> Can we add BUILD_BUG_ON() somewhere to capture at least some >>> synchronization issue? >> >> Hi Huang, we discussed this in the v1 thread with you and John, seems you >> missed it. Now we just add a comment to do the synchronization, and we can >> figure out a more general way to use strings which in trace_events straight. > > Got it! And I think we can add the BUILD_BUG_ON() now and delete it > when we have a better solution to deal with that. But if you can work > out a better solution quickly, that's fine to ignore this. > I have a solution now, it's basically what I sent earlier. There appears to be some confusion about it. I'll send it out as a patch that builds on top of these two, hopefully in a few hours, if no problems pop up during testing. thanks,
On 9/21/21 11:00, John Hubbard wrote: > On 9/21/21 07:06, Huang, Ying wrote: > ... >>>> Can we add BUILD_BUG_ON() somewhere to capture at least some >>>> synchronization issue? >>> >>> Hi Huang, we discussed this in the v1 thread with you and John, seems you >>> missed it. Now we just add a comment to do the synchronization, and we can >>> figure out a more general way to use strings which in trace_events straight. >> >> Got it! And I think we can add the BUILD_BUG_ON() now and delete it >> when we have a better solution to deal with that. But if you can work >> out a better solution quickly, that's fine to ignore this. >> > > I have a solution now, it's basically what I sent earlier. There appears to be > some confusion about it. I'll send it out as a patch that builds on top of > these two, hopefully in a few hours, if no problems pop up during testing. > Oh OK, I think the confusion was on my end: you are hoping to integrate the TRACE_DEFINE_ENUM in with this. Let me take a peek there, because otherwise, we can only reduce, but not fully remove the duplication. thanks,
John Hubbard <jhubbard@nvidia.com> writes: > On 9/21/21 07:06, Huang, Ying wrote: > ... >>>> Can we add BUILD_BUG_ON() somewhere to capture at least some >>>> synchronization issue? >>> >>> Hi Huang, we discussed this in the v1 thread with you and John, seems you >>> missed it. Now we just add a comment to do the synchronization, and we can >>> figure out a more general way to use strings which in trace_events straight. >> Got it! And I think we can add the BUILD_BUG_ON() now and delete it >> when we have a better solution to deal with that. But if you can work >> out a better solution quickly, that's fine to ignore this. >> > > I have a solution now, it's basically what I sent earlier. There appears to be > some confusion about it. I'll send it out as a patch that builds on top of > these two, hopefully in a few hours, if no problems pop up during testing. Sorry, I didn't read your latest email. The solution looks good! Thanks! Best Regards, Huang, Ying
On 9/20/21 23:45, Weizhao Ouyang wrote: > Sync up MR_DEMOTION to migrate_reason_names and add a synch prompt. > > Fixes: 26aa2d199d6f ("mm/migrate: demote pages during reclaim") > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> > Reviewed-by: "Huang, Ying" <ying.huang@intel.com> > --- > include/linux/migrate.h | 6 +++++- > mm/debug.c | 1 + > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/linux/migrate.h b/include/linux/migrate.h > index 326250996b4e..c8077e936691 100644 > --- a/include/linux/migrate.h > +++ b/include/linux/migrate.h > @@ -19,6 +19,11 @@ struct migration_target_control; > */ > #define MIGRATEPAGE_SUCCESS 0 > > +/* > + * Keep sync with: > + * - macro MIGRATE_REASON in include/trace/events/migrate.h > + * - migrate_reason_names[MR_TYPES] in mm/debug.c > + */ > enum migrate_reason { > MR_COMPACTION, > MR_MEMORY_FAILURE, > @@ -32,7 +37,6 @@ enum migrate_reason { > MR_TYPES > }; > > -/* In mm/debug.c; also keep sync with include/trace/events/migrate.h */ > extern const char *migrate_reason_names[MR_TYPES]; > > #ifdef CONFIG_MIGRATION > diff --git a/mm/debug.c b/mm/debug.c > index e61037cded98..fae0f81ad831 100644 > --- a/mm/debug.c > +++ b/mm/debug.c > @@ -26,6 +26,7 @@ const char *migrate_reason_names[MR_TYPES] = { > "numa_misplaced", > "contig_range", > "longterm_pin", > + "demotion", > }; > > const struct trace_print_flags pageflag_names[] = { > Looks good. Reviewed-by: John Hubbard <jhubbard@nvidia.com> thanks,
diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 326250996b4e..c8077e936691 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -19,6 +19,11 @@ struct migration_target_control; */ #define MIGRATEPAGE_SUCCESS 0 +/* + * Keep sync with: + * - macro MIGRATE_REASON in include/trace/events/migrate.h + * - migrate_reason_names[MR_TYPES] in mm/debug.c + */ enum migrate_reason { MR_COMPACTION, MR_MEMORY_FAILURE, @@ -32,7 +37,6 @@ enum migrate_reason { MR_TYPES }; -/* In mm/debug.c; also keep sync with include/trace/events/migrate.h */ extern const char *migrate_reason_names[MR_TYPES]; #ifdef CONFIG_MIGRATION diff --git a/mm/debug.c b/mm/debug.c index e61037cded98..fae0f81ad831 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -26,6 +26,7 @@ const char *migrate_reason_names[MR_TYPES] = { "numa_misplaced", "contig_range", "longterm_pin", + "demotion", }; const struct trace_print_flags pageflag_names[] = {
Sync up MR_DEMOTION to migrate_reason_names and add a synch prompt. Fixes: 26aa2d199d6f ("mm/migrate: demote pages during reclaim") Signed-off-by: Weizhao Ouyang <o451686892@gmail.com> Reviewed-by: "Huang, Ying" <ying.huang@intel.com> --- include/linux/migrate.h | 6 +++++- mm/debug.c | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)