[PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Huang, Ying]

From: Huang Ying <ying.huang@intel.com>

Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
is added.

Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
pages under migration may be lost as follows.

7f3f6a7e5000-7f3f837e5000 rw-p 00000000 00:00 0
Size:             409600 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:              407552 kB
Pss:              407552 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:    407552 kB
Referenced:       301056 kB
Anonymous:        407552 kB
LazyFree:              0 kB
AnonHugePages:    405504 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:        0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:		1
VmFlags: rd wr mr mw me ac

After the patch, it will be always,

7f3f6a7e5000-7f3f837e5000 rw-p 00000000 00:00 0
Size:             409600 kB
KernelPageSize:        4 kB
MMUPageSize:           4 kB
Rss:              409600 kB
Pss:              409600 kB
Shared_Clean:          0 kB
Shared_Dirty:          0 kB
Private_Clean:         0 kB
Private_Dirty:    409600 kB
Referenced:       294912 kB
Anonymous:        409600 kB
LazyFree:              0 kB
AnonHugePages:    407552 kB
ShmemPmdMapped:        0 kB
FilePmdMapped:        0 kB
Shared_Hugetlb:        0 kB
Private_Hugetlb:       0 kB
Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:		1
VmFlags: rd wr mr mw me ac

Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Yang Shi <yang.shi@linux.alibaba.com>
---

v2:

- Use thp_migration_supported() in condition to reduce code size if THP
  migration isn't enabled.

- Replace VM_BUG_ON() with VM_WARN_ON_ONCE(), it's not necessary to nuking
  kernel for this.

---
 fs/proc/task_mmu.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 8d382d4ec067..9c72f9ce2dd8 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
 	struct mem_size_stats *mss = walk->private;
 	struct vm_area_struct *vma = walk->vma;
 	bool locked = !!(vma->vm_flags & VM_LOCKED);
-	struct page *page;
+	struct page *page = NULL;
 
-	/* FOLL_DUMP will return -EFAULT on huge zero page */
-	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
+	if (pmd_present(*pmd)) {
+		/* FOLL_DUMP will return -EFAULT on huge zero page */
+		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
+	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
+		swp_entry_t entry = pmd_to_swp_entry(*pmd);
+
+		if (is_migration_entry(entry))
+			page = migration_entry_to_page(entry);
+		else
+			VM_WARN_ON_ONCE(1);
+	}
 	if (IS_ERR_OR_NULL(page))
 		return;
 	if (PageAnon(page))
@@ -578,8 +587,7 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 
 	ptl = pmd_trans_huge_lock(pmd, vma);
 	if (ptl) {
-		if (pmd_present(*pmd))
-			smaps_pmd_entry(pmd, addr, walk);
+		smaps_pmd_entry(pmd, addr, walk);
 		spin_unlock(ptl);
 		goto out;
 	}
-- 
2.25.0

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Michal Hocko]

On Thu 02-04-20 10:00:31, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
> 
> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
> is added.
> 
> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
> pages under migration may be lost as follows.

Interesting. How did you reproduce this?
[...]

> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 8d382d4ec067..9c72f9ce2dd8 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>  	struct mem_size_stats *mss = walk->private;
>  	struct vm_area_struct *vma = walk->vma;
>  	bool locked = !!(vma->vm_flags & VM_LOCKED);
> -	struct page *page;
> +	struct page *page = NULL;
>  
> -	/* FOLL_DUMP will return -EFAULT on huge zero page */
> -	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
> +	if (pmd_present(*pmd)) {
> +		/* FOLL_DUMP will return -EFAULT on huge zero page */
> +		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
> +	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
> +		swp_entry_t entry = pmd_to_swp_entry(*pmd);
> +
> +		if (is_migration_entry(entry))
> +			page = migration_entry_to_page(entry);
> +		else
> +			VM_WARN_ON_ONCE(1);

Could you explain why do we need this WARN_ON? I haven't really checked
the swap support for THP but cannot we have normal swap pmd entries?

> +	}
>  	if (IS_ERR_OR_NULL(page))
>  		return;
>  	if (PageAnon(page))
> @@ -578,8 +587,7 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  
>  	ptl = pmd_trans_huge_lock(pmd, vma);
>  	if (ptl) {
> -		if (pmd_present(*pmd))
> -			smaps_pmd_entry(pmd, addr, walk);
> +		smaps_pmd_entry(pmd, addr, walk);
>  		spin_unlock(ptl);
>  		goto out;
>  	}
> -- 
> 2.25.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Huang, Ying]

