Add support for SVM atomics in Nouveau
Subsystems (19)
Alistair Popple
reviewerv3 cover Add support for SVM atomics in Nouveau
This is the third version of a series to add support to Nouveau for atomic memory operations on OpenCL shared virtual memory (SVM) regions. This is achieved using the atomic PTE bits on the GPU to only permit atomic operations to system memory when a page is not mapped in userspace on the CPU. The previous version of this series used an unmap and pin page migration, however this resulted in problems with ZONE_MOVABLE and other issues so this series uses a different approach.
Instead exclusive device access is implemented by adding a new swap entry type (SWAP_DEVICE_EXCLUSIVE) which is similar to a migration entry. The main difference is that on fault the original entry is immediately restored by the fault handler instead of waiting.
Restoring the entry triggers calls to MMU notifers which allows a device driver to revoke the atomic access permission from the GPU prior to the CPU finalising the entry.
Patches 1 & 2 are new for v3 and refactor the existing migration and device private entry functions.
Patches 3 & 4 rework try_to_unmap_one() by splitting out unrelated functionality into separate functions - try_to_migrate_one() and try_to_munlock_one(). These should not change any functionality, but any help testing would be much appreciated as I have not been able to test every single usage of try_to_unmap_one().
Patch 5 contains the bulk of the implementation for device exclusive memory.
Patch 6 contains some additions to the HMM selftests to ensure everything works as expected and has not changed significantly since v2.
Patch 7 was posted previously and has not changed.
Patch 8 was posted for v2 and has not changed significantly.
This has been tested using the latest upstream Mesa userspace with a simple OpenCL test program which checks the results of atomic GPU operations on a SVM buffer whilst also writing to the same buffer from the CPU.
v3:
* Refactored some existing functionality.
* Switched to using get_user_pages_remote() instead of open-coding it.
* Moved code out of hmm.
v2:
* Changed implementation to use special swap entries instead of device private pages.
Alistair Popple (8):
mm: Remove special swap entry functions
mm/swapops: Rework swap entry manipulation code
mm/rmap: Split try_to_munlock from try_to_unmap
mm/rmap: Split migration into its own function
mm: Device exclusive memory access
mm: Selftests for exclusive device memory
nouveau/svm: Refactor nouveau_range_fault
nouveau/svm: Implement atomic SVM access
Documentation/vm/hmm.rst | 15 + arch/s390/mm/pgtable.c | 2 +- drivers/gpu/drm/nouveau/include/nvif/if000c.h | 1 + drivers/gpu/drm/nouveau/nouveau_svm.c | 120 +++- drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 1 + .../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c | 6 + fs/proc/task_mmu.c | 23 +- include/linux/rmap.h | 8 +- include/linux/swap.h | 8 +- include/linux/swapops.h | 105 ++-- lib/test_hmm.c | 124 ++++ lib/test_hmm_uapi.h | 2 + mm/debug_vm_pgtable.c | 12 +- mm/hmm.c | 12 +- mm/huge_memory.c | 36 +- mm/hugetlb.c | 10 +- mm/memcontrol.c | 2 +- mm/memory.c | 128 +++- mm/migrate.c | 41 +- mm/mprotect.c | 18 +- mm/page_vma_mapped.c | 15 +- mm/rmap.c | 558 +++++++++++++++--- tools/testing/selftests/vm/hmm-tests.c | 219 +++++++ 23 files changed, 1207 insertions(+), 259 deletions(-)
Signature · 6 lines
2.20.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerPATCH v3 1/8 mm: Remove special swap entry functions
Remove the migration and device private entry_to_page() and entry_to_pfn() inline functions and instead open code them directly. This results in shorter code which is easier to understand.
--- arch/s390/mm/pgtable.c | 2 +- fs/proc/task_mmu.c | 23 +++++++------------ include/linux/swap.h | 4 ++-- include/linux/swapops.h | 51 ++++------------------------------------- mm/hmm.c | 5 ++-- mm/memcontrol.c | 2 +- mm/memory.c | 10 ++++---- mm/migrate.c | 6 ++--- mm/page_vma_mapped.c | 6 ++--- 9 files changed, 30 insertions(+), 79 deletions(-)
arch/s390/mm/pgtable.c +1 -1
@@ -691,7 +691,7 @@ static void ptep_zap_swap_entry(struct mm_struct *mm, swp_entry_t entry)if (!non_swap_entry (entry))dec_mm_counter (mm,MM_SWAPENTS );else if (is_migration_entry (entry)) { -struct page *page =migration_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));dec_mm_counter (mm,mm_counter (page)); }
fs/proc/task_mmu.c +8 -15
@@ -514,10 +514,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, }else { mss->swap_pss += (u64 )PAGE_SIZE <<PSS_SHIFT ; } - }else if (is_migration_entry (swpent)) - page =migration_entry_to_page (swpent); -else if (is_device_private_entry (swpent)) - page =device_private_entry_to_page (swpent); + }else if (is_special_entry (swpent)) + page =pfn_to_page (swp_offset (swpent)); }else if (unlikely (IS_ENABLED (CONFIG_SHMEM ) && mss->check_shmem_swap &&pte_none (*pte))) { page = xa_load(&vma->vm_file->f_mapping->i_pages,
@@ -549,7 +547,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr,swp_entry_t entry =pmd_to_swp_entry (*pmd);if (is_migration_entry (entry)) - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); }if (IS_ERR_OR_NULL (page))return ;
@@ -691,10 +689,8 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask, }else if (is_swap_pte (*pte)) {swp_entry_t swpent =pte_to_swp_entry (*pte); -if (is_migration_entry (swpent)) - page =migration_entry_to_page (swpent); -else if (is_device_private_entry (swpent)) - page =device_private_entry_to_page (swpent); +if (is_special_entry (swpent)) + page =pfn_to_page (swp_offset (swpent)); }if (page) {int mapcount =page_mapcount (page);
@@ -1382,11 +1378,8 @@ static pagemap_entry_t pte_to_pagemap_entry(struct pagemapread *pm, frame =swp_type (entry) | (swp_offset (entry) <<MAX_SWAPFILES_SHIFT ); flags |=PM_SWAP ; -if (is_migration_entry (entry)) - page =migration_entry_to_page (entry); - -if (is_device_private_entry (entry)) - page =device_private_entry_to_page (entry); +if (is_special_entry (entry)) + page =pfn_to_page (swp_offset (entry)); }if (page && !PageAnon (page))
@@ -1443,7 +1436,7 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,if (pmd_swp_soft_dirty (pmd)) flags |=PM_SOFT_DIRTY ;VM_BUG_ON (!is_pmd_migration_entry (pmd)); - page = migration_entry_to_page(entry); + page = pfn_to_page(swp_offset(entry)); }#endif
include/linux/swap.h +2 -2
@@ -519,8 +519,8 @@ static inline void show_swap_cache_info(void) { } -#define free_swap_and_cache (e) ({(is_migration_entry(e) || is_device_private_entry(e));}) -#define swapcache_prepare (e) ({(is_migration_entry(e) || is_device_private_entry(e));}) +#define free_swap_and_cache (e) is_special_entry(e) +#define swapcache_prepare (e) is_special_entry(e)static inline int add_swap_count_continuation (swp_entry_t swp,gfp_t gfp_mask) {
include/linux/swapops.h +5 -46
81 lines not on this page · raw message
mm/hmm.c +2 -3
@@ -214,7 +214,7 @@ static inline bool hmm_is_device_private_entry(struct hmm_range *range, swp_entry_t entry) {return is_device_private_entry (entry) && -device_private_entry_to_page (entry)->pgmap->owner == +pfn_to_page (swp_offset (entry))->pgmap->owner == range->dev_private_owner; }
@@ -257,8 +257,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, cpu_flags =HMM_PFN_VALID ;if (is_write_device_private_entry (entry)) cpu_flags |=HMM_PFN_WRITE ; - *hmm_pfn =device_private_entry_to_pfn (entry) | - cpu_flags; + *hmm_pfn =swp_offset (entry) | cpu_flags;return 0 ; }
mm/memcontrol.c +1 -1
@@ -5544,7 +5544,7 @@ static struct page *mc_handle_swap_pte(struct vm_area_struct *vma,* as special swap entry in the CPU page table. */ if (is_device_private_entry (ent)) { - page =device_private_entry_to_page (ent); + page =pfn_to_page (swp_offset (ent));/* * MEMORY_DEVICE_PRIVATE means ZONE_DEVICE page and which have * a refcount of 1 when free (unlike normal page)
mm/memory.c +5 -5
@@ -718,7 +718,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm, } rss[MM_SWAPENTS ]++; }else if (is_migration_entry (entry)) { - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]++;
@@ -737,7 +737,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,set_pte_at (src_mm, addr, src_pte, pte); } }else if (is_device_private_entry (entry)) { - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Update rss count even for unaddressable pages, as
@@ -1274,7 +1274,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb, entry =pte_to_swp_entry (ptent);if (is_device_private_entry (entry)) { -struct page *page =device_private_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));if (unlikely (details && details->check_mapping)) {/*
@@ -1303,7 +1303,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,else if (is_migration_entry (entry)) {struct page *page; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]--; }if (unlikely (!free_swap_and_cache (entry)))
@@ -3271,7 +3271,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)migration_entry_wait (vma->vm_mm, vmf->pmd, vmf->address); }else if (is_device_private_entry (entry)) { - vmf->page =device_private_entry_to_page (entry); + vmf->page =pfn_to_page (swp_offset (entry)); ret = vmf->page->pgmap->ops->migrate_to_ram (vmf); }else if (is_hwpoison_entry (entry)) { ret =VM_FAULT_HWPOISON ;
mm/migrate.c +3 -3
@@ -321,7 +321,7 @@ void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,if (!is_migration_entry (entry)) goto out; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Once page cache replacement of page migration started, page_count
@@ -361,7 +361,7 @@ void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd) ptl =pmd_lock (mm, pmd);if (!is_pmd_migration_entry (*pmd)) goto unlock; - page =migration_entry_to_page (pmd_to_swp_entry (*pmd)); + page =pfn_to_page (swp_offset (pmd_to_swp_entry (*pmd)));if (!get_page_unless_zero (page)) goto unlock;spin_unlock (ptl);
@@ -2437,7 +2437,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,if (!is_device_private_entry (entry)) goto next; - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));if (!(migrate->flags &MIGRATE_VMA_SELECT_DEVICE_PRIVATE ) || page->pgmap->owner != migrate->pgmap_owner)
mm/page_vma_mapped.c +3 -3
@@ -96,7 +96,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)if (!is_migration_entry (entry))return false; - pfn =migration_entry_to_pfn (entry); + pfn =swp_offset (entry); }else if (is_swap_pte (*pvmw->pte)) {swp_entry_t entry;
@@ -105,7 +105,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)if (!is_device_private_entry (entry))return false; - pfn =device_private_entry_to_pfn (entry); + pfn =swp_offset (entry); }else {if (!pte_present (*pvmw->pte))return false;
@@ -200,7 +200,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)if (is_migration_entry (pmd_to_swp_entry (*pvmw->pmd))) {swp_entry_t entry =pmd_to_swp_entry (*pvmw->pmd); -if (migration_entry_to_page (entry) != page) +if (pfn_to_page (swp_offset (entry)) != page)return not_found (pvmw);return true; }
Signature · 6 lines
2.20.1 _______________________________________________ Nouveau mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH v3 1/8] mm: Remove special swap entry functions
arch/s390/mm/pgtable.c static void ptep_zap_swap_entry(struct mm_struct *mm, swp_entry_t entry) · line 694
Quoted 2 lines
arch/s390/mm/pgtable.c +1
-1-struct page *page =migration_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));
I wonder if keeping a single special_entry_to_page() helper would still me a useful. But I'm not entirely sure. There are also two more open coded copies of this in the THP migration code.
include/linux/swap.h static inline void show_swap_cache_info(void) · line 523
Quoted 4 lines
include/linux/swap.h +2
-2-#define free_swap_and_cache (e) ({(is_migration_entry(e) || is_device_private_entry(e));}) -#define swapcache_prepare (e) ({(is_migration_entry(e) || is_device_private_entry(e));}) +#define free_swap_and_cache (e) is_special_entry(e) +#define swapcache_prepare (e) is_special_entry(e)
Staring at this I'm really, really confused at what this is doing.
Looking a little closer these are the !CONFIG_SWAP stubs, but it could probably use a comment or two.
Quoted 107 lines
mm/memory.c +5
-5}else if (is_migration_entry (entry)) { - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]++;@@ -737,7 +737,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,set_pte_at (src_mm, addr, src_pte, pte); } }else if (is_device_private_entry (entry)) { - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Update rss count even for unaddressable pages, as @@ -1274,7 +1274,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb, entry =pte_to_swp_entry (ptent);if (is_device_private_entry (entry)) { -struct page *page =device_private_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));if (unlikely (details && details->check_mapping)) {/* @@ -1303,7 +1303,7 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,else if (is_migration_entry (entry)) {struct page *page; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]--; }if (unlikely (!free_swap_and_cache (entry)))@@ -3271,7 +3271,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)migration_entry_wait (vma->vm_mm, vmf->pmd, vmf->address); }else if (is_device_private_entry (entry)) { - vmf->page =device_private_entry_to_page (entry); + vmf->page =pfn_to_page (swp_offset (entry)); ret = vmf->page->pgmap->ops->migrate_to_ram (vmf); }else if (is_hwpoison_entry (entry)) { ret =VM_FAULT_HWPOISON ;mm/migrate.c +3
-3@@ -321,7 +321,7 @@ void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,if (!is_migration_entry (entry)) goto out; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Once page cache replacement of page migration started, page_count @@ -361,7 +361,7 @@ void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd) ptl =pmd_lock (mm, pmd);if (!is_pmd_migration_entry (*pmd)) goto unlock; - page =migration_entry_to_page (pmd_to_swp_entry (*pmd)); + page =pfn_to_page (swp_offset (pmd_to_swp_entry (*pmd)));if (!get_page_unless_zero (page)) goto unlock;spin_unlock (ptl);@@ -2437,7 +2437,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,if (!is_device_private_entry (entry)) goto next; - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));if (!(migrate->flags &MIGRATE_VMA_SELECT_DEVICE_PRIVATE ) || page->pgmap->owner != migrate->pgmap_owner)mm/page_vma_mapped.c +3
-3@@ -96,7 +96,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)if (!is_migration_entry (entry))return false; - pfn =migration_entry_to_pfn (entry); + pfn =swp_offset (entry); }else if (is_swap_pte (*pvmw->pte)) {swp_entry_t entry;@@ -105,7 +105,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)if (!is_device_private_entry (entry))return false; - pfn =device_private_entry_to_pfn (entry); + pfn =swp_offset (entry); }else {if (!pte_present (*pvmw->pte))return false;@@ -200,7 +200,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)if (is_migration_entry (pmd_to_swp_entry (*pvmw->pmd))) {swp_entry_t entry =pmd_to_swp_entry (*pvmw->pmd); -if (migration_entry_to_page (entry) != page) +if (pfn_to_page (swp_offset (entry)) != page)return not_found (pvmw);return true; }Signature · 1 line
2.20.1
---end quoted text---
Alistair Popple
reviewerRe: [PATCH v3 1/8] mm: Remove special swap entry functions
Quoted 6 lines · AEDT Christoph Hellwig
arch/s390/mm/pgtable.c static void ptep_zap_swap_entry(struct mm_struct *mm, swp_entry_t entry) · line 694
Quoted 2 lines
arch/s390/mm/pgtable.c +1
-1-struct page *page =migration_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));I wonder if keeping a single
special_entry_to_page()helper would still me a useful. But I'm not entirely sure. There are also two more open coded copies of this in the THP migration code.
I think it might be if only to clearly document where these entries are used. Will add it for the next version to see what it looks like.
Quoted 1 line
Quoted 1 line
-#define
free_swap_and_cache(e)({(is_migration_entry(e)||
is_device_private_entry(e));})
Quoted 1 line
Quoted 1 line
-#define
swapcache_prepare(e)({(is_migration_entry(e)||
is_device_private_entry(e));})
Quoted 7 lines
include/linux/swap.h static inline void show_swap_cache_info(void) · line 523
Quoted 2 lines
include/linux/swap.h +2
-0+#define free_swap_and_cache (e) is_special_entry(e) +#define swapcache_prepare (e) is_special_entry(e)Staring at this I'm really, really confused at what this is doing.
Looking a little closer these are the !CONFIG_SWAP stubs, but it could probably use a comment or two.
Will do, thanks.
- Alistair
Quoted 7 lines
mm/memory.c copy_nonpresent_pte(struct mm_struct *dst_mm, struct
Quoted 7 lines
mm/memory.c +1
-1}else if (is_migration_entry (entry)) { - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]++;@@ -737,7 +737,7 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct
mm_struct *src_mm,
Quoted 9 lines
mm/memory.c static unsigned long zap_pte_range(struct mmu_gather
Quoted 9 lines
mm/memory.c +1
-1set_pte_at (src_mm, addr, src_pte, pte); } }else if (is_device_private_entry (entry)) { - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Update rss count even for unaddressable pages, as @@ -1274,7 +1274,7 @@ static unsigned long zap_pte_range(struct mmu_gather
*tlb,
Quoted 9 lines
mm/memory.c static unsigned long zap_pte_range(struct mmu_gather
Quoted 9 lines
mm/memory.c +1
-1entry =pte_to_swp_entry (ptent);if (is_device_private_entry (entry)) { -struct page *page =device_private_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));if (unlikely (details && details->check_mapping)) {/* @@ -1303,7 +1303,7 @@ static unsigned long zap_pte_range(struct mmu_gather
*tlb,
Quoted 14 lines
Quoted 14 lines
mm/memory.c +2
-2else if (is_migration_entry (entry)) {struct page *page; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry)); rss[mm_counter (page)]--; }if (unlikely (!free_swap_and_cache (entry)))@@ -3271,7 +3271,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)migration_entry_wait (vma->vm_mm, vmf->pmd, vmf->address); }else if (is_device_private_entry (entry)) { - vmf->page =device_private_entry_to_page (entry); + vmf->page =pfn_to_page (swp_offset (entry));
Quoted 8 lines
mm/migrate.c void __migration_entry_wait(struct mm_struct *mm,
Quoted 8 lines
mm/memory.c +0
-0ret = vmf->page->pgmap->ops->migrate_to_ram (vmf); }else if (is_hwpoison_entry (entry)) { ret =VM_FAULT_HWPOISON ;mm/migrate.c +0
-0@@ -321,7 +321,7 @@ void __migration_entry_wait(struct mm_struct *mm,
pte_t *ptep,
Quoted 9 lines
mm/migrate.c void pmd_migration_entry_wait(struct mm_struct *mm,
Quoted 9 lines
mm/migrate.c +1
-1if (!is_migration_entry (entry)) goto out; - page =migration_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));/* * Once page cache replacement of page migration started, page_count @@ -361,7 +361,7 @@ void pmd_migration_entry_wait(struct mm_struct *mm,
pmd_t *pmd)
Quoted 25 lines
Quoted 25 lines
mm/migrate.c +2
-2ptl =pmd_lock (mm, pmd);if (!is_pmd_migration_entry (*pmd)) goto unlock; - page =migration_entry_to_page (pmd_to_swp_entry (*pmd)); + page =pfn_to_page (swp_offset (pmd_to_swp_entry (*pmd)));if (!get_page_unless_zero (page)) goto unlock;spin_unlock (ptl);@@ -2437,7 +2437,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,if (!is_device_private_entry (entry)) goto next; - page =device_private_entry_to_page (entry); + page =pfn_to_page (swp_offset (entry));if (!(migrate->flags &MIGRATE_VMA_SELECT_DEVICE_PRIVATE ) || page->pgmap->owner != migrate->pgmap_owner)mm/page_vma_mapped.c +0
-0@@ -96,7 +96,7 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)if (!is_migration_entry (entry))return false;
Quoted 6 lines
mm/page_vma_mapped.c static bool check_pte(struct page_vma_mapped_walk
Quoted 6 lines
mm/page_vma_mapped.c +1
-1- pfn =migration_entry_to_pfn (entry); + pfn =swp_offset (entry); }else if (is_swap_pte (*pvmw->pte)) {swp_entry_t entry;@@ -105,7 +105,7 @@ static bool check_pte(struct page_vma_mapped_walk
*pvmw)
Quoted 9 lines
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk
Quoted 9 lines
mm/page_vma_mapped.c +1
-1if (!is_device_private_entry (entry))return false; - pfn =device_private_entry_to_pfn (entry); + pfn =swp_offset (entry); }else {if (!pte_present (*pvmw->pte))return false;@@ -200,7 +200,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk
*pvmw)
Quoted 10 lines
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 206
Quoted 8 lines
mm/page_vma_mapped.c +1
-1if (is_migration_entry (pmd_to_swp_entry (*pvmw->pmd))) {swp_entry_t entry =pmd_to_swp_entry (*pvmw->pmd); -if (migration_entry_to_page (entry) != page) +if (pfn_to_page (swp_offset (entry)) != page)return not_found (pvmw);return true; }---end quoted text---
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerRe: [PATCH v3 1/8] mm: Remove special swap entry functions
Quoted 9 lines · AEDT Alistair Popple
Quoted 6 lines · AEDT Christoph Hellwig
arch/s390/mm/pgtable.c static void ptep_zap_swap_entry(struct mm_struct *mm, swp_entry_t entry) · line 694
Quoted 2 lines
arch/s390/mm/pgtable.c +1
-1-struct page *page =migration_entry_to_page (entry); +struct page *page =pfn_to_page (swp_offset (entry));I wonder if keeping a single
special_entry_to_page()helper would still me a useful. But I'm not entirely sure. There are also two more open coded copies of this in the THP migration code.I think it might be if only to clearly document where these entries are
used.
Quoted 1 line
Will add it for the next version to see what it looks like.
Actually the main advantage ends up being that it becomes easy to retain the page locked check for migration entries.
- Alistair
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Jason Gunthorpe
maintainerRe: [PATCH v3 1/8] mm: Remove special swap entry functions
In reply to Alistair PoppleQuoted 5 lines · Alistair Popple
Remove the migration and device private
entry_to_page()andentry_to_pfn()inline functions and instead open code them directly. This results in shorter code which is easier to understand.Signed-off-by Alistair Popple <[email protected]>
Quoted 11 lines
--- arch/s390/mm/pgtable.c | 2 +- fs/proc/task_mmu.c | 23 +++++++------------ include/linux/swap.h | 4 ++-- include/linux/swapops.h | 51 ++++------------------------------------- mm/hmm.c | 5 ++-- mm/memcontrol.c | 2 +- mm/memory.c | 10 ++++---- mm/migrate.c | 6 ++--- mm/page_vma_mapped.c | 6 ++--- 9 files changed, 30 insertions(+), 79 deletions(-)
I wish you could come up with a more descriptive word that special here
What I understand is this is true when the swap_offset is a pfn?
include/linux/swapops.h static inline int is_write_migration_entry(swp_entry_t entry) · line 198
Quoted 9 lines
include/linux/swapops.h +0
-9-static inline struct page *migration_entry_to_page (swp_entry_t entry) -{ -struct page *p =pfn_to_page (swp_offset (entry)); -/* -* Any use of migration entries may only occur while the -* corresponding page is locked -*/ -BUG_ON (!PageLocked (compound_head (p))); -return p;
And this constraint has been completely lost?
A comment in front of the is_special_entry explaining all the rule would help alot
Transformation looks fine otherwise
Jason
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerRe: [PATCH v3 1/8] mm: Remove special swap entry functions
Quoted 5 lines · AEDT Jason Gunthorpe
I wish you could come up with a more descriptive word that special here
What I understand is this is true when the
swap_offsetis a pfn?
Correct, and that points to a better name. Maybe is_pfn_swap_entry()? In which case adding a helper as Christoph suggested makes some more sense. Eg:pfn_swap_entry_to_page()
Quoted 11 lines
include/linux/swapops.h static inline int is_write_migration_entry(swp_entry_t entry) · line 198
Quoted 9 lines
include/linux/swapops.h +0
-9-static inline struct page *migration_entry_to_page (swp_entry_t entry) -{ -struct page *p =pfn_to_page (swp_offset (entry)); -/* -* Any use of migration entries may only occur while the -* corresponding page is locked -*/ -BUG_ON (!PageLocked (compound_head (p))); -return p;And this constraint has been completely lost?
Yes, sorry I should have called that out. I didn't think loosing the check was a big deal, but I can add some checks to some of the call sites which would catch a page being incorrectly unlocked.
Quoted 2 lines
A comment in front of the
is_special_entryexplaining all the rule would help alot
Will add one.
Quoted 1 line
Transformation looks fine otherwise
Thanks.
- Alistair
Quoted 2 lines
Jason
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerPATCH v3 2/8 mm/swapops: Rework swap entry manipulation code
In reply to Alistair PoppleBoth migration and device private pages use special swap entries which are manipluated by a range of inline functions. The arguments to these are somewhat inconsitent so rework them to remove flag type arguments and to make the arguments similar for both a read and write entry creation.
--- include/linux/swapops.h | 56 ++++++++++++++++++++++------------------- mm/debug_vm_pgtable.c | 12 ++++----- mm/hmm.c | 2 +- mm/huge_memory.c | 26 +++++++++++++------ mm/hugetlb.c | 10 +++++--- mm/memory.c | 10 +++++--- mm/migrate.c | 26 ++++++++++++++----- mm/mprotect.c | 10 +++++--- mm/rmap.c | 10 +++++--- 9 files changed, 100 insertions(+), 62 deletions(-)
include/linux/swapops.h +30 -26
@@ -100,35 +100,35 @@ static inline void *swp_to_radix_entry(swp_entry_t entry) }#if IS_ENABLED (CONFIG_DEVICE_PRIVATE ) -static inline swp_entry_t make_device_private_entry (struct page *page,bool write) +static inline swp_entry_t make_readable_device_private_entry (pgoff_t offset) { -return swp_entry (write ?SWP_DEVICE_WRITE :SWP_DEVICE_READ , -page_to_pfn (page)); +return swp_entry (SWP_DEVICE_READ , offset); } -static inline bool is_device_private_entry (swp_entry_t entry) +static inline swp_entry_t make_writable_device_private_entry (pgoff_t offset) { -int type =swp_type (entry); -return type ==SWP_DEVICE_READ || type ==SWP_DEVICE_WRITE ; +return swp_entry (SWP_DEVICE_WRITE , offset); } -static inline void make_device_private_entry_read (swp_entry_t *entry) +static inline bool is_device_private_entry (swp_entry_t entry) { - *entry =swp_entry (SWP_DEVICE_READ ,swp_offset (*entry)); +int type =swp_type (entry); +return type ==SWP_DEVICE_READ || type ==SWP_DEVICE_WRITE ; } -static inline bool is_write_device_private_entry (swp_entry_t entry) +static inline bool is_writable_device_private_entry (swp_entry_t entry) {return unlikely (swp_type (entry) ==SWP_DEVICE_WRITE ); }#else /* CONFIG_DEVICE_PRIVATE */ -static inline swp_entry_t make_device_private_entry (struct page *page,bool write) +static inline swp_entry_t make_readable_device_private_entry (pgoff_t offset) {return swp_entry (0 ,0 ); } -static inline void make_device_private_entry_read (swp_entry_t *entry) +static inline swp_entry_t make_writable_device_private_entry (pgoff_t offset) { +return swp_entry (0 ,0 ); }static inline bool is_device_private_entry (swp_entry_t entry)
@@ -136,35 +136,32 @@ static inline bool is_device_private_entry(swp_entry_t entry)return false; } -static inline bool is_write_device_private_entry (swp_entry_t entry) +static inline bool is_writable_device_private_entry (swp_entry_t entry) {return false; }#endif /* CONFIG_DEVICE_PRIVATE */ #ifdef CONFIG_MIGRATION -static inline swp_entry_t make_migration_entry (struct page *page,int write) -{ -BUG_ON (!PageLocked (compound_head (page))); - -return swp_entry (write ?SWP_MIGRATION_WRITE :SWP_MIGRATION_READ , -page_to_pfn (page)); -} -static inline int is_migration_entry (swp_entry_t entry) {return unlikely (swp_type (entry) ==SWP_MIGRATION_READ ||swp_type (entry) ==SWP_MIGRATION_WRITE ); } -static inline int is_write_migration_entry (swp_entry_t entry) +static inline int is_writable_migration_entry (swp_entry_t entry) {return unlikely (swp_type (entry) ==SWP_MIGRATION_WRITE ); } -static inline void make_migration_entry_read (swp_entry_t *entry) +static inline swp_entry_t make_readable_migration_entry (pgoff_t offset) { - *entry =swp_entry (SWP_MIGRATION_READ ,swp_offset (*entry)); +return swp_entry (SWP_MIGRATION_READ , offset); +} + +static inline swp_entry_t make_writable_migration_entry (pgoff_t offset) +{ +return swp_entry (SWP_MIGRATION_WRITE , offset); } extern void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,
@@ -174,21 +171,28 @@ extern void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,extern void migration_entry_wait_huge (struct vm_area_struct *vma,struct mm_struct *mm,pte_t *pte);#else +static inline swp_entry_t make_readable_migration_entry (pgoff_t offset) +{ +return swp_entry (0 ,0 ); +} + +static inline swp_entry_t make_writable_migration_entry (pgoff_t offset) +{ +return swp_entry (0 ,0 ); +} -#define make_migration_entry (page, write) swp_entry(0, 0)static inline int is_migration_entry (swp_entry_t swp) {return 0 ; } -static inline void make_migration_entry_read (swp_entry_t *entryp) { }static inline void __migration_entry_wait (struct mm_struct *mm,pte_t *ptep,spinlock_t *ptl) { }static inline void migration_entry_wait (struct mm_struct *mm,pmd_t *pmd,unsigned long address) { }static inline void migration_entry_wait_huge (struct vm_area_struct *vma,struct mm_struct *mm,pte_t *pte) { } -static inline int is_write_migration_entry (swp_entry_t entry) +static inline int is_writable_migration_entry (swp_entry_t entry) {return 0 ; }
mm/debug_vm_pgtable.c +6 -6
@@ -772,17 +772,17 @@ static void __init swap_migration_tests(void)* locked, otherwise it stumbles upon a BUG_ON(). */ __SetPageLocked (page); - swp =make_migration_entry (page,1 ); + swp =make_writable_migration_entry (page_to_pfn (page));WARN_ON (!is_migration_entry (swp)); -WARN_ON (!is_write_migration_entry (swp)); +WARN_ON (!is_writable_migration_entry (swp)); -make_migration_entry_read (&swp); + swp =make_readable_migration_entry (swp_offset (swp));WARN_ON (!is_migration_entry (swp)); -WARN_ON (is_write_migration_entry (swp)); +WARN_ON (is_writable_migration_entry (swp)); - swp =make_migration_entry (page,0 ); + swp =make_readable_migration_entry (page_to_pfn (page));WARN_ON (!is_migration_entry (swp)); -WARN_ON (is_write_migration_entry (swp)); +WARN_ON (is_writable_migration_entry (swp));__ClearPageLocked (page);__free_page (page); }
mm/hmm.c +1 -1
@@ -255,7 +255,7 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,*/ if (hmm_is_device_private_entry (range, entry)) { cpu_flags =HMM_PFN_VALID ; -if (is_write_device_private_entry (entry)) +if (is_writable_device_private_entry (entry)) cpu_flags |=HMM_PFN_WRITE ; *hmm_pfn =swp_offset (entry) | cpu_flags;return 0 ;
mm/huge_memory.c +18 -8
@@ -1046,8 +1046,9 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,swp_entry_t entry =pmd_to_swp_entry (pmd);VM_BUG_ON (!is_pmd_migration_entry (pmd)); -if (is_write_migration_entry (entry)) { -make_migration_entry_read (&entry); +if (is_writable_migration_entry (entry)) { + entry =make_readable_migration_entry ( +swp_offset (entry)); pmd =swp_entry_to_pmd (entry);if (pmd_swp_soft_dirty (*src_pmd)) pmd =pmd_swp_mksoft_dirty (pmd);
@@ -1820,13 +1821,14 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,swp_entry_t entry =pmd_to_swp_entry (*pmd);VM_BUG_ON (!is_pmd_migration_entry (*pmd)); -if (is_write_migration_entry (entry)) { +if (is_writable_migration_entry (entry)) {pmd_t newpmd;/* * A protection check is difficult so * just be safe and disable write */ -make_migration_entry_read (&entry); + entry =make_readable_migration_entry ( +swp_offset (entry)); newpmd =swp_entry_to_pmd (entry);if (pmd_swp_soft_dirty (*pmd)) newpmd =pmd_swp_mksoft_dirty (newpmd);
@@ -2104,7 +2106,7 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd, entry =pmd_to_swp_entry (old_pmd); page =pfn_to_page (swp_offset (entry)); - write =is_write_migration_entry (entry); + write =is_writable_migration_entry (entry); young = false; soft_dirty =pmd_swp_soft_dirty (old_pmd); uffd_wp =pmd_swp_uffd_wp (old_pmd);
@@ -2136,7 +2138,12 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,*/ if (freeze || pmd_migration) {swp_entry_t swp_entry; - swp_entry =make_migration_entry (page + i, write); +if (write) + swp_entry =make_writable_migration_entry ( +page_to_pfn (page + i)); +else + swp_entry =make_readable_migration_entry ( +page_to_pfn (page + i)); entry =swp_entry_to_pte (swp_entry);if (soft_dirty) entry =pte_swp_mksoft_dirty (entry);
@@ -2988,7 +2995,10 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw, pmdval =pmdp_invalidate (vma, address, pvmw->pmd);if (pmd_dirty (pmdval))set_page_dirty (page); - entry =make_migration_entry (page,pmd_write (pmdval)); +if (pmd_write (pmdval)) + entry =make_writable_migration_entry (page_to_pfn (page)); +else + entry =make_readable_migration_entry (page_to_pfn (page)); pmdswp =swp_entry_to_pmd (entry);if (pmd_soft_dirty (pmdval)) pmdswp =pmd_swp_mksoft_dirty (pmdswp);
@@ -3014,7 +3024,7 @@ void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new) pmde =pmd_mkold (mk_huge_pmd (new, vma->vm_page_prot));if (pmd_swp_soft_dirty (*pvmw->pmd)) pmde =pmd_mksoft_dirty (pmde); -if (is_write_migration_entry (entry)) +if (is_writable_migration_entry (entry)) pmde =maybe_pmd_mkwrite (pmde, vma);flush_cache_range (vma, mmun_start, mmun_start +HPAGE_PMD_SIZE );
mm/hugetlb.c +6 -4
@@ -3842,12 +3842,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, is_hugetlb_entry_hwpoisoned(entry))) {swp_entry_t swp_entry =pte_to_swp_entry (entry); -if (is_write_migration_entry (swp_entry) && cow) { +if (is_writable_migration_entry (swp_entry) && cow) {/* * COW mappings require pages in both * parent and child to be set to read. */ -make_migration_entry_read (&swp_entry); + swp_entry =make_readable_migration_entry ( +swp_offset (swp_entry)); entry =swp_entry_to_pte (swp_entry);set_huge_swap_pte_at (src, addr, src_pte, entry, sz);
@@ -5019,10 +5020,11 @@ unsigned long hugetlb_change_protection(struct vm_area_struct *vma,if (unlikely (is_hugetlb_entry_migration (pte))) {swp_entry_t entry =pte_to_swp_entry (pte); -if (is_write_migration_entry (entry)) { +if (is_writable_migration_entry (entry)) {pte_t newpte; -make_migration_entry_read (&entry); + entry =make_readable_migration_entry ( +swp_offset (entry)); newpte =swp_entry_to_pte (entry);set_huge_swap_pte_at (mm, address, ptep, newpte,huge_page_size (h));
mm/memory.c +6 -4
@@ -722,13 +722,14 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm, rss[mm_counter (page)]++; -if (is_write_migration_entry (entry) && +if (is_writable_migration_entry (entry) &&is_cow_mapping (vm_flags)) {/* * COW mappings require pages in both * parent and child to be set to read. */ -make_migration_entry_read (&entry); + entry =make_readable_migration_entry ( +swp_offset (entry)); pte =swp_entry_to_pte (entry);if (pte_swp_soft_dirty (*src_pte)) pte =pte_swp_mksoft_dirty (pte);
@@ -759,9 +760,10 @@ copy_nonpresent_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,* when a device driver is involved (you cannot easily * save and restore device driver state). */ -if (is_write_device_private_entry (entry) && +if (is_writable_device_private_entry (entry) &&is_cow_mapping (vm_flags)) { -make_device_private_entry_read (&entry); + entry =make_readable_device_private_entry ( +swp_offset (entry)); pte =swp_entry_to_pte (entry);if (pte_swp_uffd_wp (*src_pte)) pte =pte_swp_mkuffd_wp (pte);
mm/migrate.c +20 -6
@@ -237,13 +237,18 @@ static bool remove_migration_pte(struct page *page, struct vm_area_struct *vma,* Recheck VMA as permissions can change since migration started */ entry =pte_to_swp_entry (*pvmw.pte); -if (is_write_migration_entry (entry)) +if (is_writable_migration_entry (entry)) pte =maybe_mkwrite (pte, vma);else if (pte_swp_uffd_wp (*pvmw.pte)) pte =pte_mkuffd_wp (pte);if (unlikely (is_device_private_page (new))) { - entry =make_device_private_entry (new,pte_write (pte)); +if (pte_write (pte)) + entry =make_writable_device_private_entry ( +page_to_pfn (new)); +else + entry =make_readable_device_private_entry ( +page_to_pfn (new)); pte =swp_entry_to_pte (entry);if (pte_swp_soft_dirty (*pvmw.pte)) pte =pte_swp_mksoft_dirty (pte);
@@ -2445,7 +2450,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, mpfn =migrate_pfn (page_to_pfn (page)) |MIGRATE_PFN_MIGRATE ; -if (is_write_device_private_entry (entry)) +if (is_writable_device_private_entry (entry)) mpfn |=MIGRATE_PFN_WRITE ; }else {if (!(migrate->flags &MIGRATE_VMA_SELECT_SYSTEM ))
@@ -2491,8 +2496,12 @@ 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, mpfn & -MIGRATE_PFN_WRITE ); +if (mpfn &MIGRATE_PFN_WRITE ) + entry =make_writable_migration_entry ( +page_to_pfn (page)); +else + entry =make_readable_migration_entry ( +page_to_pfn (page)); swp_pte =swp_entry_to_pte (entry);if (pte_present (pte)) {if (pte_soft_dirty (pte))
@@ -2965,7 +2974,12 @@ static void migrate_vma_insert_page(struct migrate_vma *migrate,if (is_device_private_page (page)) {swp_entry_t swp_entry; - swp_entry =make_device_private_entry (page, vma->vm_flags &VM_WRITE ); +if (vma->vm_flags &VM_WRITE ) + swp_entry =make_writable_device_private_entry ( +page_to_pfn (page)); +else + swp_entry =make_readable_device_private_entry ( +page_to_pfn (page)); entry =swp_entry_to_pte (swp_entry); } }else {
mm/mprotect.c +6 -4
@@ -143,23 +143,25 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,swp_entry_t entry =pte_to_swp_entry (oldpte);pte_t newpte; -if (is_write_migration_entry (entry)) { +if (is_writable_migration_entry (entry)) {/* * A protection check is difficult so * just be safe and disable write */ -make_migration_entry_read (&entry); + entry =make_readable_migration_entry ( +swp_offset (entry)); newpte =swp_entry_to_pte (entry);if (pte_swp_soft_dirty (oldpte)) newpte =pte_swp_mksoft_dirty (newpte);if (pte_swp_uffd_wp (oldpte)) newpte =pte_swp_mkuffd_wp (newpte); - }else if (is_write_device_private_entry (entry)) { + }else if (is_writable_device_private_entry (entry)) {/* * We do not preserve soft-dirtiness. See * copy_one_pte() for explanation. */ -make_device_private_entry_read (&entry); + entry =make_readable_device_private_entry ( +swp_offset (entry)); newpte =swp_entry_to_pte (entry);if (pte_swp_uffd_wp (oldpte)) newpte =pte_swp_mkuffd_wp (newpte);
mm/rmap.c +7 -3
@@ -1512,7 +1512,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,* pte. do_swap_page() will wait until the migration * pte is removed and then restart fault handling. */ - entry =make_migration_entry (page,0 ); + entry =make_readable_migration_entry (page_to_pfn (page)); swp_pte =swp_entry_to_pte (entry);/*
@@ -1608,8 +1608,12 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,* pte. do_swap_page() will wait until the migration * pte is removed and then restart fault handling. */ - entry =make_migration_entry (subpage, -pte_write (pteval)); +if (pte_write (pteval)) + entry =make_writable_migration_entry ( +page_to_pfn (subpage)); +else + entry =make_readable_migration_entry ( +page_to_pfn (subpage)); swp_pte =swp_entry_to_pte (entry);if (pte_soft_dirty (pteval)) swp_pte =pte_swp_mksoft_dirty (swp_pte);
Signature · 1 line
2.20.1
Re: [PATCH v3 2/8] mm/swapops: Rework swap entry manipulation code
Quoted 7 lines · Alistair Popple
Both migration and device private pages use special swap entries which are manipluated by a range of inline functions. The arguments to these are somewhat inconsitent so rework them to remove flag type arguments and to make the arguments similar for both a read and write entry creation.
Signed-off-by Alistair Popple <[email protected]>
Looks good,
Re: [PATCH v3 2/8] mm/swapops: Rework swap entry manipulation code
In reply to Alistair PoppleQuoted 7 lines · Alistair Popple
Both migration and device private pages use special swap entries which are manipluated by a range of inline functions. The arguments to these are somewhat inconsitent so rework them to remove flag type arguments and to make the arguments similar for both a read and write entry creation.
Signed-off-by Alistair Popple <[email protected]>
Quoted 11 lines
--- include/linux/swapops.h | 56 ++++++++++++++++++++++------------------- mm/debug_vm_pgtable.c | 12 ++++----- mm/hmm.c | 2 +- mm/huge_memory.c | 26 +++++++++++++------ mm/hugetlb.c | 10 +++++--- mm/memory.c | 10 +++++--- mm/migrate.c | 26 ++++++++++++++----- mm/mprotect.c | 10 +++++--- mm/rmap.c | 10 +++++--- 9 files changed, 100 insertions(+), 62 deletions(-)
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerPATCH v3 3/8 mm/rmap: Split try_to_munlock from try_to_unmap
In reply to Alistair PoppleThe behaviour of try_to_unmap_one() is difficult to follow because it performs different operations based on a fairly large set of flags used in different combinations.
TTU_MUNLOCK is one such flag. However it is exclusively used by try_to_munlock() which specifies no other flags. Therefore rather than overload try_to_unmap_one() with unrelated behaviour split this out into it's own function and remove the flag.
---
Given the comments on not needing to hold mmap_lock it was not 100% clear to me if it is safe to check vma->vma_flags & VM_LOCKED and if re-checking under the ptl was significant. I left the extra check in case it was, but it seems one of the checks is redunant as either the first check is racey or the second check is unneccsary.
--- include/linux/rmap.h | 1 - mm/rmap.c | 47 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 9 deletions(-)
include/linux/rmap.h +0 -1
@@ -87,7 +87,6 @@ struct anon_vma_chain {enum ttu_flags {TTU_MIGRATION =0x1 ,/* migration mode */ -TTU_MUNLOCK =0x2 ,/* munlock mode */ TTU_SPLIT_HUGE_PMD =0x4 ,/* split huge PMD if any */ TTU_IGNORE_MLOCK =0x8 ,/* ignore mlock */
mm/rmap.c +39 -8
71 lines not on this page · raw message
Signature · 1 line
2.20.1
Re: [PATCH v3 3/8] mm/rmap: Split try_to_munlock from try_to_unmap
Quoted 8 lines
mm/rmap.c +8
-0+while (page_vma_mapped_walk (&pvmw)) { +/* +* If the page is mlock()d, we cannot swap it out. +* If it's recently referenced (perhaps page_referenced +* skipped over this mm) then we should reactivate it. +*/ +if (vma->vm_flags &VM_LOCKED ) { +/* PTE-mapped THP are never mlocked */
mm/rmap.c static int page_not_mapped(struct page *page) · line 1803
Quoted 10 lines
mm/rmap.c +10
-0+if (!PageTransCompound (page)) { +/* +* Holding pte lock, we do *not* need +* mmap_lock here +*/ +mlock_vma_page (page); + } + ret = false; +page_vma_mapped_walk_done (&pvmw); +break ;
Just return false here directly and remove the ret variable?
Very nice cleanup!
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Jason Gunthorpe
maintainerRe: [PATCH v3 3/8] mm/rmap: Split try_to_munlock from try_to_unmap
In reply to Alistair PoppleQuoted 9 lines · Alistair Popple
The behaviour of
try_to_unmap_one()is difficult to follow because it performs different operations based on a fairly large set of flags used in different combinations.
TTU_MUNLOCKis one such flag. However it is exclusively used bytry_to_munlock()which specifies no other flags. Therefore rather than overloadtry_to_unmap_one()with unrelated behaviour split this out into it's own function and remove the flag.
Quoted 8 lines
Signed-off-by Alistair Popple <[email protected]>Given the comments on not needing to hold
mmap_lockit was not 100% clear to me if it is safe to checkvma->vma_flags&VM_LOCKEDand if re-checking under the ptl was significant. I left the extra check in case it was, but it seems one of the checks is redunant as either the first check is racey or the second check is unneccsary.
The rmap doesn't hold the mmap_lock so I think both of these cases are racey.
eg
apply_vma_lock_flags()
vma = find_vma(current->mm, start);
if (!vma || vma->vm_start > start)
return -ENOMEM;
prev = vma->vm_prev;
if (start > vma->vm_start)
prev = vma;
for (nstart = start ; ; ) {
vm_flags_t newflags = vma->vm_flags & VM_LOCKED_CLEAR_MASK;
newflags |= flags;
[...]
mlock_fixup()
/*
* vm_flags is protected by the mmap_lock held in write mode.
* It's okay if try_to_unmap_one unmaps a page just after we
* set VM_LOCKED, populate_vma_page_range will bring it back.
*/
if (lock)
vma->vm_flags = newflags;
else
vma->vm_flags &= VM_LOCKED_CLEAR_MASK;Which is only done under the mmap_sem
mm/rmap.c static int page_not_mapped(struct page *page) · line 1784
Quoted 13 lines
mm/rmap.c +13
-0+static bool try_to_munlock_one (struct page *page,struct vm_area_struct *vma, +unsigned long address,void *arg) +{ +struct page_vma_mapped_walk pvmw = { + .page = page, + .vma = vma, + .address = address, + }; +bool ret = true; + +/* munlock has nothing to gain from examining un-locked vmas */ +if (!(vma->vm_flags &VM_LOCKED )) +return true;
The mmap_sem can't be obtained in the rmap walkers due to lock ordering, the various rmap locks are nested under the mmap_sem
So, when reading data that is not locked it should be written as:
READ_ONCE(vma->vm_flags) & VM_LOCKED
mm/rmap.c static int page_not_mapped(struct page *page) · line 1792
Quoted 7 lines
mm/rmap.c +7
-0+while (page_vma_mapped_walk (&pvmw)) { +/* +* If the page is mlock()d, we cannot swap it out. +* If it's recently referenced (perhaps page_referenced +* skipped over this mm) then we should reactivate it. +*/ +if (vma->vm_flags &VM_LOCKED ) {
And since we write the data without holding the PTLs this looks pointless, unless there is some other VM_LOCKED manipulation
Jason
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerRe: [PATCH v3 3/8] mm/rmap: Split try_to_munlock from try_to_unmap
Quoted 11 lines · AEDT Jason Gunthorpe
mm/rmap.c static int page_not_mapped(struct page *page) · line 1792
Quoted 7 lines
mm/rmap.c +7
-0+while (page_vma_mapped_walk (&pvmw)) { +/* +* If the page is mlock()d, we cannot swap it out. +* If it's recently referenced (perhaps page_referenced +* skipped over this mm) then we should reactivate it. +*/ +if (vma->vm_flags &VM_LOCKED ) {And since we write the data without holding the PTLs this looks pointless, unless there is some other
VM_LOCKEDmanipulation
Thanks. I couldn't find any other manipulation of VM_LOCKED whilst holding the PTL so I'll remove this redundant check.
- Alistair
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerPATCH v3 4/8 mm/rmap: Split migration into its own function
In reply to Alistair PoppleMigration is currently implemented as a mode of operation for try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE.
However it does not have much in common with the rest of the unmap functionality of try_to_unmap_one() and thus splitting it into a separate function reduces the complexity of try_to_unmap_one() making it more readable.
Several simplifications can also be made in try_to_migrate_one() based on the following observations:
- All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK.
- No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON.
- No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH.
TTU_SPLIT_FREEZE is a special case of migration used when splitting an anonymous page. This is most easily dealt with by calling the correct function from unmap_page() in mm/huge_memory.c - either try_to_migrate() for PageAnon or try_to_unmap().
--- include/linux/rmap.h | 4 +- mm/huge_memory.c | 10 +- mm/migrate.c | 9 +- mm/rmap.c | 352 +++++++++++++++++++++++++++++++------------ 4 files changed, 269 insertions(+), 106 deletions(-)
include/linux/rmap.h +1 -3
@@ -86,8 +86,6 @@ struct anon_vma_chain { };enum ttu_flags { -TTU_MIGRATION =0x1 ,/* migration mode */ -TTU_SPLIT_HUGE_PMD =0x4 ,/* split huge PMD if any */ TTU_IGNORE_MLOCK =0x8 ,/* ignore mlock */ TTU_IGNORE_HWPOISON =0x20 ,/* corrupted page is recoverable */
@@ -96,7 +94,6 @@ enum ttu_flags {* do a final flush if necessary */ TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:* caller holds it */ - TTU_SPLIT_FREEZE = 0x100, /* freeze pte under splitting thp */ };#ifdef CONFIG_MMU
@@ -193,6 +190,7 @@ static inline void page_dup_rmap(struct page *page, bool compound)int page_referenced (struct page *,int is_locked,struct mem_cgroup *memcg,unsigned long *vm_flags); +bool try_to_migrate (struct page *page,enum ttu_flags flags);bool try_to_unmap (struct page *,enum ttu_flags flags);/* Avoid racy checks */
mm/huge_memory.c +5 -5
@@ -2351,16 +2351,16 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma,static void unmap_page (struct page *page) { -enum ttu_flags ttu_flags =TTU_IGNORE_MLOCK | -TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ; +enum ttu_flags ttu_flags =TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ;bool unmap_success;VM_BUG_ON_PAGE (!PageHead (page), page);if (PageAnon (page)) - ttu_flags |=TTU_SPLIT_FREEZE ; - - unmap_success =try_to_unmap (page, ttu_flags); + unmap_success =try_to_migrate (page, ttu_flags); +else + unmap_success =try_to_unmap (page, ttu_flags | +TTU_IGNORE_MLOCK );VM_BUG_ON_PAGE (!unmap_success, page); }
mm/migrate.c +4 -5
@@ -1124,7 +1124,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage,/* Establish migration ptes */ VM_BUG_ON_PAGE (PageAnon (page) && !PageKsm (page) && !anon_vma, page); -try_to_unmap (page,TTU_MIGRATION |TTU_IGNORE_MLOCK ); +try_to_migrate (page,0 ); page_was_mapped =1 ; }
@@ -1326,7 +1326,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,if (page_mapped (hpage)) {bool mapping_locked = false; -enum ttu_flags ttu =TTU_MIGRATION |TTU_IGNORE_MLOCK ; +enum ttu_flags ttu =0 ;if (!PageAnon (hpage)) {/*
@@ -1343,7 +1343,7 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, ttu |=TTU_RMAP_LOCKED ; } -try_to_unmap (hpage, ttu); +try_to_migrate (hpage, ttu); page_was_mapped =1 ;if (mapping_locked)
@@ -2750,7 +2750,6 @@ static void migrate_vma_prepare(struct migrate_vma *migrate)*/ static void migrate_vma_unmap (struct migrate_vma *migrate) { -int flags =TTU_MIGRATION |TTU_IGNORE_MLOCK ;const unsigned long npages = migrate->npages;const unsigned long start = migrate->start;unsigned long addr, i, restore =0 ;
@@ -2762,7 +2761,7 @@ static void migrate_vma_unmap(struct migrate_vma *migrate)continue ;if (page_mapped (page)) { -try_to_unmap (page, flags); +try_to_migrate (page,0 );if (page_mapped (page)) goto restore; }
mm/rmap.c +259 -93
388 lines not on this page · raw message
Signature · 1 line
2.20.1
Re: [PATCH v3 4/8] mm/rmap: Split migration into its own function
Nice cleanup!
Zi Yan
reviewerRe: [PATCH v3 4/8] mm/rmap: Split migration into its own function
In reply to Alistair PoppleQuoted 34 lines · Alistair Popple
Migration is currently implemented as a mode of operation for
try_to_unmap_one()generally specified by passing theTTU_MIGRATIONflag or in the case of splitting a huge anonymous pageTTU_SPLIT_FREEZE.However it does not have much in common with the rest of the unmap functionality of
try_to_unmap_one()and thus splitting it into a separate function reduces the complexity oftry_to_unmap_one()making it more readable.Several simplifications can also be made in
try_to_migrate_one()based on the following observations:- All users of
TTU_MIGRATIONalso setTTU_IGNORE_MLOCK.
- No users ofTTU_MIGRATIONever setTTU_IGNORE_HWPOISON.
- No users ofTTU_MIGRATIONever setTTU_BATCH_FLUSH.
TTU_SPLIT_FREEZEis a special case of migration used when splitting an anonymous page. This is most easily dealt with by calling the correct function fromunmap_page()inmm/huge_memory.c- eithertry_to_migrate()for PageAnon ortry_to_unmap().Signed-off-by Alistair Popple <[email protected]>--- include/linux/rmap.h | 4 +- mm/huge_memory.c | 10 +- mm/migrate.c | 9 +- mm/rmap.c | 352 +++++++++++++++++++++++++++++++------------ 4 files changed, 269 insertions(+), 106 deletions(-)include/linux/rmap.h +0
-0
include/linux/rmap.h struct anon_vma_chain { · line 89
Quoted 6 lines
include/linux/rmap.h +0
-2@@ -86,8 +86,6 @@ struct anon_vma_chain { };enum ttu_flags { -TTU_MIGRATION =0x1 ,/* migration mode */ -TTU_SPLIT_HUGE_PMD =0x4 ,/* split huge PMD if any */
It implies freeze in try_to_migrate() and no freeze in try_to_unmap(). I think we need some comments here, above try_to_migrate(), and above try_to_unmap() to clarify the implication.
Quoted 31 lines
include/linux/rmap.h +1
-1TTU_IGNORE_MLOCK =0x8 ,/* ignore mlock */ TTU_IGNORE_HWPOISON = 0x20, /* corrupted page is recoverable */@@ -96,7 +94,6 @@ enum ttu_flags {* do a final flush if necessary */ TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:* caller holds it */ - TTU_SPLIT_FREEZE = 0x100, /* freeze pte under splitting thp */ };#ifdef CONFIG_MMU @@ -193,6 +190,7 @@ static inline void page_dup_rmap(struct page *page, bool compound)int page_referenced (struct page *,int is_locked,struct mem_cgroup *memcg,unsigned long *vm_flags); +bool try_to_migrate (struct page *page,enum ttu_flags flags);bool try_to_unmap (struct page *,enum ttu_flags flags);/* Avoid racy checks */ mm/huge_memory.c +1
-2@@ -2351,16 +2351,16 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma,static void unmap_page (struct page *page) { -enum ttu_flags ttu_flags =TTU_IGNORE_MLOCK | -TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ; +enum ttu_flags ttu_flags =TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ;bool unmap_success;
mm/huge_memory.c void vma_adjust_trans_huge(struct vm_area_struct *vma, · line 2363
Quoted 10 lines
mm/huge_memory.c +4
-3VM_BUG_ON_PAGE (!PageHead (page), page);if (PageAnon (page)) - ttu_flags |=TTU_SPLIT_FREEZE ; - - unmap_success =try_to_unmap (page, ttu_flags); + unmap_success =try_to_migrate (page, ttu_flags); +else + unmap_success =try_to_unmap (page, ttu_flags | +TTU_IGNORE_MLOCK );
I think we need a comment here about why anonymous pages need try_to_migrate() and others need try_to_unmap().
Thanks.
—
Best Regards,
Yan Zi
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerRe: [PATCH v3 4/8] mm/rmap: Split migration into its own function
Quoted 9 lines · AEDT Zi Yan
Quoted 7 lines · Alistair Popple
include/linux/rmap.h +0
-0@@ -86,8 +86,6 @@ struct anon_vma_chain { };
Quoted 6 lines
include/linux/rmap.h struct anon_vma_chain { · line 89
Quoted 4 lines
include/linux/rmap.h +0
-2enum ttu_flags { -TTU_MIGRATION =0x1 ,/* migration mode */ -TTU_SPLIT_HUGE_PMD =0x4 ,/* split huge PMD if any */ It implies freeze in
try_to_migrate()and no freeze intry_to_unmap(). I
think
Quoted 2 lines
we need some comments here, above
try_to_migrate(), and abovetry_to_unmap()to clarify the implication.
Sure. This confused me for a bit and I was initially tempted to leave TTU_SPLIT_FREEZE as a separate mode flag but looking at what freeze actually does it made sense to remove it because try_to_migrate() is for installing migration entries (which is what freeze does) and try_to_unmap() just unmaps. So I'll add some comments to that effect.
Quoted 11 lines
include/linux/rmap.h static inline void page_dup_rmap(struct page *page,
Quoted 11 lines
include/linux/rmap.h +0
-1TTU_IGNORE_MLOCK =0x8 ,/* ignore mlock */ TTU_IGNORE_HWPOISON = 0x20, /* corrupted page is recoverable */@@ -96,7 +94,6 @@ enum ttu_flags {* do a final flush if necessary */ TTU_RMAP_LOCKED = 0x80, /* do not grab rmap lock:* caller holds it */ - TTU_SPLIT_FREEZE = 0x100, /* freeze pte under splitting thp */ };#ifdef CONFIG_MMU @@ -193,6 +190,7 @@ static inline void page_dup_rmap(struct page *page,
bool compound)
Quoted 6 lines
Quoted 6 lines
include/linux/rmap.h +1
-0int page_referenced (struct page *,int is_locked,struct mem_cgroup *memcg,unsigned long *vm_flags); +bool try_to_migrate (struct page *page,enum ttu_flags flags);bool try_to_unmap (struct page *,enum ttu_flags flags);
Quoted 6 lines
mm/huge_memory.c void vma_adjust_trans_huge(struct vm_area_struct
Quoted 6 lines
include/linux/rmap.h +0
-0/* Avoid racy checks */ mm/huge_memory.c +0
-0@@ -2351,16 +2351,16 @@ void vma_adjust_trans_huge(struct vm_area_struct
*vma,
Quoted 10 lines
Quoted 10 lines
mm/huge_memory.c +1
-2static void unmap_page (struct page *page) { -enum ttu_flags ttu_flags =TTU_IGNORE_MLOCK | -TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ; +enum ttu_flags ttu_flags =TTU_RMAP_LOCKED |TTU_SPLIT_HUGE_PMD ;bool unmap_success;VM_BUG_ON_PAGE (!PageHead (page), page);
Quoted 10 lines
mm/huge_memory.c void vma_adjust_trans_huge(struct vm_area_struct *vma, · line 2363
Quoted 8 lines
mm/huge_memory.c +4
-3if (PageAnon (page)) - ttu_flags |=TTU_SPLIT_FREEZE ; - - unmap_success =try_to_unmap (page, ttu_flags); + unmap_success =try_to_migrate (page, ttu_flags); +else + unmap_success =try_to_unmap (page, ttu_flags | +TTU_IGNORE_MLOCK );I think we need a comment here about why anonymous pages need
try_to_migrate()
Quoted 1 line
and others need
try_to_unmap().
Historically this comes from baa355fd3314 ("thp: file pages support for split_huge_page()") which says:
"We don't setup migration entries. Just unmap pages. It helps handling cases when i_size is in the middle of the page: no need handle unmap pages beyond i_size manually."
But I'll add a comment here, thanks.
- Alistair
Quoted 5 lines
Thanks.
—
Best Regards,
Yan Zi
Alistair Popple
reviewerPATCH v3 5/8 mm: Device exclusive memory access
In reply to Alistair PoppleSome devices require exclusive write access to shared virtual memory (SVM) ranges to perform atomic operations on that memory. This requires CPU page tables to be updated to deny access whilst atomic operations are occurring.
In order to do this introduce a new swap entry
type (SWP_DEVICE_EXCLUSIVE). When a SVM range needs to be marked for exclusive access by a device all page table mappings for the particular range are replaced with device exclusive swap entries. This causes any CPU access to the page to result in a fault.
Faults are resovled by replacing the faulting entry with the original mapping. This results in MMU notifiers being called which a driver uses to update access permissions such as revoking atomic access. After notifiers have been called the device will no longer have exclusive access to the region.
--- Documentation/vm/hmm.rst | 15 ++++ include/linux/rmap.h | 3 + include/linux/swap.h | 4 +- include/linux/swapops.h | 44 ++++++++++- mm/hmm.c | 5 ++ mm/memory.c | 108 +++++++++++++++++++++++++- mm/mprotect.c | 8 ++ mm/page_vma_mapped.c | 9 ++- mm/rmap.c | 163 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 352 insertions(+), 7 deletions(-)
Documentation/vm/hmm.rst +15 -0
@@ -405,6 +405,21 @@ between device driver specific code and shared common code: The lock can now be released. +Exclusive access memory +======================= + +Not all devices support atomic access to system memory. To support atomic +operations to a shared virtual memory page such a device needs access to that +page which is exclusive of any userspace access from the CPU. The +``make_device_exclusive_range()`` function can be used to make a memory range +inaccessible from userspace. + +This replaces all mappings for pages in the given range with special swap +entries. Any attempt to access the swap entry results in a fault which is +resovled by replacing the entry with the original mapping. A driver gets +notified that the mapping has been changed by MMU notifiers, after which point +it will no longer have exclusive access to the page. + Memory cgroup (memcg) and rss accounting ========================================
include/linux/rmap.h +3 -0
@@ -193,6 +193,9 @@ int page_referenced(struct page *, int is_locked,bool try_to_migrate (struct page *page,enum ttu_flags flags);bool try_to_unmap (struct page *,enum ttu_flags flags); +int make_device_exclusive_range (struct mm_struct *mm,unsigned long start, +unsigned long end,struct page **pages); +/* Avoid racy checks */ #define PVMW_SYNC (1 << 0)/* Look for migarion entries rather than present PTEs */
include/linux/swap.h +3 -1
@@ -63,9 +63,11 @@ static inline int current_is_kswapd(void)* to a special SWP_DEVICE_* entry. */ #ifdef CONFIG_DEVICE_PRIVATE -#define SWP_DEVICE_NUM 2 +#define SWP_DEVICE_NUM 4#define SWP_DEVICE_WRITE (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM)#define SWP_DEVICE_READ (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM+1) +#define SWP_DEVICE_EXCLUSIVE_WRITE (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM+2) +#define SWP_DEVICE_EXCLUSIVE_READ (MAX_SWAPFILES+SWP_HWPOISON_NUM+SWP_MIGRATION_NUM+3)#else #define SWP_DEVICE_NUM 0#endif
include/linux/swapops.h +43 -1
62 lines not on this page · raw message
mm/hmm.c +5 -0
@@ -26,6 +26,8 @@#include <linux/mmu_notifier.h> #include <linux/memory_hotplug.h> +#include "internal.h" +struct hmm_vma_walk {struct hmm_range *range;unsigned long last;
@@ -271,6 +273,9 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,if (!non_swap_entry (entry)) goto fault; +if (is_device_exclusive_entry (entry)) + goto fault; +if (is_migration_entry (entry)) {pte_unmap (ptep); hmm_vma_walk->last = addr;
mm/memory.c +106 -2
144 lines not on this page · raw message
mm/mprotect.c +8 -0
@@ -165,6 +165,14 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd, newpte =swp_entry_to_pte (entry);if (pte_swp_uffd_wp (oldpte)) newpte =pte_swp_mkuffd_wp (newpte); + }else if (is_writable_device_exclusive_entry (entry)) { + entry =make_readable_device_exclusive_entry ( +swp_offset (entry)); + newpte =swp_entry_to_pte (entry); +if (pte_swp_soft_dirty (oldpte)) + newpte =pte_swp_mksoft_dirty (newpte); +if (pte_swp_uffd_wp (oldpte)) + newpte =pte_swp_mkuffd_wp (newpte); }else { newpte = oldpte; }
mm/page_vma_mapped.c +6 -3
@@ -41,7 +41,8 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw)/* Handle un-addressable ZONE_DEVICE memory */ entry =pte_to_swp_entry (*pvmw->pte); -if (!is_device_private_entry (entry)) +if (!is_device_private_entry (entry) && + !is_device_exclusive_entry (entry))return false; }else if (!pte_present (*pvmw->pte))return false;
@@ -93,7 +94,8 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)return false; entry =pte_to_swp_entry (*pvmw->pte); -if (!is_migration_entry (entry)) +if (!is_migration_entry (entry) && + !is_device_exclusive_entry (entry))return false; pfn =swp_offset (entry);
@@ -102,7 +104,8 @@ static bool check_pte(struct page_vma_mapped_walk *pvmw)/* Handle un-addressable ZONE_DEVICE memory */ entry =pte_to_swp_entry (*pvmw->pte); -if (!is_device_private_entry (entry)) +if (!is_device_private_entry (entry) && + !is_device_exclusive_entry (entry))return false; pfn =swp_offset (entry);
mm/rmap.c +163 -0
169 lines not on this page · raw message
Signature · 1 line
2.20.1
Jason Gunthorpe
maintainerRe: [PATCH v3 5/8] mm: Device exclusive memory access
Quoted 11 lines · Alistair Popple
Some devices require exclusive write access to shared virtual memory (SVM) ranges to perform atomic operations on that memory. This requires CPU page tables to be updated to deny access whilst atomic operations are occurring.
In order to do this introduce a new swap entry
type (SWP_DEVICE_EXCLUSIVE). When a SVM range needs to be marked for exclusive access by a device all page table mappings for the particular range are replaced with device exclusive swap entries. This causes any CPU access to the page to result in a fault.
Quoted 5 lines
Faults are resovled by replacing the faulting entry with the original mapping. This results in MMU notifiers being called which a driver uses to update access permissions such as revoking atomic access. After notifiers have been called the device will no longer have exclusive access to the region.
This makes alot more sense than the prior versions!
I don't know the migration area especially well, but nothing caught my eye in here
Jason
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
RE: [PATCH v3 5/8] mm: Device exclusive memory access
In reply to Alistair PoppleQuoted 37 lines
From: Alistair Popple <[email protected]>
Sent: Thursday, February 25, 2021 11:18 PM
To: [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; dri- [email protected]; John Hubbard <[email protected]>; Ralph Campbell <[email protected]>; [email protected]; Jason Gunthorpe <[email protected]>; [email protected]; [email protected]; Alistair Popple <[email protected]>
Subject: [PATCH v3 5/8] mm: Device exclusive memory accessSome devices require exclusive write access to shared virtual memory (SVM) ranges to perform atomic operations on that memory. This requires CPU page tables to be updated to deny access whilst atomic operations are occurring.
In order to do this introduce a new swap entry type (
SWP_DEVICE_EXCLUSIVE). When a SVM range needs to be marked for exclusive access by a device all page table mappings for the particular range are replaced with device exclusive swap entries. This causes any CPU access to the page to result in a fault.Faults are resovled by replacing the faulting entry with the original mapping. This results in MMU notifiers being called which a driver uses to update access permissions such as revoking atomic access. After notifiers have been called the device will no longer have exclusive access to the region.
Signed-off-by Alistair Popple <[email protected]>--- Documentation/vm/hmm.rst | 15 ++++ include/linux/rmap.h | 3 + include/linux/swap.h | 4 +- include/linux/swapops.h | 44 ++++++++++- mm/hmm.c | 5 ++ mm/memory.c | 108 +++++++++++++++++++++++++- mm/mprotect.c | 8 ++ mm/page_vma_mapped.c | 9 ++- mm/rmap.c | 163 +++++++++++++++++++++++++++++++++++++++ 9 files changed, 352 insertions(+), 7 deletions(-)
...
mm/rmap.c void try_to_munlock(struct page *page) · line 2140
Quoted 4 lines
mm/rmap.c +4
-0+int make_device_exclusive_range (struct mm_struct *mm,unsigned long start, +unsigned long end,struct page **pages) { +long npages = (end - start) >>PAGE_SHIFT ; +long i;
Nit: you should use unsigned long for 'i' and 'npages' to match start/end.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jason Gunthorpe
maintainerRe: [PATCH v3 5/8] mm: Device exclusive memory access
In reply to Alistair PoppleQuoted 21 lines · Alistair Popple
mm/rmap.c +21
-0+/** +* make_device_exclusive_range() - Mark a range for exclusive use by a device +* @mm: mm_struct of assoicated target process +* @start: start of the region to mark for exclusive device access +* @end: end address of region +* @pages: returns the pages which were successfully mark for exclusive acces +* +* Returns: number of pages successfully marked for exclusive access +* +* This function finds the ptes mapping page(s) to the given address range and +* replaces them with special swap entries preventing userspace CPU access. On +* fault these entries are replaced with the original mapping after calling MMU +* notifiers. +*/ +int make_device_exclusive_range (struct mm_struct *mm,unsigned long start, +unsigned long end,struct page **pages) +{ +long npages = (end - start) >>PAGE_SHIFT ; +long i; + + npages = get_user_pages_remote(mm, start, npages,
mm/rmap.c void try_to_munlock(struct page *page) · line 2152
Quoted 10 lines
mm/rmap.c +10
-0+FOLL_GET |FOLL_WRITE |FOLL_SPLIT_PMD , + pages, NULL, NULL); +for (i =0 ; i < npages; i++) { +if (!trylock_page (pages[i])) { +put_page (pages[i]); + pages[i] =NULL ; +continue ; + } + +if (!try_to_protect (pages[i])) {
Isn't this racy? get_user_pages returns the ptes at an instant in time, they could have already been changed to something else?
I would think you'd want to switch to the swap entry atomically under th PTLs?
Jason
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerRe: [PATCH v3 5/8] mm: Device exclusive memory access
Quoted 4 lines · AEDT Jason Gunthorpe
Quoted 2 lines · Alistair Popple
+/**
+ *make_device_exclusive_range()- Mark a range for exclusive use by a
device
Quoted 4 lines
Quoted 4 lines
+ * @mm:
mm_structof assoicated target process
+ * @start: start of the region to mark for exclusive device access
+ * @end: end address of region
+ * @pages: returns the pages which were successfully mark for exclusive
acces
Quoted 4 lines
Quoted 4 lines
+ *
+ * Returns: number of pages successfully marked for exclusive access
+ *
+ * This function finds the ptes mapping page(s) to the given address
range and
Quoted 1 line
Quoted 1 line
+ * replaces them with special swap entries preventing userspace CPU
access. On
Quoted 1 line
Quoted 1 line
+ * fault these entries are replaced with the original mapping after
calling MMU
Quoted 3 lines
Quoted 3 lines
+ * notifiers.
+ */
+int make_device_exclusive_range(structmm_struct*mm, unsigned long
start,
Quoted 11 lines
Quoted 11 lines
mm/rmap.c +11
-0+ unsigned long end, struct page **pages) +{ + long npages = (end - start) >> PAGE_SHIFT; +long i; + + npages =get_user_pages_remote (mm, start, npages, +FOLL_GET |FOLL_WRITE |FOLL_SPLIT_PMD , + pages,NULL ,NULL ); +for (i =0 ; i < npages; i++) { +if (!trylock_page (pages[i])) { +put_page (pages[i]);
Quoted 8 lines
mm/rmap.c void try_to_munlock(struct page *page) · line 2152
Quoted 5 lines
mm/rmap.c +5
-0+ pages[i] =NULL ; +continue ; + } + +if (!try_to_protect (pages[i])) {Isn't this racy?
get_user_pagesreturns the ptes at an instant in time, they could have already been changed to something else?
Right. On it's own this does not guarantee that the page is mapped at the given location, only that a mapping won't get established without an mmu notifier callback to clear the swap entry.
The intent was a driver could use HMM or some other mechanism to keep PTEs synchronised if required. However I just looked at patch 8 in the series again and it appears I got this wrong when converting from the old migration approach:
+ mutex_unlock(&svmm->mutex);
+ ret = nouveau_atomic_range_fault(svmm, drm, args,
+ size, hmm_flags, mm);
The mutex needs to be unlocked after the range fault to ensure the PTE hasn't changed. But this ends up being a problem because try_to_protect() calls notifiers which need to take that mutex and hence deadlocks.
Quoted 2 lines
I would think you'd want to switch to the swap entry atomically under th PTLs?
That is one approach, but the reuse of get_user_pages() to walk the page tables and fault/gather the pages is a nice simplification and adding a new FOLL flag/mode to atomically swap entries doesn't seem right.
However try_to_protect() scans the PTEs again under the PTL so checking the mapping of interest actually gets replaced during the rmap walk seems like a reasonable solution. Thanks for the comments.
- Alistair
Quoted 2 lines
Jason
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Jason Gunthorpe
maintainerRe: [PATCH v3 5/8] mm: Device exclusive memory access
Quoted 9 lines · Alistair Popple
The intent was a driver could use HMM or some other mechanism to keep PTEs synchronised if required. However I just looked at patch 8 in the series again and it appears I got this wrong when converting from the old migration approach:
(quoted diff) +3
-0+ mutex_unlock(&svmm->mutex); + ret = nouveau_atomic_range_fault(svmm, drm, args, + size, hmm_flags, mm);
Quoted 3 lines
The mutex needs to be unlocked after the range fault to ensure the PTE hasn't changed. But this ends up being a problem because
try_to_protect()calls notifiers which need to take that mutex and hence deadlocks.
you have to check the notifier sequence under the mutex and loop again. The mutex should only cover programming the HW to use the pages, nothing else.
Quoted 3 lines
However
try_to_protect()scans the PTEs again under the PTL so checking the mapping of interest actually gets replaced during the rmap walk seems like a reasonable solution. Thanks for the comments.
It does seem cleaner if you can manage it, the notifier will still be needd to program the HW though
Jason
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
Alistair Popple
reviewerRe: [PATCH v3 5/8] mm: Device exclusive memory access
Quoted 1 line · AEDT Jason Gunthorpe
Quoted 1 line
However
try_to_protect()scans the PTEs again under the PTL so checking
the
Quoted 1 line
Quoted 1 line
mapping of interest actually gets replaced during the rmap walk seems like
a
Quoted 4 lines
Quoted 1 line
reasonable solution. Thanks for the comments.
It does seem cleaner if you can manage it, the notifier will still be needd to program the HW though
Checking during the rmap walk wasn't hard but ultimately pointless. As you say a range notifier and lock is required to program the hardware, which requires checking the mappings with a mmu notifier sequence anyway.
- Alistair
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerPATCH v3 6/8 mm: Selftests for exclusive device memory
In reply to Alistair PoppleAdds some selftests for exclusive device memory.
--- lib/test_hmm.c | 124 ++++++++++++++ lib/test_hmm_uapi.h | 2 + tools/testing/selftests/vm/hmm-tests.c | 219 +++++++++++++++++++++++++ 3 files changed, 345 insertions(+)
lib/test_hmm.c +124 -0
@@ -25,6 +25,7 @@#include <linux/swapops.h> #include <linux/sched/mm.h> #include <linux/platform_device.h> +#include <linux/rmap.h> #include "test_hmm_uapi.h"
@@ -46,6 +47,7 @@ struct dmirror_bounce {unsigned long cpages; }; +#define DPT_XA_TAG_ATOMIC 1UL#define DPT_XA_TAG_WRITE 3UL/*
@@ -619,6 +621,54 @@ static void dmirror_migrate_alloc_and_copy(struct migrate_vma *args, } } +static int dmirror_check_atomic (struct dmirror *dmirror,unsigned long start, +unsigned long end) +{ +unsigned long pfn; + +for (pfn = start >>PAGE_SHIFT ; pfn < (end >>PAGE_SHIFT ); pfn++) { +void *entry; +struct page *page; + + entry =xa_load (&dmirror->pt, pfn); + page =xa_untag_pointer (entry); +if (xa_pointer_tag (entry) ==DPT_XA_TAG_ATOMIC ) +return -EPERM ; + } + +return 0 ; +} + +static int dmirror_atomic_map (unsigned long start,unsigned long end, +struct page **pages,struct dmirror *dmirror) +{ +unsigned long pfn, mapped =0 ; +int i; + +/* Map the migrated pages into the device's page tables. */ +mutex_lock (&dmirror->mutex); + +for (i =0 , pfn = start >>PAGE_SHIFT ; pfn < (end >>PAGE_SHIFT ); pfn++, i++) { +void *entry; + +if (!pages[i]) +continue ; + + entry = pages[i]; + entry =xa_tag_pointer (entry,DPT_XA_TAG_ATOMIC ); + entry =xa_store (&dmirror->pt, pfn, entry,GFP_ATOMIC ); +if (xa_is_err (entry)) { +mutex_unlock (&dmirror->mutex); +return xa_err (entry); + } + + mapped++; + } + +mutex_unlock (&dmirror->mutex); +return mapped; +} +static int dmirror_migrate_finalize_and_map (struct migrate_vma *args,struct dmirror *dmirror) {
@@ -661,6 +711,71 @@ static int dmirror_migrate_finalize_and_map(struct migrate_vma *args,return 0 ; } +static int dmirror_exclusive (struct dmirror *dmirror, +struct hmm_dmirror_cmd *cmd) +{ +unsigned long start, end, addr; +unsigned long size = cmd->npages <<PAGE_SHIFT ; +struct mm_struct *mm = dmirror->notifier.mm; +struct page *pages[64 ]; +struct dmirror_bounce bounce; +unsigned long next; +int ret; + + start = cmd->addr; + end = start + size; +if (end < start) +return -EINVAL ; + +/* Since the mm is for the mirrored process, get a reference first. */ +if (!mmget_not_zero (mm)) +return -EINVAL ; + +mmap_read_lock (mm); +for (addr = start; addr < end; addr = next) { +int i, mapped; + +if (end < addr + (64 <<PAGE_SHIFT )) + next = end; +else + next = addr + (64 <<PAGE_SHIFT ); + + ret =make_device_exclusive_range (mm, addr, next, pages); + mapped =dmirror_atomic_map (addr, next, pages, dmirror); +for (i =0 ; i < ret; i++) { +if (pages[i]) { +unlock_page (pages[i]); +put_page (pages[i]); + } + } + +if (addr + (mapped <<PAGE_SHIFT ) < next) { +mmap_read_unlock (mm); +mmput (mm); +return -EBUSY ; + } + } +mmap_read_unlock (mm); +mmput (mm); + +/* Return the migrated data for verification. */ + ret =dmirror_bounce_init (&bounce, start, size); +if (ret) +return ret; +mutex_lock (&dmirror->mutex); + ret =dmirror_do_read (dmirror, start, end, &bounce); +mutex_unlock (&dmirror->mutex); +if (ret ==0 ) { +if (copy_to_user (u64_to_user_ptr (cmd->ptr), bounce.ptr, + bounce.size)) + ret = -EFAULT ; + } + + cmd->cpages = bounce.cpages; +dmirror_bounce_fini (&bounce); +return ret; +} +static int dmirror_migrate (struct dmirror *dmirror,struct hmm_dmirror_cmd *cmd) {
@@ -949,6 +1064,15 @@ static long dmirror_fops_unlocked_ioctl(struct file *filp, ret =dmirror_migrate (dmirror, &cmd);break ; +case HMM_DMIRROR_EXCLUSIVE : + ret =dmirror_exclusive (dmirror, &cmd); +break ; + +case HMM_DMIRROR_CHECK_EXCLUSIVE : + ret =dmirror_check_atomic (dmirror, cmd.addr, + cmd.addr + (cmd.npages <<PAGE_SHIFT )); +break ; +case HMM_DMIRROR_SNAPSHOT : ret =dmirror_snapshot (dmirror, &cmd);break ;
lib/test_hmm_uapi.h +2 -0
@@ -33,6 +33,8 @@ struct hmm_dmirror_cmd {#define HMM_DMIRROR_WRITE _IOWR('H', 0x01, struct hmm_dmirror_cmd)#define HMM_DMIRROR_MIGRATE _IOWR('H', 0x02, struct hmm_dmirror_cmd)#define HMM_DMIRROR_SNAPSHOT _IOWR('H', 0x03, struct hmm_dmirror_cmd) +#define HMM_DMIRROR_EXCLUSIVE _IOWR('H', 0x04, struct hmm_dmirror_cmd) +#define HMM_DMIRROR_CHECK_EXCLUSIVE _IOWR('H', 0x05, struct hmm_dmirror_cmd)/* * Values returned in hmm_dmirror_cmd.ptr for HMM_DMIRROR_SNAPSHOT.
tools/testing/selftests/vm/hmm-tests.c +219 -0
223 lines not on this page · raw message
Signature · 1 line
2.20.1
Re: [PATCH v3 6/8] mm: Selftests for exclusive device memory
Quoted 8 lines · Alistair Popple
Adds some selftests for exclusive device memory.
Signed-off-by Alistair Popple <[email protected]>--- lib/test_hmm.c | 124 ++++++++++++++ lib/test_hmm_uapi.h | 2 + tools/testing/selftests/vm/hmm-tests.c | 219 +++++++++++++++++++++++++ 3 files changed, 345 insertions(+)
Please get Ralph to review this, otherwise:
Jason
_______________________________________________
Nouveau mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/nouveau
RE: [PATCH v3 6/8] mm: Selftests for exclusive device memory
Quoted 21 lines
From: Jason Gunthorpe <[email protected]>
Sent: Monday, March 1, 2021 9:56 AM
To: Alistair Popple <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; John Hubbard <[email protected]>; Ralph Campbell <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3 6/8] mm: Selftests for exclusive device memoryQuoted 8 lines · Alistair Popple
Adds some selftests for exclusive device memory.
Signed-off-by Alistair Popple <[email protected]>--- lib/test_hmm.c | 124 ++++++++++++++ lib/test_hmm_uapi.h | 2 + tools/testing/selftests/vm/hmm-tests.c | 219 +++++++++++++++++++++++++ 3 files changed, 345 insertions(+)
Quoted 5 lines
Please get Ralph to review this, otherwise:
Acked-by Jason Gunthorpe <[email protected]>Jason
I'm working on it. Thanks for encouragement. 😊
RE: [PATCH v3 6/8] mm: Selftests for exclusive device memory
In reply to Alistair PoppleQuoted 11 lines
From: Alistair Popple <[email protected]>
Sent: Thursday, February 25, 2021 11:19 PM
To: [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; dri- [email protected]; John Hubbard <[email protected]>; Ralph Campbell <[email protected]>; [email protected]; Jason Gunthorpe <[email protected]>; [email protected]; [email protected]; Alistair Popple <[email protected]>
Subject: [PATCH v3 6/8] mm: Selftests for exclusive device memory
Quoted 3 lines
Adds some selftests for exclusive device memory.
Signed-off-by Alistair Popple <[email protected]>
One minor nit below, but you can add
Quoted 18 lines
lib/test_hmm.c +18
-0+static int dmirror_exclusive (struct dmirror *dmirror, +struct hmm_dmirror_cmd *cmd) +{ +unsigned long start, end, addr; +unsigned long size = cmd->npages <<PAGE_SHIFT ; +struct mm_struct *mm = dmirror->notifier.mm; +struct page *pages[64 ]; +struct dmirror_bounce bounce; +unsigned long next; +int ret; + + start = cmd->addr; + end = start + size; +if (end < start) +return -EINVAL ; + +/* Since the mm is for the mirrored process, get a reference first. */ +if (!mmget_not_zero (mm))
lib/test_hmm.c static int dmirror_migrate_finalize_and_map(struct migrate_vma *args, · line 741
Quoted 10 lines
lib/test_hmm.c +10
-0+return -EINVAL ; + +mmap_read_lock (mm); +for (addr = start; addr < end; addr = next) { +int i, mapped; + +if (end < addr + (64 <<PAGE_SHIFT )) + next = end; +else + next = addr + (64 <<PAGE_SHIFT );
I suggest using ARRAY_SIZE(pages) instead of '64' to make the meaning clear.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerRe: [PATCH v3 6/8] mm: Selftests for exclusive device memory
Quoted 11 lines · AEDT Ralph Campbell
Quoted 11 lines
From: Alistair Popple <[email protected]>
Sent: Thursday, February 25, 2021 11:19 PM
To: [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; dri- [email protected]; John Hubbard <[email protected]>; Ralph Campbell <[email protected]>; [email protected]; Jason Gunthorpe <[email protected]>; [email protected]; [email protected]; Alistair Popple <[email protected]>
Subject: [PATCH v3 6/8] mm: Selftests for exclusive device memory
Quoted 5 lines
Quoted 3 lines
Adds some selftests for exclusive device memory.
Signed-off-by Alistair Popple <[email protected]>One minor nit below, but you can add
Thanks Ralph. Will fix that.
Quoted 25 lines
Tested-by Ralph Campbell <[email protected]>Reviewed-by Ralph Campbell <[email protected]>Quoted 22 lines
lib/test_hmm.c +22
-0+static int dmirror_exclusive (struct dmirror *dmirror, +struct hmm_dmirror_cmd *cmd) +{ +unsigned long start, end, addr; +unsigned long size = cmd->npages <<PAGE_SHIFT ; +struct mm_struct *mm = dmirror->notifier.mm; +struct page *pages[64 ]; +struct dmirror_bounce bounce; +unsigned long next; +int ret; + + start = cmd->addr; + end = start + size; +if (end < start) +return -EINVAL ; + +/* Since the mm is for the mirrored process, get a reference first. */ +if (!mmget_not_zero (mm)) +return -EINVAL ; + +mmap_read_lock (mm); +for (addr = start; addr < end; addr = next) {
Quoted 10 lines
lib/test_hmm.c static int dmirror_migrate_finalize_and_map(struct migrate_vma *args, · line 741
Quoted 6 lines
lib/test_hmm.c +6
-0+int i, mapped; + +if (end < addr + (64 <<PAGE_SHIFT )) + next = end; +else + next = addr + (64 <<PAGE_SHIFT );I suggest using
ARRAY_SIZE(pages)instead of '64' to make the meaning clear.
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alistair Popple
reviewerPATCH v3 7/8 nouveau/svm: Refactor nouveau_range_fault
In reply to Alistair PoppleCall mmu_interval_notifier_insert() as part of nouveau_range_fault(). This doesn't introduce any functional change but makes it easier for a subsequent patch to alter the behaviour of nouveau_range_fault() to support GPU atomic operations.
--- drivers/gpu/drm/nouveau/nouveau_svm.c | 34 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 14 deletions(-)
drivers/gpu/drm/nouveau/nouveau_svm.c +20 -14
@@ -567,18 +567,27 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm,unsigned long hmm_pfns[1 ];struct hmm_range range = { .notifier = ¬ifier->notifier, - .start = notifier->notifier.interval_tree.start, - .end = notifier->notifier.interval_tree.last +1 , .default_flags = hmm_flags, .hmm_pfns = hmm_pfns, .dev_private_owner = drm->dev, }; -struct mm_struct *mm = notifier->notifier.mm; +struct mm_struct *mm = svmm->notifier.mm;int ret; + ret =mmu_interval_notifier_insert (¬ifier->notifier, mm, + args->p.addr, args->p.size, + &nouveau_svm_mni_ops); +if (ret) +return ret; + + range.start = notifier->notifier.interval_tree.start; + range.end = notifier->notifier.interval_tree.last +1 ; +while (true) { -if (time_after (jiffies, timeout)) -return -EBUSY ; +if (time_after (jiffies, timeout)) { + ret = -EBUSY ; + goto out; + } range.notifier_seq =mmu_interval_read_begin (range.notifier);mmap_read_lock (mm);
@@ -587,7 +596,7 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm,if (ret) {if (ret == -EBUSY )continue ; -return ret; + goto out; }mutex_lock (&svmm->mutex);
@@ -606,6 +615,9 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm, svmm->vmm->vmm.object.client->super = false;mutex_unlock (&svmm->mutex); +out: +mmu_interval_notifier_remove (¬ifier->notifier); +return ret; }
@@ -727,14 +739,8 @@ nouveau_svm_fault(struct nvif_notify *notify) } notifier.svmm = svmm; - ret =mmu_interval_notifier_insert (¬ifier.notifier, mm, - args.i.p.addr, args.i.p.size, - &nouveau_svm_mni_ops); -if (!ret) { - ret =nouveau_range_fault (svmm, svm->drm, &args.i, -sizeof (args), hmm_flags, ¬ifier); -mmu_interval_notifier_remove (¬ifier.notifier); - } + ret =nouveau_range_fault (svmm, svm->drm, &args.i, +sizeof (args), hmm_flags, ¬ifier);mmput (mm); limit = args.i.p.addr + args.i.p.size;
Signature · 1 line
2.20.1
Alistair Popple
reviewerPATCH v3 8/8 nouveau/svm: Implement atomic SVM access
In reply to Alistair PoppleSome NVIDIA GPUs do not support direct atomic access to system memory via PCIe. Instead this must be emulated by granting the GPU exclusive access to the memory. This is achieved by replacing CPU page table entries with special swap entries that fault on userspace access.
The driver then grants the GPU permission to update the page undergoing atomic access via the GPU page tables. When CPU access to the page is required a CPU fault is raised which calls into the device driver via MMU notifiers to revoke the atomic access. The original page table entries are then restored allowing CPU access to proceed.
--- drivers/gpu/drm/nouveau/include/nvif/if000c.h | 1 + drivers/gpu/drm/nouveau/nouveau_svm.c | 88 ++++++++++++++++--- drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h | 1 + .../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c | 6 ++ 4 files changed, 83 insertions(+), 13 deletions(-)
drivers/gpu/drm/nouveau/include/nvif/if000c.h +1 -0
@@ -77,6 +77,7 @@ struct nvif_vmm_pfnmap_v0 {#define NVIF_VMM_PFNMAP_V0_APER 0x00000000000000f0ULL#define NVIF_VMM_PFNMAP_V0_HOST 0x0000000000000000ULL#define NVIF_VMM_PFNMAP_V0_VRAM 0x0000000000000010ULL +#define NVIF_VMM_PFNMAP_V0_A 0x0000000000000004ULL#define NVIF_VMM_PFNMAP_V0_W 0x0000000000000002ULL#define NVIF_VMM_PFNMAP_V0_V 0x0000000000000001ULL#define NVIF_VMM_PFNMAP_V0_NONE 0x0000000000000000ULL
drivers/gpu/drm/nouveau/nouveau_svm.c +75 -13
145 lines not on this page · raw message
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmm.h +1 -0
@@ -178,6 +178,7 @@ void nvkm_vmm_unmap_region(struct nvkm_vmm *, struct nvkm_vma *);#define NVKM_VMM_PFN_APER 0x00000000000000f0ULL#define NVKM_VMM_PFN_HOST 0x0000000000000000ULL#define NVKM_VMM_PFN_VRAM 0x0000000000000010ULL +#define NVKM_VMM_PFN_A 0x0000000000000004ULL#define NVKM_VMM_PFN_W 0x0000000000000002ULL#define NVKM_VMM_PFN_V 0x0000000000000001ULL#define NVKM_VMM_PFN_NONE 0x0000000000000000ULL
drivers/gpu/drm/nouveau/nvkm/subdev/mmu/vmmgp100.c +6 -0
@@ -88,6 +88,9 @@ gp100_vmm_pgt_pfn(struct nvkm_vmm *vmm, struct nvkm_mmu_pt *pt,if (!(*map->pfn &NVKM_VMM_PFN_W )) data |=BIT_ULL (6 );/* RO. */ +if (!(*map->pfn &NVKM_VMM_PFN_A )) + data |=BIT_ULL (7 );/* Atomic disable. */ +if (!(*map->pfn &NVKM_VMM_PFN_VRAM )) { addr = *map->pfn >>NVKM_VMM_PFN_ADDR_SHIFT ; addr = dma_map_page(dev, pfn_to_page(addr), 0,
@@ -322,6 +325,9 @@ gp100_vmm_pd0_pfn(struct nvkm_vmm *vmm, struct nvkm_mmu_pt *pt,if (!(*map->pfn &NVKM_VMM_PFN_W )) data |=BIT_ULL (6 );/* RO. */ +if (!(*map->pfn &NVKM_VMM_PFN_A )) + data |=BIT_ULL (7 );/* Atomic disable. */ +if (!(*map->pfn &NVKM_VMM_PFN_VRAM )) { addr = *map->pfn >>NVKM_VMM_PFN_ADDR_SHIFT ; addr = dma_map_page(dev, pfn_to_page(addr), 0,
Signature · 6 lines
2.20.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel