On 19 Mar 2024, at 21:09, Zi Yan wrote:
> On 19 Mar 2024, at 21:08, SeongJae Park wrote:
>
>> Hello,
>>
>> On Tue, 19 Mar 2024 11:47:53 -0400 Zi Yan <zi.yan@xxxxxxxx> wrote:
>>
>>> From: Zi Yan <ziy@xxxxxxxxxx>
>>>
>>> If the source folio is on deferred split list, it is likely some subpages
>>> are not used. Split it before migration to avoid migrating unused subpages.
>>>
>>> Commit 616b8371539a6 ("mm: thp: enable thp migration in generic path")
>>> did not check if a THP is on deferred split list before migration, thus,
>>> the destination THP is never put on deferred split list even if the source
>>> THP might be. The opportunity of reclaiming free pages in a partially
>>> mapped THP during deferred list scanning is lost, but no other harmful
>>> consequence is present[1].
>>>
>>> From v2:
>>> 1. Split the source folio instead of migrating it (per Matthew Wilcox)[2].
>>>
>>> From v1:
>>> 1. Used dst to get correct deferred split list after migration
>>> (per Ryan Roberts).
>>>
>>> [1]: https://lore.kernel.org/linux-mm/03CE3A00-917C-48CC-8E1C-6A98713C817C@xxxxxxxxxx/
>>> [2]: https://lore.kernel.org/linux-mm/Ze_P6xagdTbcu1Kz@xxxxxxxxxxxxxxxxxxxx/
>>>
>>> Fixes: 616b8371539a ("mm: thp: enable thp migration in generic path")
>>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>>> ---
>>> mm/huge_memory.c | 22 ------------------
>>> mm/internal.h | 23 +++++++++++++++++++
>>> mm/migrate.c | 60 +++++++++++++++++++++++++++++++++++++++---------
>>> 3 files changed, 72 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 9859aa4f7553..c6d4d0cdf4b3 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -766,28 +766,6 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
>>> return pmd;
>>> }
>>>
>>> -#ifdef CONFIG_MEMCG
>>> -static inline
>>> -struct deferred_split *get_deferred_split_queue(struct folio *folio)
>>> -{
>>> - struct mem_cgroup *memcg = folio_memcg(folio);
>>> - struct pglist_data *pgdat = NODE_DATA(folio_nid(folio));
>>> -
>>> - if (memcg)
>>> - return &memcg->deferred_split_queue;
>>> - else
>>> - return &pgdat->deferred_split_queue;
>>> -}
>>> -#else
>>> -static inline
>>> -struct deferred_split *get_deferred_split_queue(struct folio *folio)
>>> -{
>>> - struct pglist_data *pgdat = NODE_DATA(folio_nid(folio));
>>> -
>>> - return &pgdat->deferred_split_queue;
>>> -}
>>> -#endif
>>> -
>>> void folio_prep_large_rmappable(struct folio *folio)
>>> {
>>> if (!folio || !folio_test_large(folio))
>>> diff --git a/mm/internal.h b/mm/internal.h
>>> index d1c69119b24f..8fa36e84463a 100644
>>> --- a/mm/internal.h
>>> +++ b/mm/internal.h
>>> @@ -1107,6 +1107,29 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
>>> unsigned long addr, pmd_t *pmd,
>>> unsigned int flags);
>>>
>>> +#ifdef CONFIG_MEMCG
>>> +static inline
>>> +struct deferred_split *get_deferred_split_queue(struct folio *folio)
>>> +{
>>> + struct mem_cgroup *memcg = folio_memcg(folio);
>>> + struct pglist_data *pgdat = NODE_DATA(folio_nid(folio));
>>> +
>>> + if (memcg)
>>> + return &memcg->deferred_split_queue;
>>> + else
>>> + return &pgdat->deferred_split_queue;
>>> +}
>>> +#else
>>> +static inline
>>> +struct deferred_split *get_deferred_split_queue(struct folio *folio)
>>> +{
>>> + struct pglist_data *pgdat = NODE_DATA(folio_nid(folio));
>>> +
>>> + return &pgdat->deferred_split_queue;
>>> +}
>>> +#endif
>>
>> I found this breaks the build when CONFIG_TRANSPARENT_HUGEPAGE is not set, with
>> below error:
>>
>> .../lib/../mm/internal.h: In function 'get_deferred_split_queue':
>> .../lib/../mm/internal.h:1127:22: error: 'struct pglist_data' has no member named 'deferred_split_queue'
>> 1127 | return &pgdat->deferred_split_queue;
>> | ^~
>>
>> Since the code was in hugepage.c, maybe the above chunk need to be wrapped by
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE? I confirmed below change is fixing the
>> build on my setup.
>
> Thanks. Will fix it in the next version.
Actually, since get_deferred_split_queue() is used in mm/migrate.c, that
part needs to be guarded by CONFIG_TRANSPARENT_HUGEPAGE as well.
--
Best Regards,
Yan, Zi
Attachment:
signature.asc
Description: OpenPGP digital signature