Michal Hocko <mhocko@kernel.org> writes:

> On Thu 02-04-20 10:00:31, Huang, Ying wrote:
>> From: Huang Ying <ying.huang@intel.com>
>> 
>> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
>> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
>> is added.
>> 
>> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
>> pages under migration may be lost as follows.
>
> Interesting. How did you reproduce this?
> [...]

I run the pmbench in background to eat memory, then run
`/usr/bin/migratepages` and `cat /proc/PID/smaps` every second.  The
issue can be reproduced within 60 seconds.

>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index 8d382d4ec067..9c72f9ce2dd8 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>>  	struct mem_size_stats *mss = walk->private;
>>  	struct vm_area_struct *vma = walk->vma;
>>  	bool locked = !!(vma->vm_flags & VM_LOCKED);
>> -	struct page *page;
>> +	struct page *page = NULL;
>>  
>> -	/* FOLL_DUMP will return -EFAULT on huge zero page */
>> -	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>> +	if (pmd_present(*pmd)) {
>> +		/* FOLL_DUMP will return -EFAULT on huge zero page */
>> +		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>> +	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
>> +		swp_entry_t entry = pmd_to_swp_entry(*pmd);
>> +
>> +		if (is_migration_entry(entry))
>> +			page = migration_entry_to_page(entry);
>> +		else
>> +			VM_WARN_ON_ONCE(1);
>
> Could you explain why do we need this WARN_ON? I haven't really checked
> the swap support for THP but cannot we have normal swap pmd entries?

I have some patches to add the swap pmd entry support, but they haven't
been merged yet.

Similar checks are for all THP migration code paths, so I follow the
same style.

Best Regards,
Huang, Ying

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Michal Hocko]

On Thu 02-04-20 15:03:23, Huang, Ying wrote:
> Michal Hocko <mhocko@kernel.org> writes:
> 
> > On Thu 02-04-20 10:00:31, Huang, Ying wrote:
> >> From: Huang Ying <ying.huang@intel.com>
> >> 
> >> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
> >> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
> >> is added.
> >> 
> >> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
> >> pages under migration may be lost as follows.
> >
> > Interesting. How did you reproduce this?
> > [...]
> 
> I run the pmbench in background to eat memory, then run
> `/usr/bin/migratepages` and `cat /proc/PID/smaps` every second.  The
> issue can be reproduced within 60 seconds.

Please add that information to the changelog. I was probably too
optimistic about the migration duration because I found it highly
unlikely to be visible. I was clearly wrong here.

> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >> index 8d382d4ec067..9c72f9ce2dd8 100644
> >> --- a/fs/proc/task_mmu.c
> >> +++ b/fs/proc/task_mmu.c
> >> @@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
> >>  	struct mem_size_stats *mss = walk->private;
> >>  	struct vm_area_struct *vma = walk->vma;
> >>  	bool locked = !!(vma->vm_flags & VM_LOCKED);
> >> -	struct page *page;
> >> +	struct page *page = NULL;
> >>  
> >> -	/* FOLL_DUMP will return -EFAULT on huge zero page */
> >> -	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
> >> +	if (pmd_present(*pmd)) {
> >> +		/* FOLL_DUMP will return -EFAULT on huge zero page */
> >> +		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
> >> +	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
> >> +		swp_entry_t entry = pmd_to_swp_entry(*pmd);
> >> +
> >> +		if (is_migration_entry(entry))
> >> +			page = migration_entry_to_page(entry);
> >> +		else
> >> +			VM_WARN_ON_ONCE(1);
> >
> > Could you explain why do we need this WARN_ON? I haven't really checked
> > the swap support for THP but cannot we have normal swap pmd entries?
> 
> I have some patches to add the swap pmd entry support, but they haven't
> been merged yet.
> 
> Similar checks are for all THP migration code paths, so I follow the
> same style.

I haven't checked other migration code paths but what is the reason to
add the warning here? Even if this shouldn't happen, smaps is perfectly
fine to ignore that situation, no?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Huang, Ying]

Michal Hocko <mhocko@kernel.org> writes:

> On Thu 02-04-20 15:03:23, Huang, Ying wrote:
>> Michal Hocko <mhocko@kernel.org> writes:
>> 
>> > On Thu 02-04-20 10:00:31, Huang, Ying wrote:
>> >> From: Huang Ying <ying.huang@intel.com>
>> >> 
>> >> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
>> >> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
>> >> is added.
>> >> 
>> >> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
>> >> pages under migration may be lost as follows.
>> >
>> > Interesting. How did you reproduce this?
>> > [...]
>> 
>> I run the pmbench in background to eat memory, then run
>> `/usr/bin/migratepages` and `cat /proc/PID/smaps` every second.  The
>> issue can be reproduced within 60 seconds.
>
> Please add that information to the changelog. I was probably too
> optimistic about the migration duration because I found it highly
> unlikely to be visible. I was clearly wrong here.

Sure.  Will add that in the next version.

>> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> >> index 8d382d4ec067..9c72f9ce2dd8 100644
>> >> --- a/fs/proc/task_mmu.c
>> >> +++ b/fs/proc/task_mmu.c
>> >> @@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>> >>  	struct mem_size_stats *mss = walk->private;
>> >>  	struct vm_area_struct *vma = walk->vma;
>> >>  	bool locked = !!(vma->vm_flags & VM_LOCKED);
>> >> -	struct page *page;
>> >> +	struct page *page = NULL;
>> >>  
>> >> -	/* FOLL_DUMP will return -EFAULT on huge zero page */
>> >> -	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>> >> +	if (pmd_present(*pmd)) {
>> >> +		/* FOLL_DUMP will return -EFAULT on huge zero page */
>> >> +		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>> >> +	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
>> >> +		swp_entry_t entry = pmd_to_swp_entry(*pmd);
>> >> +
>> >> +		if (is_migration_entry(entry))
>> >> +			page = migration_entry_to_page(entry);
>> >> +		else
>> >> +			VM_WARN_ON_ONCE(1);
>> >
>> > Could you explain why do we need this WARN_ON? I haven't really checked
>> > the swap support for THP but cannot we have normal swap pmd entries?
>> 
>> I have some patches to add the swap pmd entry support, but they haven't
>> been merged yet.
>> 
>> Similar checks are for all THP migration code paths, so I follow the
>> same style.
>
> I haven't checked other migration code paths but what is the reason to
> add the warning here? Even if this shouldn't happen, smaps is perfectly
> fine to ignore that situation, no?

Yes. smaps itself is perfectly fine to ignore it.  I think this is used
to find bugs in other code paths such as THP migration related.

Best Regards,
Huang, Ying

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Michal Hocko]

On Thu 02-04-20 16:10:29, Huang, Ying wrote:
> Michal Hocko <mhocko@kernel.org> writes:
> 
> > On Thu 02-04-20 15:03:23, Huang, Ying wrote:
[...]
> >> > Could you explain why do we need this WARN_ON? I haven't really checked
> >> > the swap support for THP but cannot we have normal swap pmd entries?
> >> 
> >> I have some patches to add the swap pmd entry support, but they haven't
> >> been merged yet.
> >> 
> >> Similar checks are for all THP migration code paths, so I follow the
> >> same style.
> >
> > I haven't checked other migration code paths but what is the reason to
> > add the warning here? Even if this shouldn't happen, smaps is perfectly
> > fine to ignore that situation, no?
> 
> Yes. smaps itself is perfectly fine to ignore it.  I think this is used
> to find bugs in other code paths such as THP migration related.

Please do not add new warnings without a good an strong reasons. As a
matter of fact there are people running with panic_on_warn and each
warning is fatal for them. Please also note that this is a user trigable
path and that requires even more care.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Huang, Ying]

Michal Hocko <mhocko@kernel.org> writes:

