Subject: [PATCH v1 2/9] mm: add device coherent vma selection for memory migration

This case is used to migrate pages from device memory, back to system
memory. Device coherent type memory is cache coherent from device and CPU
point of view.

Signed-off-by: Alex Sierra <[email protected]>
---
v2:
condition added when migrations from device coherent pages.
---
include/linux/migrate.h | 1 +
mm/migrate.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index c8077e936691..e74bb0978f6f 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -138,6 +138,7 @@ static inline unsigned long migrate_pfn(unsigned long pfn)
enum migrate_vma_direction {
MIGRATE_VMA_SELECT_SYSTEM = 1 << 0,
MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1,
+ MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2,
};

struct migrate_vma {
diff --git a/mm/migrate.c b/mm/migrate.c
index f74422a42192..166bfec7d85e 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2340,8 +2340,6 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
if (is_writable_device_private_entry(entry))
mpfn |= MIGRATE_PFN_WRITE;
} else {
- if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
- goto next;
pfn = pte_pfn(pte);
if (is_zero_pfn(pfn)) {
mpfn = MIGRATE_PFN_MIGRATE;
@@ -2349,6 +2347,13 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
goto next;
}
page = vm_normal_page(migrate->vma, addr, pte);
+ if (!is_zone_device_page(page) &&
+ !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
+ goto next;
+ if (is_zone_device_page(page) &&
+ (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_COHERENT) ||
+ page->pgmap->owner != migrate->pgmap_owner))
+ goto next;
mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
}
--
2.32.0



2021-11-18 06:55:26

by Alistair Popple

[permalink] [raw]
Subject: Re: [PATCH v1 2/9] mm: add device coherent vma selection for memory migration

On Tuesday, 16 November 2021 6:30:19 AM AEDT Alex Sierra wrote:
> This case is used to migrate pages from device memory, back to system
> memory. Device coherent type memory is cache coherent from device and CPU
> point of view.
>
> Signed-off-by: Alex Sierra <[email protected]>
> ---
> v2:
> condition added when migrations from device coherent pages.
> ---
> include/linux/migrate.h | 1 +
> mm/migrate.c | 9 +++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index c8077e936691..e74bb0978f6f 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -138,6 +138,7 @@ static inline unsigned long migrate_pfn(unsigned long pfn)
> enum migrate_vma_direction {
> MIGRATE_VMA_SELECT_SYSTEM = 1 << 0,
> MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1,
> + MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2,
> };
>
> struct migrate_vma {
> diff --git a/mm/migrate.c b/mm/migrate.c
> index f74422a42192..166bfec7d85e 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2340,8 +2340,6 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
> if (is_writable_device_private_entry(entry))
> mpfn |= MIGRATE_PFN_WRITE;
> } else {
> - if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
> - goto next;
> pfn = pte_pfn(pte);
> if (is_zero_pfn(pfn)) {
> mpfn = MIGRATE_PFN_MIGRATE;
> @@ -2349,6 +2347,13 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
> goto next;
> }
> page = vm_normal_page(migrate->vma, addr, pte);
> + if (!is_zone_device_page(page) &&
> + !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
> + goto next;
> + if (is_zone_device_page(page) &&
> + (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_COHERENT) ||
> + page->pgmap->owner != migrate->pgmap_owner))
> + goto next;

Thanks Alex, couple of comments on this:

1. vm_normal_page() can return NULL so you need to add a check for
page == NULL otherwise the call to is_zone_device_page(NULL) will crash.
2. The check for a coherent device page is too indirect. Being explicit and
using is_zone_device_coherent_page() instead would make it more direct and
obvious, particularly for developers who may not immediately realise that
device-private pages should never have pte_present() entries.

> mpfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
> mpfn |= pte_write(pte) ? MIGRATE_PFN_WRITE : 0;
> }
>