Weizhao Ouyang <[email protected]> writes:
> On 2021/9/21 15:07, Huang, Ying wrote:
>> Weizhao Ouyang <[email protected]> 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 <[email protected]>
>>> Reviewed-by: "Huang, Ying" <[email protected]>
>>> ---
>>> 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,
--
John Hubbard
NVIDIA
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
NVIDIA
John Hubbard <[email protected]> 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