> On Thu 02-04-20 16:10:29, Huang, Ying wrote:
>> Michal Hocko <mhocko@kernel.org> writes:
>> 
>> > On Thu 02-04-20 15:03:23, Huang, Ying wrote:
> [...]
>> >> > Could you explain why do we need this WARN_ON? I haven't really checked
>> >> > the swap support for THP but cannot we have normal swap pmd entries?
>> >> 
>> >> I have some patches to add the swap pmd entry support, but they haven't
>> >> been merged yet.
>> >> 
>> >> Similar checks are for all THP migration code paths, so I follow the
>> >> same style.
>> >
>> > I haven't checked other migration code paths but what is the reason to
>> > add the warning here? Even if this shouldn't happen, smaps is perfectly
>> > fine to ignore that situation, no?
>> 
>> Yes. smaps itself is perfectly fine to ignore it.  I think this is used
>> to find bugs in other code paths such as THP migration related.
>
> Please do not add new warnings without a good an strong reasons. As a
> matter of fact there are people running with panic_on_warn and each
> warning is fatal for them. Please also note that this is a user trigable
> path and that requires even more care.

OK for me.

Best Regards,
Huang, Ying

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Konstantin Khlebnikov]

On 02/04/2020 11.21, Michal Hocko wrote:
> On Thu 02-04-20 16:10:29, Huang, Ying wrote:
>> Michal Hocko <mhocko@kernel.org> writes:
>>
>>> On Thu 02-04-20 15:03:23, Huang, Ying wrote:
> [...]
>>>>> Could you explain why do we need this WARN_ON? I haven't really checked
>>>>> the swap support for THP but cannot we have normal swap pmd entries?
>>>>
>>>> I have some patches to add the swap pmd entry support, but they haven't
>>>> been merged yet.
>>>>
>>>> Similar checks are for all THP migration code paths, so I follow the
>>>> same style.
>>>
>>> I haven't checked other migration code paths but what is the reason to
>>> add the warning here? Even if this shouldn't happen, smaps is perfectly
>>> fine to ignore that situation, no?
>>
>> Yes. smaps itself is perfectly fine to ignore it.  I think this is used
>> to find bugs in other code paths such as THP migration related.
> 
> Please do not add new warnings without a good an strong reasons. As a
> matter of fact there are people running with panic_on_warn and each
> warning is fatal for them. Please also note that this is a user trigable
> path and that requires even more care.
> 

But this should not happen and if it does we'll never know without debug.
VM_WARN_ON checks something only if build with CONFIG_DEBUG_VM=y.

Anybody who runs debug kernels with panic_on_warn shouldn't expect much stability =)

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Michal Hocko]

On Thu 02-04-20 11:29:09, Konstantin Khlebnikov wrote:
> 
> 
> On 02/04/2020 11.21, Michal Hocko wrote:
> > On Thu 02-04-20 16:10:29, Huang, Ying wrote:
> > > Michal Hocko <mhocko@kernel.org> writes:
> > > 
> > > > On Thu 02-04-20 15:03:23, Huang, Ying wrote:
> > [...]
> > > > > > Could you explain why do we need this WARN_ON? I haven't really checked
> > > > > > the swap support for THP but cannot we have normal swap pmd entries?
> > > > > 
> > > > > I have some patches to add the swap pmd entry support, but they haven't
> > > > > been merged yet.
> > > > > 
> > > > > Similar checks are for all THP migration code paths, so I follow the
> > > > > same style.
> > > > 
> > > > I haven't checked other migration code paths but what is the reason to
> > > > add the warning here? Even if this shouldn't happen, smaps is perfectly
> > > > fine to ignore that situation, no?
> > > 
> > > Yes. smaps itself is perfectly fine to ignore it.  I think this is used
> > > to find bugs in other code paths such as THP migration related.
> > 
> > Please do not add new warnings without a good an strong reasons. As a
> > matter of fact there are people running with panic_on_warn and each
> > warning is fatal for them. Please also note that this is a user trigable
> > path and that requires even more care.
> > 
> 
> But this should not happen and if it does we'll never know without debug.

The migration path which already deals with this will notice, right?
Those are paths which really care about consistency.

> VM_WARN_ON checks something only if build with CONFIG_DEBUG_VM=y.
> 
> Anybody who runs debug kernels with panic_on_warn shouldn't expect much stability =)

