EMM Notifier V3
Subsystems (14)
cover EMM Notifier V3
V2->V3:
- Fix rcu issues
- Fix emm_referenced handling
- Use Andrea's mm_lock/unlock to prevent registration races.
- Keep simple API since there does not seem to be a need to add additional callbacks (mm_lock does not require callbacks like emm_start/stop that I envisioned).
- Reduce CC list (the volume we are producing here must be annoying...).
V1->V2:
- Additional optimizations in the VM
- Convert vm spinlocks to rw sems.
- Add XPMEM driver (requires sleeping in callbacks)
- Add XPMEM example
This patch implements a simple callback for device drivers that establish their own references to pages (KVM, GRU, XPmem, RDMA/Infiniband, DMA engines etc). These references are unknown to the VM (therefore external).
With these callbacks it is possible for the device driver to release external references when the VM requests it. This enables swapping, page migration and allows support of remapping, permission changes etc etc for the externally mapped memory.
With this functionality it becomes also possible to avoid pinning or mlocking pages (commonly done to stop the VM from unmapping device mapped pages).
A device driver must subscribe to a process using
emm_register_notifier(struct emm_notifier *, struct mm_struct *)
The VM will then perform callbacks for operations that unmap or change permissions of pages in that address space. When the process terminates the callback function is called with emm_release.
Callbacks are performed before and after the unmapping action of the VM.
emm_invalidate_start before
emm_invalidate_end after
The device driver must hold off establishing new references to pages in the range specified between a callback with emm_invalidate_start and the subsequent call with emm_invalidate_end set. This allows the VM to ensure that no concurrent driver actions are performed on an address range while performing remapping or unmapping operations.
This patchset contains additional modifications needed to ensure that the callbacks can sleep. For that purpose two key locks in the vm need to be converted to rw_sems. These patches are brand new, invasive and need extensive discussion and evaluation.
The first patch alone may be applied if callbacks in atomic context are sufficient for a device driver (likely the case for KVM and GRU and simple DMA drivers).
Following the VM modifications is the XPMEM device driver that allows sharing of memory between processes running on different instances of Linux. This is also a prototype. It is known to run trivial sample programs included as the last patch.
PATCH 1/10 emm: mm_lock: Lock a process against reclaim
Provide a way to lock an mm_struct against reclaim (try_to_unmap etc). This is necessary for the invalidate notifier approaches so that they can reliably add and remove a notifier.
--- include/linux/mm.h | 10 ++++++++ mm/mmap.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+)
include/linux/mm.h +10 -0
@@ -1050,6 +1050,16 @@ extern int install_special_mapping(struc unsigned long addr, unsigned long len, unsigned long flags, struct page **pages); +/* +* Locking and unlocking am mm against reclaim. +* +* mm_lock will take mmap_sem writably (to prevent additional vmas from being +* added) and then take all mapping locks of the existing vmas. With that +* reclaim is effectively stopped. +*/ +extern void mm_lock (struct mm_struct *mm); +extern void mm_unlock (struct mm_struct *mm); +extern unsigned long get_unmapped_area (struct file *,unsigned long ,unsigned long ,unsigned long ,unsigned long ); extern unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
mm/mmap.c +66 -0
69 lines not on this page · raw message
Re: [patch 01/10] emm: mm_lock: Lock a process against reclaim
Quoted 43 lines · Christoph Lameter
Provide a way to lock an
mm_structagainst reclaim (try_to_unmapetc). This is necessary for the invalidate notifier approaches so that they can reliably add and remove a notifier.Signed-off-by Andrea Arcangeli <[email protected]>Signed-off-by Christoph Lameter <[email protected]>--- include/linux/mm.h | 10 ++++++++ mm/mmap.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+)Index:
linux-2.6/include/linux/mm.h
===================================================================include/linux/mm.h +10
-0@@ -1050,6 +1050,16 @@ extern int install_special_mapping(struc unsigned long addr, unsigned long len, unsigned long flags, struct page **pages); +/* +* Locking and unlocking am mm against reclaim. +* +* mm_lock will take mmap_sem writably (to prevent additional vmas from being +* added) and then take all mapping locks of the existing vmas. With that +* reclaim is effectively stopped. +*/ +extern void mm_lock (struct mm_struct *mm); +extern void mm_unlock (struct mm_struct *mm); +extern unsigned long get_unmapped_area (struct file *,unsigned long ,unsigned long ,unsigned long ,unsigned long ); extern unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,Index:
linux-2.6/mm/mmap.c
===================================================================mm/mmap.c +1
-0@@ -2242,3 +2242,69 @@ int install_special_mapping(struct mm_streturn 0 ; } +
mm/mmap.c int install_special_mapping(struct mm_st · line 2255
Quoted 10 lines
mm/mmap.c +10
-0+static void mm_lock_unlock (struct mm_struct *mm,int lock) +{ +struct vm_area_struct *vma; +spinlock_t *i_mmap_lock_last, *anon_vma_lock_last; + + i_mmap_lock_last =NULL ; +for (;;) { +spinlock_t *i_mmap_lock = (spinlock_t *)-1UL ; +for (vma = mm->mmap; vma; vma = vma->vm_next) + if (vma->vm_file && vma->vm_file->f_mapping &&
I think you can break this if() down a bit:
if (!(vma->vm_file && vma->vm_file->f_mapping)) continue;
mm/mmap.c int install_special_mapping(struct mm_st · line 2263
Quoted 9 lines
mm/mmap.c +8
-0+ (unsigned long ) i_mmap_lock > + (unsigned long ) + &vma->vm_file->f_mapping->i_mmap_lock && + (unsigned long ) + &vma->vm_file->f_mapping->i_mmap_lock > + (unsigned long) i_mmap_lock_last) + i_mmap_lock = + &vma->vm_file->f_mapping->i_mmap_lock;
So this is an O(n^2) algorithm to take the i_mmap_locks from low to high order? A comment would be nice. And O(n^2)? Ouch. How often is it called?
And is it necessary to mush lock and unlock together? Unlock ordering doesn't matter, so you should just be able to have a much simpler loop, no?
Quoted 52 lines
mm/mmap.c +47
-0+if (i_mmap_lock == (spinlock_t *)-1UL ) +break ; + i_mmap_lock_last = i_mmap_lock; +if (lock) +spin_lock (i_mmap_lock); +else +spin_unlock (i_mmap_lock); + } + + anon_vma_lock_last =NULL ; +for (;;) { +spinlock_t *anon_vma_lock = (spinlock_t *)-1UL ; +for (vma = mm->mmap; vma; vma = vma->vm_next) +if (vma->anon_vma && + (unsigned long ) anon_vma_lock > + (unsigned long ) &vma->anon_vma->lock && + (unsigned long ) &vma->anon_vma->lock > + (unsigned long ) anon_vma_lock_last) + anon_vma_lock = &vma->anon_vma->lock; +if (anon_vma_lock == (spinlock_t *)-1UL ) +break ; + anon_vma_lock_last = anon_vma_lock; +if (lock) +spin_lock (anon_vma_lock); +else +spin_unlock (anon_vma_lock); + } +} + +/* +* This operation locks against the VM for all pte/vma/mm related +* operations that could ever happen on a certain mm. This includes +* vmtruncate, try_to_unmap, and all page faults. The holder +* must not hold any mm related lock. A single task can't take more +* than one mm lock in a row or it would deadlock. +*/ +void mm_lock (struct mm_struct * mm) +{ +down_write (&mm->mmap_sem); +mm_lock_unlock (mm,1 ); +} + +void mm_unlock (struct mm_struct *mm) +{ +mm_lock_unlock (mm,0 ); +up_write (&mm->mmap_sem); +}
Re: [patch 01/10] emm: mm_lock: Lock a process against reclaim
Quoted 4 lines · Jeremy Fitzhardinge
I think you can break this
if()down a bit:if (!(vma->vm_file && vma->vm_file->f_mapping)) continue;
It makes no difference at runtime, coding style preferences are quite subjective.
Quoted 3 lines
So this is an O(n^2) algorithm to take the
i_mmap_locksfrom low to high order? A comment would be nice. And O(n^2)? Ouch. How often is it called?
It's called a single time when the mmu notifier is registered. It's a very slow path of course. Any other approach to reduce the complexity would require memory allocations and it would require mmu_notifier_register to return -ENOMEM failure. It didn't seem worth it.
Quoted 2 lines
And is it necessary to mush lock and unlock together? Unlock ordering doesn't matter, so you should just be able to have a much simpler loop, no?
That avoids duplicating .text. Originally they were separated. unlock can't be a simpler loop because I didn't reserve vm_flags bitflags to do a single O(N) loop for unlock. If you do malloc+fork+munmap two vmas will point to the same anon-vma lock, that's why the unlock isn't simpler unless I mark what I locked with a vm_flags bitflag.
Re: [patch 01/10] emm: mm_lock: Lock a process against reclaim
Quoted 8 lines · Andrea Arcangeli
Quoted 4 lines · Jeremy Fitzhardinge
I think you can break this
if()down a bit:if (!(vma->vm_file && vma->vm_file->f_mapping)) continue;It makes no difference at runtime, coding style preferences are quite subjective.
I'll have to concurr with Jeremy here, please break that monstrous if stmt down. It might not matter to the compiler, but it sure as hell helps for anyone trying to understand/maintain the thing.
Re: [patch 01/10] emm: mm_lock: Lock a process against reclaim
In reply to Andrea ArcangeliQuoted 11 lines · Andrea Arcangeli
Quoted 5 lines · Jeremy Fitzhardinge
I think you can break this
if()down a bit:if (!(vma->vm_file && vma->vm_file->f_mapping)) continue;It makes no difference at runtime, coding style preferences are quite subjective.
Well, overall the formatting of that if statement is very hard to read. Separating out the logically distinct pieces in to different ifs at least shows the reader that they are distinct. Aside from that, doing some manual CSE to remove all the casts and expose the actual thing you're testing for would help a lot (are the casts even necessary?).
Quoted 9 lines
Quoted 2 lines
So this is an O(n^2) algorithm to take the
i_mmap_locksfrom low to high order? A comment would be nice. And O(n^2)? Ouch. How often is it called?It's called a single time when the mmu notifier is registered. It's a very slow path of course. Any other approach to reduce the complexity would require memory allocations and it would require
mmu_notifier_registerto return-ENOMEMfailure. It didn't seem worth it.
It's per-mm though. How many processes would need to have notifiers?
Quoted 8 lines
Quoted 2 lines
And is it necessary to mush lock and unlock together? Unlock ordering doesn't matter, so you should just be able to have a much simpler loop, no?
That avoids duplicating .text. Originally they were separated. unlock can't be a simpler loop because I didn't reserve
vm_flagsbitflags to do a single O(N) loop for unlock. If you do malloc+fork+munmap two vmas will point to the same anon-vma lock, that's why the unlock isn't simpler unless I mark what I locked with avm_flagsbitflag.
Well, its definitely going to need more comments then. I assumed it would end up locking everything, so unlocking everything would be sufficient.
J
Re: [patch 01/10] emm: mm_lock: Lock a process against reclaim
Quoted 1 line · Jeremy Fitzhardinge
It's per-mm though. How many processes would need to have notifiers?
There can be up to hundreds of VM in a single system. Not sure to understand the point of the question though.
Quoted 2 lines
Well, its definitely going to need more comments then. I assumed it would end up locking everything, so unlocking everything would be sufficient.
After your comments, I'm writing an alternate version that will guarantee a O(N) worst case to both sigkill and cond_resched but frankly this is low priority. Without mmu notifiers /dev/kvm can't be given to a normal luser without at least losing mlock ulimits, so lack of a mmu notifiers is a bigger issue than whatever complexity in mm_lock as far as /dev/kvm ownership is concerned.
PATCH 2/10 emm: notifier logic
In reply to Christoph LameterThis patch implements a simple callback for device drivers that establish their own references to pages (KVM, GRU, XPmem, RDMA/Infiniband, DMA engines etc). These references are unknown to the VM (therefore external).
With these callbacks it is possible for the device driver to release external references when the VM requests it. This enables swapping, page migration and allows support of remapping, permission changes etc etc for externally mapped memory.
With this functionality it becomes also possible to avoid pinning or mlocking pages (commonly done to stop the VM from unmapping device mapped pages).
A device driver must subscribe to a process using
emm_register_notifier(struct emm_notifier *, struct mm_struct *)
The VM will then perform callbacks for operations that unmap or change permissions of pages in that address space. When the process terminates the callback function is called with emm_release.
Callbacks are performed before and after the unmapping action of the VM.
emm_invalidate_start before
emm_invalidate_end after
The device driver must hold off establishing new references to pages in the range specified between a callback with emm_invalidate_start and the subsequent call with emm_invalidate_end set. This allows the VM to ensure that no concurrent driver actions are performed on an address range while performing remapping or unmapping operations.
Callbacks are mostly performed in a non atomic context. However, in various places spinlocks are held to traverse rmaps. So this patch here is only useful for those devices that can remove mappings in an atomic context (f.e. KVM/GRU).
If the rmap spinlocks are converted to semaphores then all callbacks will be performed in a nonatomic context. No additional changes will be necessary to this patchset.
V1->V2:
- page_referenced_one: Do not increment reference count if it is already != 0.
- Use rcu_assign_pointer and rcu_derefence_pointer instead of putting in our own barriers.
V2->V3:
- Fix rcu (thanks Paul)
- Fix exit code handling to come up with the right semantings for emm_referenced (thanks Andrea)
- Call mm_lock/mm_unlock to protect against registration races.
--- include/linux/mm_types.h | 3 + include/linux/rmap.h | 50 +++++++++++++++++++++++ kernel/fork.c | 3 + mm/Kconfig | 5 ++ mm/filemap_xip.c | 4 + mm/fremap.c | 2 mm/hugetlb.c | 3 + mm/memory.c | 42 +++++++++++++++---- mm/mmap.c | 3 + mm/mprotect.c | 3 + mm/mremap.c | 4 + mm/rmap.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++- 12 files changed, 212 insertions(+), 10 deletions(-)
include/linux/mm_types.h +3 -0
@@ -225,6 +225,9 @@ struct mm_struct {#ifdef CONFIG_CGROUP_MEM_RES_CTLR struct mem_cgroup *mem_cgroup;#endif +#ifdef CONFIG_EMM_NOTIFIER +struct emm_notifier *emm_notifier; +#endif };#endif /* _LINUX_MM_TYPES_H */
mm/Kconfig +5 -0
@@ -193,3 +193,8 @@ config NR_QUICK config VIRT_TO_BUS def_bool y depends on !ARCH_NO_VIRT_TO_BUS + +config EMM_NOTIFIER + def_bool n + bool "External Mapped Memory Notifier for drivers directly mapping memory" +
include/linux/rmap.h +50 -0
56 lines not on this page · raw message
mm/rmap.c +98 -2
158 lines not on this page · raw message
kernel/fork.c +3 -0
@@ -362,6 +362,9 @@ static struct mm_struct * mm_init(structif (likely (!mm_alloc_pgd (mm))) { mm->def_flags =0 ; +#ifdef CONFIG_EMM_NOTIFIER + mm->emm_notifier =NULL ; +#endif return mm; }
mm/memory.c +34 -8
137 lines not on this page · raw message
mm/mmap.c +3 -0
@@ -1744,6 +1744,7 @@ static void unmap_region(struct mm_strucstruct mmu_gather *tlb;unsigned long nr_accounted =0 ; +emm_notify (mm, emm_invalidate_start, start, end);lru_add_drain (); tlb =tlb_gather_mmu (mm,0 );update_hiwater_rss (mm);
@@ -1752,6 +1753,7 @@ static void unmap_region(struct mm_strucfree_pgtables (&tlb, vma, prev? prev->vm_end:FIRST_USER_ADDRESS , next? next->vm_start:0 );tlb_finish_mmu (tlb, start, end); +emm_notify (mm, emm_invalidate_end, start, end); }/*
@@ -2038,6 +2040,7 @@ void exit_mmap(struct mm_struct *mm)/* mm's last user has gone, and its about to be pulled down */ arch_exit_mmap (mm); +emm_notify (mm, emm_release,0 ,TASK_SIZE );lru_add_drain ();flush_cache_mm (mm);
mm/mprotect.c +3 -0
@@ -21,6 +21,7 @@#include <linux/syscalls.h> #include <linux/swap.h> #include <linux/swapops.h> +#include <linux/rmap.h> #include <asm/uaccess.h> #include <asm/pgtable.h> #include <asm/cacheflush.h>
@@ -198,10 +199,12 @@ success: dirty_accountable =1 ; } +emm_notify (mm, emm_invalidate_start, start, end);if (is_vm_hugetlb_page (vma))hugetlb_change_protection (vma, start, end, vma->vm_page_prot);else change_protection (vma, start, end, vma->vm_page_prot, dirty_accountable); +emm_notify (mm, emm_invalidate_end, start, end);vm_stat_account (mm, oldflags, vma->vm_file, -nrpages);vm_stat_account (mm, newflags, vma->vm_file, nrpages);return 0 ;
mm/mremap.c +4 -0
@@ -18,6 +18,7 @@#include <linux/highmem.h> #include <linux/security.h> #include <linux/syscalls.h> +#include <linux/rmap.h> #include <asm/uaccess.h> #include <asm/cacheflush.h>
@@ -74,7 +75,9 @@ static void move_ptes(struct vm_area_strstruct mm_struct *mm = vma->vm_mm;pte_t *old_pte, *new_pte, pte;spinlock_t *old_ptl, *new_ptl; +unsigned long old_start = old_addr; +emm_notify (mm, emm_invalidate_start, old_start, old_end);if (vma->vm_file) {/* * Subtle point from Rajesh Venkatasubramanian: before
@@ -116,6 +119,7 @@ static void move_ptes(struct vm_area_strpte_unmap_unlock (old_pte -1 , old_ptl);if (mapping)spin_unlock (&mapping->i_mmap_lock); +emm_notify (mm, emm_invalidate_end, old_start, old_end); }#define LATENCY_LIMIT (64 * PAGE_SIZE)
mm/filemap_xip.c +4 -0
@@ -190,6 +190,8 @@ __xip_unmap (struct address_space * mapp address = vma->vm_start + ((pgoff - vma->vm_pgoff) <<PAGE_SHIFT );BUG_ON (address < vma->vm_start || address >= vma->vm_end); +emm_notify (mm, emm_invalidate_start, + address, address +PAGE_SIZE ); pte =page_check_address (page, mm, address, &ptl);if (pte) {/* Nuke the page table entry. */
@@ -201,6 +203,8 @@ __xip_unmap (struct address_space * mapppte_unmap_unlock (pte, ptl);page_cache_release (page); } +emm_notify (mm, emm_invalidate_end, + address, address +PAGE_SIZE ); }spin_unlock (&mapping->i_mmap_lock); }
mm/fremap.c +2 -0
@@ -214,7 +214,9 @@ asmlinkage long sys_remap_file_pages(unsspin_unlock (&mapping->i_mmap_lock); } +emm_notify (mm, emm_invalidate_start, start, end); err =populate_range (mm, vma, start, size, pgoff); +emm_notify (mm, emm_invalidate_end, start, end);if (!err && !(flags &MAP_NONBLOCK )) {if (unlikely (has_write_lock)) {downgrade_write (&mm->mmap_sem);
mm/hugetlb.c +3 -0
@@ -14,6 +14,7 @@#include <linux/mempolicy.h> #include <linux/cpuset.h> #include <linux/mutex.h> +#include <linux/rmap.h> #include <asm/page.h> #include <asm/pgtable.h>
@@ -799,6 +800,7 @@ void __unmap_hugepage_range(struct vm_arBUG_ON (start & ~HPAGE_MASK );BUG_ON (end & ~HPAGE_MASK ); +emm_notify (mm, emm_invalidate_start, start, end);spin_lock (&mm->page_table_lock);for (address = start; address < end; address +=HPAGE_SIZE ) { ptep =huge_pte_offset (mm, address);
@@ -819,6 +821,7 @@ void __unmap_hugepage_range(struct vm_ar }spin_unlock (&mm->page_table_lock);flush_tlb_range (vma, start, end); +emm_notify (mm, emm_invalidate_end, start, end);list_for_each_entry_safe (page, tmp, &page_list, lru) {list_del (&page->lru);put_page (page);
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Re: [patch 02/10] emm: notifier logic
mm/rmap.c pte_t *page_check_address(struct page *p · line 300
Quoted 9 lines · Christoph Lameter
mm/rmap.c +9
-0+mm_lock (mm); + e->next = mm->emm_notifier; +/* +* The update to emm_notifier (e->next) must be visible +* before the pointer becomes visible. +* rcu_assign_pointer() does exactly what we need. +*/ +rcu_assign_pointer (mm->emm_notifier, e); +mm_unlock (mm);
My mm_lock solution makes all rcu serialization an unnecessary overhead so you should remove it like I already did in #v11. If it wasn't the case, then mm_lock wouldn't be a definitive fix for the race.
mm/rmap.c pte_t *page_check_address(struct page *p · line 340
Quoted 1 line
mm/rmap.c +1
-0+ e =rcu_dereference (e->next);
Same here.
Re: [patch 02/10] emm: notifier logic
Quoted 7 lines · Andrea Arcangeli
mm/rmap.c pte_t *page_check_address(struct page *p · line 300
Quoted 2 lines
mm/rmap.c +2
-0+rcu_assign_pointer (mm->emm_notifier, e); +mm_unlock (mm);My
mm_locksolution makes all rcu serialization an unnecessary overhead so you should remove it like I already did in #v11. If it wasn't the case, thenmm_lockwouldn't be a definitive fix for the race.
There still could be junk in the cache of one cpu. If you just read the new pointer but use the earlier content pointed to then you have a problem.
So a memory fence / barrier is needed to guarantee that the contents pointed to are fetched after the pointer.
Re: [patch 02/10] emm: notifier logic
Quoted 10 lines · Christoph Lameter
Quoted 7 lines · Andrea Arcangeli
mm/rmap.c pte_t *page_check_address(struct page *p · line 300
Quoted 2 lines
mm/rmap.c +2
-0+rcu_assign_pointer (mm->emm_notifier, e); +mm_unlock (mm);My
mm_locksolution makes all rcu serialization an unnecessary overhead so you should remove it like I already did in #v11. If it wasn't the case, thenmm_lockwouldn't be a definitive fix for the race.
Quoted 3 lines
There still could be junk in the cache of one cpu. If you just read the new pointer but use the earlier content pointed to then you have a problem.
There can't be junk, spinlocks provides semantics of proper memory barriers, just like rcu, so it's entirely superflous.
There could be junk only if any of the mmu_notifier_* methods would be invoked _outside_ the i_mmap_lock and _outside_ the anon_vma and outside the mmap_sem, that is never the case of course.
Quoted 2 lines
So a memory fence / barrier is needed to guarantee that the contents pointed to are fetched after the pointer.
It's not needed... if you were right we could never possibly run a list_for_each inside any spinlock protected critical section and we'd always need to use the _rcu version instead. The _rcu version is needed only when the list walk happens outside the spinlock critical section of course (rcu = no spinlock cacheline exlusive write operation in the read side, here the read side takes the spinlock big time).
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Re: [patch 02/10] emm: notifier logic
Quoted 9 lines · Andrea Arcangeli
Quoted 8 lines
Quoted 4 lines
My
mm_locksolution makes all rcu serialization an unnecessary overhead so you should remove it like I already did in #v11. If it wasn't the case, thenmm_lockwouldn't be a definitive fix for the race.There still could be junk in the cache of one cpu. If you just read the new pointer but use the earlier content pointed to then you have a problem.
Quoted 6 lines
There can't be junk, spinlocks provides semantics of proper memory barriers, just like rcu, so it's entirely superflous.
There could be junk only if any of the mmu_notifier_* methods would be invoked _outside_ the
i_mmap_lockand _outside_ theanon_vmaand outside themmap_sem, that is never the case of course.
So we use other locks to perform serialization on the list chains? Basically the list chains are protected by either mmap_sem or an rmap lock? We need to document that.
In that case we could also add an unregister function.
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Re: [patch 02/10] emm: notifier logic
Quoted 14 lines · Christoph Lameter
Quoted 12 lines · Andrea Arcangeli
Quoted 8 lines
Quoted 4 lines
My
mm_locksolution makes all rcu serialization an unnecessary overhead so you should remove it like I already did in #v11. If it wasn't the case, thenmm_lockwouldn't be a definitive fix for the race.There still could be junk in the cache of one cpu. If you just read the new pointer but use the earlier content pointed to then you have a problem.
There can't be junk, spinlocks provides semantics of proper memory barriers, just like rcu, so it's entirely superflous.
Quoted 7 lines
Quoted 3 lines
There could be junk only if any of the mmu_notifier_* methods would be invoked _outside_ the
i_mmap_lockand _outside_ theanon_vmaand outside themmap_sem, that is never the case of course.So we use other locks to perform serialization on the list chains? Basically the list chains are protected by either
mmap_semor an rmap lock? We need to document that.
I thought it was obvious, if it wasn't the case how could mm_lock fix any range_begin/range_end race? Also to document it you've just to remove _rcu, the only confusion could arise from reading your patch, mine couldn't raise any doubt that rcu isn't needed and regular spinlocks/semaphores are serializing all methods.
Quoted 1 line
In that case we could also add an unregister function.
Indeed, but it still can't run after mm_users == 0. So for unregister to work one has to boost the mm_users first. exit_mmap doesn't take any lock when destroying the mm because it assumes nobody is messing with the mm at that time. So that requirement doesn't change, but now one can unregister before mm_users is dropped to 0.
Also I wonder if I should make a new version of the mm_lock/unlock so that they will guarantee SIGKILL handling in O(N) anywhere inside mm_lock or mm_unlock, where N is the number of vmas, that will either require a VM_MM_LOCK_I/VM_MM_LOCK_A bitflag, or a vmalloc of two bitflag arrays inside the mmap_sem critical section returned by mm_lock as a cookie and passed as param to mm_unlock. The SIGKILL check is mostly worthless in spin_lock context (especially on UP or low-smp) but given the later patches switches all relevant VM locks to mutexes (this should happen under a config option to avoid hurting server performance), it might be worth it. That will require mmu_notifier_register to return both -EINTR and -ENOMEM if using the vmalloc trick to avoid registering two more vm_flags
bitflags. Alternatively we can have mm_lock fail with -EPERM if there aren't enough capabilities and the number of vmas is bigger than a certain number. This is more or less like the requirement to attach during startup. This is preferable IMHO because it's effective even without preempt-rt and in turn with all locks being spinlocks for maximum performance, so I'll likely release #v12 with this change. In any case the mmu_notifier_register will need to return error (an unregister as well for that matter). But those are very minor issues, #v11 can go in -mm now to ensure mmu notifiers will be shipped with 2.6.26rc.
Re: [patch 02/10] emm: notifier logic
It may also be useful to allow invalidate_start() to fail in some contexts (try_to_unmap f.e., maybe if a certain flag is passed). This may allow the device to get out of tight situations (pending I/O f.e. or time out if there is no response for network communications). But then that complicates the API.
Re: [patch 02/10] emm: notifier logic
Quoted 5 lines · Christoph Lameter
It may also be useful to allow
invalidate_start()to fail in some contexts (try_to_unmapf.e., maybe if a certain flag is passed). This may allow the device to get out of tight situations (pending I/O f.e. or time out if there is no response for network communications). But then that complicates the API.
That also complicates the fact that there can't be a spte mapped and a pte not mapped or the spte would leak unswappable memory, so a failure should re-establish the pte and undo the ptep_clear_flush or equivalent... I think we can change the API later if needed. This is an internal-only API invisible to userland so it can change and break anytime to make the whole kernel faster and better (ask Greg for kernel internal APIs).
One important detail is that because the secondary mmu page fault can happen concurrently against invaldiate_page (there wasn't a range_begin to block it), the secondary mmu page fault must ensure that the pte is still established, before establishing the spte (with proper locking that will block a concurrent invalidate_page). Having a range_begin before the ptep_clear_flush effectively make lifes a bit easier but it's not needed as those are locking issues that the driver can solve (unlike range_begin being missed, now fixed by mm_lock) and this allows for higher performance both when the lock is armed and disarmed. I'm going to solve all the locking for kvm with spinlocks and/or seqlocks to avoid any dependency on the patches that makes the mmu notifier sleep capable.
PATCH 3/10 emm: Move tlb flushing into free_pgtables
In reply to Christoph LameterMove the tlb flushing into free_pgtables. The conversion of the locks taken for reverse map scanning would require taking sleeping locks in free_pgtables(). Moving the tlb flushing into free_pgtables allows sleeping in parts of free_pgtables().
This means that we do a tlb_finish_mmu() before freeing the page tables. Strictly speaking there may not be the need to do another tlb flush after freeing the tables. But its the only way to free a series of page table pages from the tlb list. And we do not want to call into the page allocator for performance reasons. Aim9 numbers look okay after this patch.
--- include/linux/mm.h | 4 ++-- mm/memory.c | 14 ++++++++++---- mm/mmap.c | 6 +++--- 3 files changed, 15 insertions(+), 9 deletions(-)
include/linux/mm.h +2 -2
@@ -751,8 +751,8 @@ int walk_page_range(const struct mm_stru void *private);void free_pgd_range (struct mmu_gather **tlb,unsigned long addr,unsigned long end,unsigned long floor,unsigned long ceiling); -void free_pgtables (struct mmu_gather **tlb,struct vm_area_struct *start_vma, -unsigned long floor,unsigned long ceiling); +void free_pgtables (struct vm_area_struct *start_vma,unsigned long floor, +unsigned long ceiling);int copy_page_range (struct mm_struct *dst,struct mm_struct *src,struct vm_area_struct *vma); void unmap_mapping_range(struct address_space *mapping,
mm/memory.c +10 -4
@@ -271,9 +271,11 @@ void free_pgd_range(struct mmu_gather ** }while (pgd++, addr = next, addr != end); } -void free_pgtables (struct mmu_gather **tlb,struct vm_area_struct *vma, -unsigned long floor,unsigned long ceiling) +void free_pgtables (struct vm_area_struct *vma,unsigned long floor, +unsigned long ceiling) { +struct mmu_gather *tlb; +while (vma) {struct vm_area_struct *next = vma->vm_next;unsigned long addr = vma->vm_start;
@@ -285,8 +287,10 @@ void free_pgtables(struct mmu_gather **tunlink_file_vma (vma);if (is_vm_hugetlb_page (vma)) { -hugetlb_free_pgd_range (tlb, addr, vma->vm_end, + tlb =tlb_gather_mmu (vma->vm_mm,0 ); +hugetlb_free_pgd_range (&tlb, addr, vma->vm_end, floor, next? next->vm_start: ceiling); +tlb_finish_mmu (tlb, addr, vma->vm_end); }else {/* * Optimization: gather nearby vmas into one call down
@@ -298,8 +302,10 @@ void free_pgtables(struct mmu_gather **tanon_vma_unlink (vma);unlink_file_vma (vma); } -free_pgd_range (tlb, addr, vma->vm_end, + tlb =tlb_gather_mmu (vma->vm_mm,0 ); +free_pgd_range (&tlb, addr, vma->vm_end, floor, next? next->vm_start: ceiling); +tlb_finish_mmu (tlb, addr, vma->vm_end); } vma = next; }
mm/mmap.c +3 -3
@@ -1750,9 +1750,9 @@ static void unmap_region(struct mm_strucupdate_hiwater_rss (mm);unmap_vmas (&tlb, vma, start, end, &nr_accounted,NULL );vm_unacct_memory (nr_accounted); -free_pgtables (&tlb, vma, prev? prev->vm_end:FIRST_USER_ADDRESS , - next? next->vm_start:0 );tlb_finish_mmu (tlb, start, end); +free_pgtables (vma, prev? prev->vm_end:FIRST_USER_ADDRESS , + next? next->vm_start:0 );emm_notify (mm, emm_invalidate_end, start, end); }
@@ -2049,8 +2049,8 @@ void exit_mmap(struct mm_struct *mm)/* Use -1 here to ensure all VMAs in the mm are unmapped */ end =unmap_vmas (&tlb, vma,0 ,-1 , &nr_accounted,NULL );vm_unacct_memory (nr_accounted); -free_pgtables (&tlb, vma,FIRST_USER_ADDRESS ,0 );tlb_finish_mmu (tlb,0 , end); +free_pgtables (vma,FIRST_USER_ADDRESS ,0 );/* * Walk the list again, actually closing and freeing it,
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
PATCH 4/10 emm: Convert i_mmap_lock to i_mmap_sem
In reply to Christoph LameterThe conversion to a rwsem allows callbacks during rmap traversal for files in a non atomic context. A rw style lock also allows concurrent walking of the reverse map. This is fairly straightforward if one removes pieces of the resched checking.
[Restarting unmapping is an issue to be discussed].
This slightly increases Aim9 performance results on an 8p.
--- arch/x86/mm/hugetlbpage.c | 4 ++-- fs/hugetlbfs/inode.c | 4 ++-- fs/inode.c | 2 +- include/linux/fs.h | 2 +- include/linux/mm.h | 2 +- kernel/fork.c | 4 ++-- mm/filemap.c | 8 ++++---- mm/filemap_xip.c | 4 ++-- mm/fremap.c | 4 ++-- mm/hugetlb.c | 10 +++++----- mm/memory.c | 29 +++++++++-------------------- mm/migrate.c | 4 ++-- mm/mmap.c | 43 ++++++++++++++++++++++--------------------- mm/mremap.c | 4 ++-- mm/rmap.c | 20 +++++++++----------- 15 files changed, 66 insertions(+), 78 deletions(-)
arch/x86/mm/hugetlbpage.c +2 -2
@@ -69,7 +69,7 @@ static void huge_pmd_share(struct mm_strif (!vma_shareable (vma, addr))return ; -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem); vma_prio_tree_foreach(svma, &iter, &mapping->i_mmap, idx, idx) {if (svma == vma)continue ;
@@ -94,7 +94,7 @@ static void huge_pmd_share(struct mm_strput_page (virt_to_page (spte));spin_unlock (&mm->page_table_lock); out: -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem); }/*
fs/hugetlbfs/inode.c +2 -2
@@ -454,10 +454,10 @@ static int hugetlb_vmtruncate(struct ino pgoff = offset >>PAGE_SHIFT ;i_size_write (inode, offset); -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem);if (!prio_tree_empty (&mapping->i_mmap))hugetlb_vmtruncate_list (&mapping->i_mmap, pgoff); -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem);truncate_hugepages (inode, offset);return 0 ; }
fs/inode.c +1 -1
@@ -210,7 +210,7 @@ void inode_init_once(struct inode *inodeINIT_LIST_HEAD (&inode->i_devices);INIT_RADIX_TREE (&inode->i_data.page_tree,GFP_ATOMIC );rwlock_init (&inode->i_data.tree_lock); -spin_lock_init (&inode->i_data.i_mmap_lock); +init_rwsem (&inode->i_data.i_mmap_sem);INIT_LIST_HEAD (&inode->i_data.private_list);spin_lock_init (&inode->i_data.private_lock);INIT_RAW_PRIO_TREE_ROOT (&inode->i_data.i_mmap);
include/linux/fs.h +1 -1
@@ -503,7 +503,7 @@ struct address_space {unsigned int i_mmap_writable;/* count VM_SHARED mappings */ struct prio_tree_root i_mmap;/* tree of private and shared mappings */ struct list_head i_mmap_nonlinear;/*list VM_NONLINEAR mappings */ -spinlock_t i_mmap_lock;/* protect tree, count, list */ +struct rw_semaphore i_mmap_sem;/* protect tree, count, list */ unsigned int truncate_count;/* Cover race condition with truncate */ unsigned long nrpages;/* number of total pages */ pgoff_t writeback_index;/* writeback starts here */
include/linux/mm.h +1 -1
@@ -716,7 +716,7 @@ struct zap_details {struct address_space *check_mapping;/* Check page->mapping if set */ pgoff_t first_index;/* Lowest page->index to unmap */ pgoff_t last_index;/* Highest page->index to unmap */ -spinlock_t *i_mmap_lock;/* For unmap_mapping_range: */ +struct rw_semaphore *i_mmap_sem;/* For unmap_mapping_range: */ unsigned long truncate_count;/* Compare vm_truncate_count */ };
kernel/fork.c +2 -2
@@ -273,12 +273,12 @@ static int dup_mmap(struct mm_struct *mmatomic_dec (&inode->i_writecount);/* insert tmp into the share list, just after mpnt */ -spin_lock (&file->f_mapping->i_mmap_lock); +down_write (&file->f_mapping->i_mmap_sem); tmp->vm_truncate_count = mpnt->vm_truncate_count;flush_dcache_mmap_lock (file->f_mapping);vma_prio_tree_add (tmp, mpnt);flush_dcache_mmap_unlock (file->f_mapping); -spin_unlock (&file->f_mapping->i_mmap_lock); +up_write (&file->f_mapping->i_mmap_sem); }/*
mm/filemap.c +4 -4
@@ -61,16 +61,16 @@ generic_file_direct_IO(int rw, struct ki/* * Lock ordering: * -* ->i_mmap_lock (vmtruncate) +* ->i_mmap_sem (vmtruncate) * ->private_lock (__free_pte->__set_page_dirty_buffers) * ->swap_lock (exclusive_swap_page, others) * ->mapping->tree_lock * * ->i_mutex -* ->i_mmap_lock (truncate->unmap_mapping_range) +* ->i_mmap_sem (truncate->unmap_mapping_range) * * ->mmap_sem -* ->i_mmap_lock +* ->i_mmap_sem * ->page_table_lock or pte_lock (various, mainly in memory.c) * ->mapping->tree_lock (arch-dependent flush_dcache_mmap_lock) *
@@ -87,7 +87,7 @@ generic_file_direct_IO(int rw, struct ki* ->sb_lock (fs/fs-writeback.c) * ->mapping->tree_lock (__sync_single_inode) * -* ->i_mmap_lock +* ->i_mmap_sem * ->anon_vma.lock (vma_adjust) * * ->anon_vma.lock
mm/filemap_xip.c +2 -2
@@ -184,7 +184,7 @@ __xip_unmap (struct address_space * mappif (!page)return ; -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem); vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) { mm = vma->vm_mm; address = vma->vm_start +
@@ -206,7 +206,7 @@ __xip_unmap (struct address_space * mappemm_notify (mm, emm_invalidate_end, address, address +PAGE_SIZE ); } -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem); }/*
mm/fremap.c +2 -2
@@ -205,13 +205,13 @@ asmlinkage long sys_remap_file_pages(uns } goto out; } -spin_lock (&mapping->i_mmap_lock); +down_write (&mapping->i_mmap_sem);flush_dcache_mmap_lock (mapping); vma->vm_flags |=VM_NONLINEAR ;vma_prio_tree_remove (vma, &mapping->i_mmap);vma_nonlinear_insert (vma, &mapping->i_mmap_nonlinear);flush_dcache_mmap_unlock (mapping); -spin_unlock (&mapping->i_mmap_lock); +up_write (&mapping->i_mmap_sem); }emm_notify (mm, emm_invalidate_start, start, end);
mm/hugetlb.c +5 -5
@@ -790,7 +790,7 @@ void __unmap_hugepage_range(struct vm_arstruct page *page;struct page *tmp;/* -* A page gathering list, protected by per file i_mmap_lock. The +* A page gathering list, protected by per file i_mmap_sem. The * lock is used to avoid list corruption from multiple unmapping * of the same page since we are using page->lru. */
@@ -840,9 +840,9 @@ void unmap_hugepage_range(struct vm_area* do nothing in this case. */ if (vma->vm_file) { -spin_lock (&vma->vm_file->f_mapping->i_mmap_lock); +down_write (&vma->vm_file->f_mapping->i_mmap_sem);__unmap_hugepage_range (vma, start, end); -spin_unlock (&vma->vm_file->f_mapping->i_mmap_lock); +up_write (&vma->vm_file->f_mapping->i_mmap_sem); } }
@@ -1085,7 +1085,7 @@ void hugetlb_change_protection(struct vmBUG_ON (address >= end);flush_cache_range (vma, address, end); -spin_lock (&vma->vm_file->f_mapping->i_mmap_lock); +down_write (&vma->vm_file->f_mapping->i_mmap_sem);spin_lock (&mm->page_table_lock);for (; address < end; address +=HPAGE_SIZE ) { ptep =huge_pte_offset (mm, address);
@@ -1100,7 +1100,7 @@ void hugetlb_change_protection(struct vm } }spin_unlock (&mm->page_table_lock); -spin_unlock (&vma->vm_file->f_mapping->i_mmap_lock); +up_write (&vma->vm_file->f_mapping->i_mmap_sem);flush_tlb_range (vma, start, end); }
mm/memory.c +9 -20
@@ -839,7 +839,6 @@ unsigned long unmap_vmas(struct mmu_gathunsigned long tlb_start =0 ;/* For tlb_finish_mmu */ int tlb_start_valid =0 ;unsigned long start = start_addr; -spinlock_t *i_mmap_lock = details? details->i_mmap_lock:NULL ;int fullmm = (*tlbp)->fullmm;for ( ; vma && vma->vm_start < end_addr; vma = vma->vm_next) {
@@ -876,22 +875,12 @@ unsigned long unmap_vmas(struct mmu_gath }tlb_finish_mmu (*tlbp, tlb_start, start); - -if (need_resched () || - (i_mmap_lock &&spin_needbreak (i_mmap_lock))) { -if (i_mmap_lock) { - *tlbp =NULL ; - goto out; - } -cond_resched (); - } - +cond_resched (); *tlbp =tlb_gather_mmu (vma->vm_mm, fullmm); tlb_start_valid =0 ; zap_work =ZAP_BLOCK_SIZE ; } } -out:return start;/* which is now the end (or restart) address */ }
@@ -1757,7 +1746,7 @@ unwritable_page:/* * Helper functions for unmap_mapping_range(). * -* __ Notes on dropping i_mmap_lock to reduce latency while unmapping __ +* __ Notes on dropping i_mmap_sem to reduce latency while unmapping __ * * We have to restart searching the prio_tree whenever we drop the lock, * since the iterator is only valid while the lock is held, and anyway
@@ -1776,7 +1765,7 @@ unwritable_page:* can't efficiently keep all vmas in step with mapping->truncate_count: * so instead reset them all whenever it wraps back to 0 (then go to 1). * mapping->truncate_count and vma->vm_truncate_count are protected by -* i_mmap_lock. +* i_mmap_sem. * * In order to make forward progress despite repeatedly restarting some * large vma, note the restart_addr from unmap_vmas when it breaks out:
@@ -1826,7 +1815,7 @@ again: restart_addr =zap_page_range (vma, start_addr, end_addr - start_addr, details); - need_break =need_resched () ||spin_needbreak (details->i_mmap_lock); + need_break =need_resched ();if (restart_addr >= end_addr) {/* We have now completed this vma: mark it so */
@@ -1840,9 +1829,9 @@ again: goto again; } -spin_unlock (details->i_mmap_lock); +up_write (details->i_mmap_sem);cond_resched (); -spin_lock (details->i_mmap_lock); +down_write (details->i_mmap_sem);return -EINTR ; }
@@ -1936,9 +1925,9 @@ void unmap_mapping_range(struct address_ details.last_index = hba + hlen -1 ;if (details.last_index < details.first_index) details.last_index =ULONG_MAX ; - details.i_mmap_lock = &mapping->i_mmap_lock; + details.i_mmap_sem = &mapping->i_mmap_sem; -spin_lock (&mapping->i_mmap_lock); +down_write (&mapping->i_mmap_sem);/* Protect against endless unmapping loops */ mapping->truncate_count++;
@@ -1953,7 +1942,7 @@ void unmap_mapping_range(struct address_unmap_mapping_range_tree (&mapping->i_mmap, &details);if (unlikely (!list_empty (&mapping->i_mmap_nonlinear)))unmap_mapping_range_list (&mapping->i_mmap_nonlinear, &details); -spin_unlock (&mapping->i_mmap_lock); +up_write (&mapping->i_mmap_sem); }EXPORT_SYMBOL (unmap_mapping_range);
mm/migrate.c +2 -2
@@ -211,12 +211,12 @@ static void remove_file_migration_ptes(sif (!mapping)return ; -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem); vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff)remove_migration_pte (vma, old, new); -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem); }/*
mm/mmap.c +22 -21
101 lines not on this page · raw message
mm/mremap.c +2 -2
@@ -86,7 +86,7 @@ static void move_ptes(struct vm_area_str* and we propagate stale pages into the dst afterward. */ mapping = vma->vm_file->f_mapping; -spin_lock (&mapping->i_mmap_lock); +down_write (&mapping->i_mmap_sem);if (new_vma->vm_truncate_count && new_vma->vm_truncate_count != vma->vm_truncate_count) new_vma->vm_truncate_count =0 ;
@@ -118,7 +118,7 @@ static void move_ptes(struct vm_area_strpte_unmap_nested (new_pte -1 );pte_unmap_unlock (old_pte -1 , old_ptl);if (mapping) -spin_unlock (&mapping->i_mmap_lock); +up_write (&mapping->i_mmap_sem);emm_notify (mm, emm_invalidate_end, old_start, old_end); }
mm/rmap.c +9 -11
@@ -24,7 +24,7 @@* inode->i_alloc_sem (vmtruncate_range) * mm->mmap_sem * page->flags PG_locked (lock_page) -* mapping->i_mmap_lock +* mapping->i_mmap_sem * anon_vma->lock * mm->page_table_lock or pte_lock * zone->lru_lock (in mark_page_accessed, isolate_lru_page)
@@ -450,14 +450,14 @@ static int page_referenced_file(struct p* The page lock not only makes sure that page->mapping cannot * suddenly be NULLified by truncation, it makes sure that the * structure at mapping cannot be freed and reused yet, -* so we can safely take mapping->i_mmap_lock. +* so we can safely take mapping->i_mmap_sem. */ BUG_ON (!PageLocked (page)); -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem);/* -* i_mmap_lock does not stabilize mapcount at all, but mapcount +* i_mmap_sem does not stabilize mapcount at all, but mapcount * is more likely to be accurate if we note it after spinning. */ mapcount =page_mapcount (page);
@@ -480,7 +480,7 @@ static int page_referenced_file(struct pbreak ; } -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem);return referenced; }
@@ -566,12 +566,12 @@ static int page_mkclean_file(struct addrBUG_ON (PageAnon (page)); -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem); vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) {if (vma->vm_flags &VM_SHARED ) ret +=page_mkclean_one (page, vma); } -spin_unlock (&mapping->i_mmap_lock); +up_read (&mapping->i_mmap_sem);return ret; }
@@ -1010,7 +1010,7 @@ static int try_to_unmap_file(struct pageunsigned long max_nl_size =0 ;unsigned int mapcount; -spin_lock (&mapping->i_mmap_lock); +down_read (&mapping->i_mmap_sem); vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) { ret =try_to_unmap_one (page, vma, migration);if (ret ==SWAP_FAIL || !page_mapped (page))
@@ -1047,7 +1047,6 @@ static int try_to_unmap_file(struct page mapcount =page_mapcount (page);if (!mapcount) goto out; -cond_resched_lock (&mapping->i_mmap_lock); max_nl_size = (max_nl_size +CLUSTER_SIZE -1 ) &CLUSTER_MASK ;if (max_nl_cursor ==0 )
@@ -1069,7 +1068,6 @@ static int try_to_unmap_file(struct page } vma->vm_private_data = (void *) max_nl_cursor; } -cond_resched_lock (&mapping->i_mmap_lock); max_nl_cursor +=CLUSTER_SIZE ; }while (max_nl_cursor <= max_nl_size);
@@ -1081,7 +1079,7 @@ static int try_to_unmap_file(struct pagelist_for_each_entry (vma, &mapping->i_mmap_nonlinear, shared.vm_set.list) vma->vm_private_data =NULL ; out: -spin_unlock (&mapping->i_mmap_lock); +up_write (&mapping->i_mmap_sem);return ret; }
PATCH 5/10 emm: Remove tlb pointer from the parameters of unmap vmas
In reply to Christoph LameterWe no longer abort unmapping in unmap vmas because we can reschedule while unmapping since we are holding a semaphore. This would allow moving more of the tlb flusing into unmap_vmas reducing code in various places.
--- include/linux/mm.h | 3 +-- mm/memory.c | 43 +++++++++++++++++-------------------------- mm/mmap.c | 18 +++--------------- 3 files changed, 21 insertions(+), 43 deletions(-)
include/linux/mm.h +1 -2
@@ -723,8 +723,7 @@ struct zap_details {struct page *vm_normal_page (struct vm_area_struct *,unsigned long ,pte_t );unsigned long zap_page_range (struct vm_area_struct *vma,unsigned long address,unsigned long size,struct zap_details *); -unsigned long unmap_vmas (struct mmu_gather **tlb, -struct vm_area_struct *start_vma,unsigned long start_addr, +unsigned long unmap_vmas (struct vm_area_struct *start_vma,unsigned long start_addr,unsigned long end_addr,unsigned long *nr_accounted,struct zap_details *);
mm/memory.c +17 -26
92 lines not on this page · raw message
mm/mmap.c +3 -15
@@ -1741,19 +1741,12 @@ static void unmap_region(struct mm_struc unsigned long start, unsigned long end) {struct vm_area_struct *next = prev? prev->vm_next: mm->mmap; -struct mmu_gather *tlb;unsigned long nr_accounted =0 ; -emm_notify (mm, emm_invalidate_start, start, end); -lru_add_drain (); - tlb =tlb_gather_mmu (mm,0 ); -update_hiwater_rss (mm); -unmap_vmas (&tlb, vma, start, end, &nr_accounted,NULL ); +unmap_vmas (vma, start, end, &nr_accounted,NULL );vm_unacct_memory (nr_accounted); -tlb_finish_mmu (tlb, start, end);free_pgtables (vma, prev? prev->vm_end:FIRST_USER_ADDRESS , next? next->vm_start:0 ); -emm_notify (mm, emm_invalidate_end, start, end); }/*
@@ -2033,7 +2026,6 @@ EXPORT_SYMBOL(do_brk);/* Release all mmaps. */ void exit_mmap (struct mm_struct *mm) { -struct mmu_gather *tlb;struct vm_area_struct *vma = mm->mmap;unsigned long nr_accounted =0 ;unsigned long end;
@@ -2041,15 +2033,11 @@ void exit_mmap(struct mm_struct *mm)/* mm's last user has gone, and its about to be pulled down */ arch_exit_mmap (mm);emm_notify (mm, emm_release,0 ,TASK_SIZE ); -lru_add_drain ();flush_cache_mm (mm); - tlb =tlb_gather_mmu (mm,1 ); -/* Don't update_hiwater_rss(mm) here, do_exit already did */ -/* Use -1 here to ensure all VMAs in the mm are unmapped */ - end =unmap_vmas (&tlb, vma,0 ,-1 , &nr_accounted,NULL ); + + end =unmap_vmas (vma,0 ,-1 , &nr_accounted,NULL );vm_unacct_memory (nr_accounted); -tlb_finish_mmu (tlb,0 , end);free_pgtables (vma,FIRST_USER_ADDRESS ,0 );/*
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
PATCH 6/10 emm: Convert anon_vma lock to rw_sem and refcount
In reply to Christoph LameterConvert the anon_vma spinlock to a rw semaphore. This allows concurrent traversal of reverse maps for try_to_unmap and page_mkclean. It also allows the calling of sleeping functions from reverse map traversal.
An additional complication is that rcu is used in some context to guarantee the presence of the anon_vma while we acquire the lock. We cannot take a semaphore within an rcu critical section. Add a refcount to the anon_vma structure which allow us to give an existence guarantee for the anon_vma structure independent of the spinlock or the list contents.
The refcount can then be taken within the RCU section. If it has been taken successfully then the refcount guarantees the existence of the anon_vma. The refcount in anon_vma also allows us to fix a nasty issue in page migration where we fudged by using rcu for a long code path to guarantee the existence of the anon_vma.
The refcount in general allows a shortening of RCU critical sections since we can do a rcu_unlock after taking the refcount. This is particularly relevant if the anon_vma chains contain hundreds of entries.
Issues:
- Atomic overhead increases in situations where a new reference to the anon_vma has to be established or removed. Overhead also increases when a speculative reference is used (try_to_unmap, page_mkclean, page migration). There is also the more frequent processor change due to up_xxx letting waiting tasks run first.
This results in f.e. the Aim9 brk performance test to got down by 10-15%.
--- include/linux/rmap.h | 20 ++++++++++++++++--- mm/migrate.c | 26 ++++++++++--------------- mm/mmap.c | 28 +++++++++++++------------- mm/rmap.c | 53 +++++++++++++++++++++++++++++---------------------- 4 files changed, 73 insertions(+), 54 deletions(-)
include/linux/rmap.h +17 -3
@@ -25,7 +25,8 @@* pointing to this anon_vma once its vma list is empty. */ struct anon_vma { -spinlock_t lock;/* Serialize access to vma list */ +atomic_t refcount;/* vmas on the list */ +struct rw_semaphore sem;/* Serialize access to vma list */ struct list_head head;/* List of private "related" vmas */ };
@@ -43,18 +44,31 @@ static inline void anon_vma_free(structkmem_cache_free (anon_vma_cachep, anon_vma); } +struct anon_vma *grab_anon_vma (struct page *page); + +static inline void get_anon_vma (struct anon_vma *anon_vma) +{ +atomic_inc (&anon_vma->refcount); +} + +static inline void put_anon_vma (struct anon_vma *anon_vma) +{ +if (atomic_dec_and_test (&anon_vma->refcount)) +anon_vma_free (anon_vma); +} +static inline void anon_vma_lock (struct vm_area_struct *vma) {struct anon_vma *anon_vma = vma->anon_vma;if (anon_vma) -spin_lock (&anon_vma->lock); +down_write (&anon_vma->sem); }static inline void anon_vma_unlock (struct vm_area_struct *vma) {struct anon_vma *anon_vma = vma->anon_vma;if (anon_vma) -spin_unlock (&anon_vma->lock); +up_write (&anon_vma->sem); }/*
mm/migrate.c +11 -15
@@ -235,15 +235,16 @@ static void remove_anon_migration_ptes(sreturn ;/* -* We hold the mmap_sem lock. So no need to call page_lock_anon_vma. +* We hold either the mmap_sem lock or a reference on the +* anon_vma. So no need to call page_lock_anon_vma. */ anon_vma = (struct anon_vma *) (mapping -PAGE_MAPPING_ANON ); -spin_lock (&anon_vma->lock); +down_read (&anon_vma->sem);list_for_each_entry (vma, &anon_vma->head, anon_vma_node)remove_migration_pte (vma, old, new); -spin_unlock (&anon_vma->lock); +up_read (&anon_vma->sem); }/*
@@ -623,7 +624,7 @@ static int unmap_and_move(new_page_t getint rc =0 ;int *result =NULL ;struct page *newpage =get_new_page (page, private, &result); -int rcu_locked =0 ; +struct anon_vma *anon_vma =NULL ;int charge =0 ;if (!newpage)
@@ -647,16 +648,14 @@ static int unmap_and_move(new_page_t get }/* * By try_to_unmap(), page->mapcount goes down to 0 here. In this case, -* we cannot notice that anon_vma is freed while we migrates a page. +* we cannot notice that anon_vma is freed while we migrate a page. * This rcu_read_lock() delays freeing anon_vma pointer until the end * of migration. File cache pages are no problem because of page_lock() * File Caches may use write_page() or lock_page() in migration, then, * just care Anon page here. */ -if (PageAnon (page)) { -rcu_read_lock (); - rcu_locked =1 ; - } +if (PageAnon (page)) + anon_vma =grab_anon_vma (page);/* * Corner case handling:
@@ -674,10 +673,7 @@ static int unmap_and_move(new_page_t getif (!PageAnon (page) &&PagePrivate (page)) {/* * Go direct to try_to_free_buffers() here because -* a) that's what try_to_release_page() would do anyway -* b) we may be under rcu_read_lock() here, so we can't -* use GFP_KERNEL which is what try_to_release_page() -* needs to be effective. +* that's what try_to_release_page() would do anyway */ try_to_free_buffers (page); }
@@ -698,8 +694,8 @@ static int unmap_and_move(new_page_t get }else if (charge)mem_cgroup_end_migration (newpage); rcu_unlock: -if (rcu_locked) -rcu_read_unlock (); +if (anon_vma) +put_anon_vma (anon_vma); unlock:
mm/rmap.c +31 -22
127 lines not on this page · raw message
mm/mmap.c +14 -14
@@ -564,7 +564,7 @@ again: remove_next = 1 + (end > next->if (vma->anon_vma) anon_vma = vma->anon_vma;if (anon_vma) { -spin_lock (&anon_vma->lock); +down_write (&anon_vma->sem);/* * Easily overlooked: when mprotect shifts the boundary, * make sure the expanding vma has anon_vma set if the
@@ -618,7 +618,7 @@ again: remove_next = 1 + (end > next-> }if (anon_vma) -spin_unlock (&anon_vma->lock); +up_write (&anon_vma->sem);if (mapping)up_write (&mapping->i_mmap_sem);
@@ -2238,7 +2238,7 @@ static void mm_lock_unlock(struct mm_str {struct vm_area_struct *vma;struct rw_semaphore *i_mmap_sem_last; -spinlock_t *anon_vma_lock_last; +struct rw_semaphore *anon_vma_sem_last; i_mmap_sem_last =NULL ;for (;;) {
@@ -2262,23 +2262,23 @@ static void mm_lock_unlock(struct mm_strup_write (i_mmap_sem); } - anon_vma_lock_last =NULL ; + anon_vma_sem_last =NULL ;for (;;) { -spinlock_t *anon_vma_lock = (spinlock_t *)-1UL ; +struct rw_semaphore *anon_vma_sem = (struct rw_semaphore *)-1UL ;for (vma = mm->mmap; vma; vma = vma->vm_next)if (vma->anon_vma && - (unsigned long ) anon_vma_lock > - (unsigned long ) &vma->anon_vma->lock && - (unsigned long ) &vma->anon_vma->lock > - (unsigned long ) anon_vma_lock_last) - anon_vma_lock = &vma->anon_vma->lock; -if (anon_vma_lock == (spinlock_t *)-1UL ) + (unsigned long ) anon_vma_sem > + (unsigned long ) &vma->anon_vma->sem && + (unsigned long ) &vma->anon_vma->sem > + (unsigned long ) anon_vma_sem_last) + anon_vma_sem = &vma->anon_vma->sem; +if (anon_vma_sem == (struct rw_semaphore *)-1UL )break ; - anon_vma_lock_last = anon_vma_lock; + anon_vma_sem_last = anon_vma_sem;if (lock) -spin_lock (anon_vma_lock); +down_write (anon_vma_sem);else -spin_unlock (anon_vma_lock); +up_write (anon_vma_sem); } }
PATCH 7/10 xpmem: This patch exports zap_page_range as it is needed by XPMEM.
In reply to Christoph LameterXPMEM would have used sys_madvise() except that madvise_dontneed() returns an -EINVAL if VM_PFNMAP is set, which is always true for the pages XPMEM imports from other partitions and is also true for uncached pages allocated locally via the mspec allocator. XPMEM needs zap_page_range() functionality for these types of pages as well as 'normal' pages.
--- mm/memory.c | 1 + 1 file changed, 1 insertion(+)
mm/memory.c +1 -0
@@ -901,6 +901,7 @@ unsigned long zap_page_range(struct vm_areturn unmap_vmas (vma, address, end, &nr_accounted, details); } +EXPORT_SYMBOL_GPL (zap_page_range);/* * Do a quick page-table lookup for a single page.
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
PATCH 8/10 xpmem: Locking rules for taking multiple mmap_sem locks.
In reply to Christoph LameterThis patch adds a lock ordering rule to avoid a potential deadlock when multiple mmap_sems need to be locked.
--- mm/filemap.c | 3 +++ 1 file changed, 3 insertions(+)
mm/filemap.c +3 -0
@@ -80,6 +80,9 @@ generic_file_direct_IO(int rw, struct ki* ->i_mutex (generic_file_buffered_write) * ->mmap_sem (fault_in_pages_readable->do_page_fault) * +* When taking multiple mmap_sems, one should lock the lowest-addressed +* one first proceeding on up to the highest-addressed one. +* * ->i_mutex * ->i_alloc_sem (various) *
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
PATCH 9/10 xpmem: The device driver
In reply to Christoph LameterXPmem device driver that allows sharing of address spaces across different instances of Linux. [Experimental, lots of issues still to be fixed].
drivers/misc/xp/Makefile new file +16 -0
@@ -0,0 +1,16 @@ +# drivers/misc/xp/Makefile +# +# This file is subject to the terms and conditions of the GNU General Public +# License. See the file "COPYING" in the main directory of this archive +# for more details. +# +# Copyright (C) 1999,2001-2008 Silicon Graphics, Inc. All Rights Reserved. +# + +# This is just temporary. Please do not comment. I am waiting for Dean +# Nelson's XPC patches to go in and will modify files introduced by his patches +# to enable. +obj-m += xpmem.o +xpmem-y := xpmem_main.o xpmem_make.o xpmem_get.o \ + xpmem_attach.o xpmem_pfn.o \ + xpmem_misc.o
drivers/misc/xp/xpmem_attach.c new file +824 -0
824 lines not on this page · raw message
drivers/misc/xp/xpmem_get.c new file +343 -0
343 lines not on this page · raw message
drivers/misc/xp/xpmem_main.c new file +440 -0
440 lines not on this page · raw message
drivers/misc/xp/xpmem_make.c new file +249 -0
249 lines not on this page · raw message
drivers/misc/xp/xpmem_misc.c new file +367 -0
367 lines not on this page · raw message
drivers/misc/xp/xpmem_pfn.c new file +1242 -0
761 lines not on this page · raw message
drivers/misc/xp/xpmem.h new file +130 -0
130 lines not on this page · raw message
drivers/misc/xp/xpmem_private.h new file +783 -0
783 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 5 lines
------------------------------------------------------------------------- This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Part of the diff is not shown here. View the raw message.
PATCH 10/10 xpmem: Simple example
In reply to Christoph LameterA simple test program (well actually a pair). They are fairly easy to use.
NOTE: the xpmem.h is copied from the kernel/drivers/misc/xp/xpmem.h file.
Type make. Then from one session, type ./A1. Grab the first line of output which should begin with ./A2 and paste the whole line into a second session. Paste as many times as you like. Each pass will increment the value one additional time. When you are tired, hit enter in the first window. You should see the same value printed from A1 as you most recently received from A2.
--- xpmem_test/A1.c | 64 +++++++++++++++++++++++++ xpmem_test/A2.c | 70 ++++++++++++++++++++++++++++ xpmem_test/Makefile | 14 +++++ xpmem_test/xpmem.h | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 278 insertions(+)
xpmem_test/A1.c new file +64 -0
@@ -0,0 +1,64 @@ +/* +* Simple test program. Makes a segment then waits for an input line +* and finally prints the value of the first integer of that segment. +*/ + +#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <stropts.h> +#include <sys/mman.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> + +#include "xpmem.h" + +int xpmem_fd; + +int +main (int argc,char **argv) +{ +char input[32 ]; +struct xpmem_cmd_make make_info; +int *data_block; +int ret; +__s64 segid; + + xpmem_fd =open ("/dev/xpmem" ,O_RDWR ); +if (xpmem_fd ==-1 ) { +perror ("Opening /dev/xpmem" ); +return -1 ; + } + + data_block =mmap (NULL ,getpagesize (),PROT_READ |PROT_WRITE , +MAP_SHARED |MAP_ANONYMOUS ,0 ,0 ); +if (data_block ==MAP_FAILED ) { +perror ("Creating mapping." ); +return -1 ; + } + data_block[0 ] =1 ; + + make_info.vaddr = (__u64 ) data_block; + make_info.size =getpagesize (); + make_info.permit_type =XPMEM_PERMIT_MODE ; + make_info.permit_value = (__u64 )0600 ; + ret =ioctl (xpmem_fd,XPMEM_CMD_MAKE , &make_info); +if (ret !=0 ) { +perror ("xpmem_make" ); +return -1 ; + } + + segid = make_info.segid; +printf ("./A2 %d %d %d %d\ndata_block[0] = %d\n" , + (int )(segid >>48 &0xffff ), (int )(segid >>32 &0xffff ), + (int )(segid >>16 &0xffff ), (int )(segid &0xffff ), + data_block[0 ]); +printf ("Waiting for input before exiting.\n" ); +fscanf (stdin,"%s" , input); + +printf ("data_block[0] = %d\n" , data_block[0 ]); + +return 0 ; +}
xpmem_test/A2.c new file +70 -0
70 lines not on this page · raw message
xpmem_test/Makefile new file +14 -0
@@ -0,0 +1,14 @@ + +default: A1 A2 + +A1: A1.c xpmem.h + gcc -Wall -o A1 A1.c + +A2: A2.c xpmem.h + gcc -Wall -o A2 A2.c + +indent: + indent -npro -kr -i8 -ts8 -sob -l80 -ss -ncs -cp1 -psl -npcs A1.c A2.c + +clean: + rm -f A1 A2 *~
xpmem_test/xpmem.h new file +130 -0
130 lines not on this page · raw message