[PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[Huang, Ying]

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

Now, when the page table is walked in the implementation of
/proc/<pid>/pagemap, pmd_soft_dirty() is used for both the PMD huge
page map and the PMD migration entries.  That is wrong,
pmd_swp_soft_dirty() should be used for the PMD migration entries
instead because the different page table entry flag is used.

Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Hugh Dickins <hughd@google.com>
Cc: "Jérôme Glisse" <jglisse@redhat.com>
Cc: Daniel Colascione <dancol@google.com>
Cc: Zi Yan <zi.yan@cs.rutgers.edu>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
---
 fs/proc/task_mmu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 2593a0c609d7..01aad772f8db 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1311,13 +1311,15 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
 		pmd_t pmd = *pmdp;
 		struct page *page = NULL;
 
-		if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
+		if (vma->vm_flags & VM_SOFTDIRTY)
 			flags |= PM_SOFT_DIRTY;
 
 		if (pmd_present(pmd)) {
 			page = pmd_page(pmd);
 
 			flags |= PM_PRESENT;
+			if (pmd_soft_dirty(pmd))
+				flags |= PM_SOFT_DIRTY;
 			if (pm->show_pfn)
 				frame = pmd_pfn(pmd) +
 					((addr & ~PMD_MASK) >> PAGE_SHIFT);
@@ -1329,6 +1331,8 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
 			frame = swp_type(entry) |
 				(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
 			flags |= PM_SWAP;
+			if (pmd_swp_soft_dirty(pmd))
+				flags |= PM_SOFT_DIRTY;
 			VM_BUG_ON(!is_pmd_migration_entry(pmd));
 			page = migration_entry_to_page(entry);
 		}
-- 
2.14.2

Re: [PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[Anshuman Khandual]

On 10/17/2017 01:48 PM, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
> 
> Now, when the page table is walked in the implementation of
> /proc/<pid>/pagemap, pmd_soft_dirty() is used for both the PMD huge
> page map and the PMD migration entries.  That is wrong,
> pmd_swp_soft_dirty() should be used for the PMD migration entries
> instead because the different page table entry flag is used.

Yeah, different flags can be used on various archs to represent
mapped a PMD and a migration PMD entry. Sounds good.

> 
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: "Jérôme Glisse" <jglisse@redhat.com>
> Cc: Daniel Colascione <dancol@google.com>
> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> ---
>  fs/proc/task_mmu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 2593a0c609d7..01aad772f8db 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1311,13 +1311,15 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
>  		pmd_t pmd = *pmdp;
>  		struct page *page = NULL;
>  
> -		if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
> +		if (vma->vm_flags & VM_SOFTDIRTY)
>  			flags |= PM_SOFT_DIRTY;
>  
>  		if (pmd_present(pmd)) {
>  			page = pmd_page(pmd);
>  
>  			flags |= PM_PRESENT;
> +			if (pmd_soft_dirty(pmd))
> +				flags |= PM_SOFT_DIRTY;
>  			if (pm->show_pfn)
>  				frame = pmd_pfn(pmd) +
>  					((addr & ~PMD_MASK) >> PAGE_SHIFT);
> @@ -1329,6 +1331,8 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
>  			frame = swp_type(entry) |
>  				(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
>  			flags |= PM_SWAP;
> +			if (pmd_swp_soft_dirty(pmd))
> +				flags |= PM_SOFT_DIRTY;

Though I was initially skeptical about whether this will compile
on POWER because of lack of a pmd_swp_soft_dirty() definition
but it turns out we have a generic one to fallback on as we dont
define ARCH_ENABLE_THP_MIGRATION yet.

#ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
#ifndef CONFIG_ARCH_ENABLE_THP_MIGRATION
static inline pmd_t pmd_swp_mksoft_dirty(pmd_t pmd)
{
	return pmd;
}

static inline int pmd_swp_soft_dirty(pmd_t pmd)
{
	return 0;
}

static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
{
	return pmd;
}
#endif

Re: [PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[Kirill A. Shutemov]

On Tue, Oct 17, 2017 at 04:18:18PM +0800, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
> 
> Now, when the page table is walked in the implementation of
> /proc/<pid>/pagemap, pmd_soft_dirty() is used for both the PMD huge
> page map and the PMD migration entries.  That is wrong,
> pmd_swp_soft_dirty() should be used for the PMD migration entries
> instead because the different page table entry flag is used.
> 
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: "Jérôme Glisse" <jglisse@redhat.com>
> Cc: Daniel Colascione <dancol@google.com>
> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>

What is effect of the misbehaviour? pagemap reports garbage?

Shoudn't it be in stable@? And maybe add Fixes: <sha1>.

Otherwise, looks good to me.

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[Zi Yan]

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]



Huang, Ying wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
> 
>> On Tue, Oct 17, 2017 at 04:18:18PM +0800, Huang, Ying wrote:
>>> From: Huang Ying <ying.huang@intel.com>
>>>
>>> Now, when the page table is walked in the implementation of
>>> /proc/<pid>/pagemap, pmd_soft_dirty() is used for both the PMD huge
>>> page map and the PMD migration entries.  That is wrong,
>>> pmd_swp_soft_dirty() should be used for the PMD migration entries
>>> instead because the different page table entry flag is used.
>>>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>>> Cc: David Rientjes <rientjes@google.com>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Hugh Dickins <hughd@google.com>
>>> Cc: "J.r.me Glisse" <jglisse@redhat.com>
>>> Cc: Daniel Colascione <dancol@google.com>
>>> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
>>> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> What is effect of the misbehaviour? pagemap reports garbage?
> 
> Yes.  pagemap may report incorrect soft dirty information for PMD
> migration entries.

Thanks for fixing it.

> 
>> Shoudn't it be in stable@? And maybe add Fixes: <sha1>.
> 
> Yes.  Will do that in the next version.

PMD migration is merged in 4.14, which is not final yet. Do we need to
split the patch, so that first hunk(for present PMD entries) goes into
stable and second hunk(for PMD migration entries) goes into 4.14?

-- 
Best Regards,
Yan Zi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 537 bytes --]

Re: [PATCH -mm] mm, pagemap: Fix soft dirty marking for PMD migration entry

[Huang, Ying]

Zi Yan <zi.yan@cs.rutgers.edu> writes:

> Huang, Ying wrote:
>> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
>> 
>>> On Tue, Oct 17, 2017 at 04:18:18PM +0800, Huang, Ying wrote:
>>>> From: Huang Ying <ying.huang@intel.com>
>>>>
>>>> Now, when the page table is walked in the implementation of
>>>> /proc/<pid>/pagemap, pmd_soft_dirty() is used for both the PMD huge
>>>> page map and the PMD migration entries.  That is wrong,
>>>> pmd_swp_soft_dirty() should be used for the PMD migration entries
>>>> instead because the different page table entry flag is used.
>>>>
>>>> Cc: Michal Hocko <mhocko@suse.com>
>>>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>>>> Cc: David Rientjes <rientjes@google.com>
>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>> Cc: Hugh Dickins <hughd@google.com>
>>>> Cc: "J.r.me Glisse" <jglisse@redhat.com>
>>>> Cc: Daniel Colascione <dancol@google.com>
>>>> Cc: Zi Yan <zi.yan@cs.rutgers.edu>
>>>> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
>>>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>>> What is effect of the misbehaviour? pagemap reports garbage?
>> 
>> Yes.  pagemap may report incorrect soft dirty information for PMD
>> migration entries.
>
> Thanks for fixing it.
>
>> 
>>> Shoudn't it be in stable@? And maybe add Fixes: <sha1>.
>> 
>> Yes.  Will do that in the next version.
>
> PMD migration is merged in 4.14, which is not final yet. Do we need to
> split the patch, so that first hunk(for present PMD entries) goes into
> stable and second hunk(for PMD migration entries) goes into 4.14?

Oh, if so, I think we don't need to back port it to stable kernel.  But
we still need Fixes: tag.

Best Regards,
Huang, Ying