That doesn't mean we should be adding warnings here and there nilly
willy.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Yang Shi]

On 4/2/20 12:44 AM, Michal Hocko wrote:
> On Thu 02-04-20 15:03:23, Huang, Ying wrote:
>> Michal Hocko <mhocko@kernel.org> writes:
>>
>>> On Thu 02-04-20 10:00:31, Huang, Ying wrote:
>>>> From: Huang Ying <ying.huang@intel.com>
>>>>
>>>> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
>>>> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
>>>> is added.
>>>>
>>>> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
>>>> pages under migration may be lost as follows.
>>> Interesting. How did you reproduce this?
>>> [...]
>> I run the pmbench in background to eat memory, then run
>> `/usr/bin/migratepages` and `cat /proc/PID/smaps` every second.  The
>> issue can be reproduced within 60 seconds.
> Please add that information to the changelog. I was probably too
> optimistic about the migration duration because I found it highly
> unlikely to be visible. I was clearly wrong here.

I believe that depends on the page is shared by how many processes. If 
it is not shared then it should just take dozens micro seconds in my 
test FYI.

>
>>>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>>>> index 8d382d4ec067..9c72f9ce2dd8 100644
>>>> --- a/fs/proc/task_mmu.c
>>>> +++ b/fs/proc/task_mmu.c
>>>> @@ -546,10 +546,19 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,
>>>>   	struct mem_size_stats *mss = walk->private;
>>>>   	struct vm_area_struct *vma = walk->vma;
>>>>   	bool locked = !!(vma->vm_flags & VM_LOCKED);
>>>> -	struct page *page;
>>>> +	struct page *page = NULL;
>>>>   
>>>> -	/* FOLL_DUMP will return -EFAULT on huge zero page */
>>>> -	page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>>>> +	if (pmd_present(*pmd)) {
>>>> +		/* FOLL_DUMP will return -EFAULT on huge zero page */
>>>> +		page = follow_trans_huge_pmd(vma, addr, pmd, FOLL_DUMP);
>>>> +	} else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) {
>>>> +		swp_entry_t entry = pmd_to_swp_entry(*pmd);
>>>> +
>>>> +		if (is_migration_entry(entry))
>>>> +			page = migration_entry_to_page(entry);
>>>> +		else
>>>> +			VM_WARN_ON_ONCE(1);
>>> Could you explain why do we need this WARN_ON? I haven't really checked
>>> the swap support for THP but cannot we have normal swap pmd entries?
>> I have some patches to add the swap pmd entry support, but they haven't
>> been merged yet.
>>
>> Similar checks are for all THP migration code paths, so I follow the
>> same style.
> I haven't checked other migration code paths but what is the reason to
> add the warning here? Even if this shouldn't happen, smaps is perfectly
> fine to ignore that situation, no?

Re: [PATCH -V2] /proc/PID/smaps: Add PMD migration entry parsing

[Kirill A. Shutemov]

On Thu, Apr 02, 2020 at 10:00:31AM +0800, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
> 
> Now, when read /proc/PID/smaps, the PMD migration entry in page table is simply
> ignored.  To improve the accuracy of /proc/PID/smaps, its parsing and processing
> is added.
> 
> Before the patch, for a fully populated 400 MB anonymous VMA, sometimes some THP
> pages under migration may be lost as follows.
> 
> 7f3f6a7e5000-7f3f837e5000 rw-p 00000000 00:00 0
> Size:             409600 kB
> KernelPageSize:        4 kB
> MMUPageSize:           4 kB
> Rss:              407552 kB
> Pss:              407552 kB
> Shared_Clean:          0 kB
> Shared_Dirty:          0 kB
> Private_Clean:         0 kB
> Private_Dirty:    407552 kB
> Referenced:       301056 kB
> Anonymous:        407552 kB
> LazyFree:              0 kB
> AnonHugePages:    405504 kB
> ShmemPmdMapped:        0 kB
> FilePmdMapped:        0 kB

The alignment makes me triggered.

Andrew, could you please apply this patch:

http://lore.kernel.org/r/20191230084125.267040-1-samuel.williams@oriontransfer.co.nz

-- 
 Kirill A. Shutemov