[PATCH] mm/migrate: properly preserve write attribute in special migrate entry

[PATCH] mm/migrate: properly preserve write attribute in special migrate entry

[jglisse]

From: Ralph Campbell <rcampbell@nvidia.com>

Use of pte_write(pte) is only valid for present pte, the common code
which set the migration entry can be reach for both valid present
pte and special swap entry (for device memory). Fix the code to use
the mpfn value which properly handle both cases.

On x86 this did not have any bad side effect because pte write bit
is below PAGE_BIT_GLOBAL and thus special swap entry have it set to
0 which in turn means we were always creating read only special
migration entry.

So once migration did finish we always write protected the CPU page
table entry (moreover this is only an issue when migrating from device
memory to system memory). End effect is that CPU write access would
fault again and restore write permission.

Signed-off-by: Ralph Campbell <rcampbell@nvidia.com>
Signed-off-by: Jérôme Glisse <jglisse@redhat.com>
---
 mm/migrate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 5d0dc7b85f90..a5c559d8e0e7 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2269,7 +2269,8 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
 			ptep_get_and_clear(mm, addr, ptep);
 
 			/* Setup special migration page table entry */
-			entry = make_migration_entry(page, pte_write(pte));
+			entry = make_migration_entry(page, mpfn &
+						     MIGRATE_PFN_WRITE);
 			swp_pte = swp_entry_to_pte(entry);
 			if (pte_soft_dirty(pte))
 				swp_pte = pte_swp_mksoft_dirty(swp_pte);
-- 
2.14.3

Re: [PATCH] mm/migrate: properly preserve write attribute in special migrate entry

[Andrew Morton]

On Sun,  1 Apr 2018 22:35:06 -0400 jglisse@redhat.com wrote:

> From: Ralph Campbell <rcampbell@nvidia.com>
> 
> Use of pte_write(pte) is only valid for present pte, the common code
> which set the migration entry can be reach for both valid present
> pte and special swap entry (for device memory). Fix the code to use
> the mpfn value which properly handle both cases.
> 
> On x86 this did not have any bad side effect because pte write bit
> is below PAGE_BIT_GLOBAL and thus special swap entry have it set to
> 0 which in turn means we were always creating read only special
> migration entry.

Does this mean that the patch only affects behaviour of non-x86 systems?

> So once migration did finish we always write protected the CPU page
> table entry (moreover this is only an issue when migrating from device
> memory to system memory). End effect is that CPU write access would
> fault again and restore write permission.

That sounds a bit serious.  Was a -stable backport considered?

Re: [PATCH] mm/migrate: properly preserve write attribute in special migrate entry

[Jerome Glisse]

On Tue, Apr 03, 2018 at 03:30:46PM -0700, Andrew Morton wrote:
> On Sun,  1 Apr 2018 22:35:06 -0400 jglisse@redhat.com wrote:
> 
> > From: Ralph Campbell <rcampbell@nvidia.com>
> > 
> > Use of pte_write(pte) is only valid for present pte, the common code
> > which set the migration entry can be reach for both valid present
> > pte and special swap entry (for device memory). Fix the code to use
> > the mpfn value which properly handle both cases.
> > 
> > On x86 this did not have any bad side effect because pte write bit
> > is below PAGE_BIT_GLOBAL and thus special swap entry have it set to
> > 0 which in turn means we were always creating read only special
> > migration entry.
> 
> Does this mean that the patch only affects behaviour of non-x86 systems?

No it affect x86 as explained below (ie it forces a second page fault).

> 
> > So once migration did finish we always write protected the CPU page
> > table entry (moreover this is only an issue when migrating from device
> > memory to system memory). End effect is that CPU write access would
> > fault again and restore write permission.
> 
> That sounds a bit serious.  Was a -stable backport considered?

Like discuss previously with Michal, for lack of upstream user yet
(and PowerPC users of this code are not upstream either yet AFAIK).

Once i get HMM inside nouveau upstream, i will evaluate if people
wants all fixes to be back ported to stable.

Finaly this one isn't too bad, it just burn CPU cycles by forcing
CPU to take a second fault on write access ie double fault the same
address. There is no corruption or incorrect states (it behave as
a COWed page from a fork with a mapcount of 1).


Do you still want me to be more aggressive with stable backport ?
I don't mind either way. I expect to get HMM nouveau upstream over
next couple release cycle.

Cheers,
Jérôme

Re: [PATCH] mm/migrate: properly preserve write attribute in special migrate entry

[Andrew Morton]

On Tue, 3 Apr 2018 19:03:36 -0400 Jerome Glisse <jglisse@redhat.com> wrote:

> > That sounds a bit serious.  Was a -stable backport considered?
> 
> Like discuss previously with Michal, for lack of upstream user yet
> (and PowerPC users of this code are not upstream either yet AFAIK).
> 
> Once i get HMM inside nouveau upstream, i will evaluate if people
> wants all fixes to be back ported to stable.
> 
> Finaly this one isn't too bad, it just burn CPU cycles by forcing
> CPU to take a second fault on write access ie double fault the same
> address. There is no corruption or incorrect states (it behave as
> a COWed page from a fork with a mapcount of 1).

OK, I updated the changelog with this info.

> Do you still want me to be more aggressive with stable backport ?
> I don't mind either way. I expect to get HMM nouveau upstream over
> next couple release cycle.

I guess that doing a single, better-organized cherrypick at a suitable
time in the future is a good approach.  You might want to discuss this
plan with Greg before committing too far.