Re: [RFC 3/4] lockless vma caching

Minchan Kim
2009-12-19T03:43:40Z

KAMEZAWA Hiroyuki wrote:
> For accessing vma in lockless style, some modification for vma lookup is
> required. Now, rb-tree is used and it doesn't allow read while modification.
>
> This is a trial to caching vma rather than diving into rb-tree. The last
> fault vma is cached to pgd's page->cached_vma field. And, add reference count
> and waitqueue to vma.
>
> The accessor will have to do
>
> vma = lookup_vma_cache(mm, address);
> if (vma) {
> if (mm_check_version(mm) && /* no write lock at this point ? */
> (vma->vm_start <= address) && (vma->vm_end > address))
> goto found_vma; /* start speculative job */
> else
> vma_release_cache(vma);
> vma = NULL;
> }
> vma = find_vma();
> found_vma:
> ....do some jobs....
> vma_release_cache(vma);
>
> Maybe some more consideration for invalidation point is necessary.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> include/linux/mm.h | 20 +++++++++
> include/linux/mm_types.h | 5 ++
> mm/memory.c | 14 ++++++
> mm/mmap.c | 102 +++++++++++++++++++++++++++++++++++++++++++++--
> mm/page_alloc.c | 1
> 5 files changed, 138 insertions(+), 4 deletions(-)
>
> Index: mmotm-mm-accessor/include/linux/mm.h
> ===================================================================
> --- mmotm-mm-accessor.orig/include/linux/mm.h
> +++ mmotm-mm-accessor/include/linux/mm.h
> @@ -763,6 +763,26 @@ unsigned long unmap_vmas(struct mmu_gath
> unsigned long end_addr, unsigned long *nr_accounted,
> struct zap_details *);
>
> +struct vm_area_struct *lookup_vma_cache(struct mm_struct *mm,
> + unsigned long address);
> +void invalidate_vma_cache(struct mm_struct *mm,
> + struct vm_area_struct *vma);
> +void wait_vmas_cache_range(struct vm_area_struct *vma, unsigned long end);
> +
> +static inline void vma_hold(struct vm_area_struct *vma)
Nitpick:
How about static inline void vma_cache_[get/put] naming?

> +{
> + atomic_inc(&vma->cache_access);
> +}
> +
> +void __vma_release(struct vm_area_struct *vma);
> +static inline void vma_release(struct vm_area_struct *vma)
> +{
> + if (atomic_dec_and_test(&vma->cache_access)) {
> + if (waitqueue_active(&vma->cache_wait))
> + __vma_release(vma);
> + }
> +}
> +
> /**
> * mm_walk - callbacks for walk_page_range
> * @pgd_entry: if set, called for each non-empty PGD (top-level) entry
> Index: mmotm-mm-accessor/include/linux/mm_types.h
> ===================================================================
> --- mmotm-mm-accessor.orig/include/linux/mm_types.h
> +++ mmotm-mm-accessor/include/linux/mm_types.h
> @@ -12,6 +12,7 @@
> #include <linux/completion.h>
> #include <linux/cpumask.h>
> #include <linux/page-debug-flags.h>
> +#include <linux/wait.h>
> #include <asm/page.h>
> #include <asm/mmu.h>
>
> @@ -77,6 +78,7 @@ struct page {
> union {
> pgoff_t index; /* Our offset within mapping. */
> void *freelist; /* SLUB: freelist req. slab lock */
> + void *cache;

Let's add annotation "/* vm_area_struct cache when the page is used as page table */".

> };
> struct list_head lru; /* Pageout list, eg. active_list
> * protected by zone->lru_lock !
> @@ -180,6 +182,9 @@ struct vm_area_struct {
> void * vm_private_data; /* was vm_pte (shared mem) */
> unsigned long vm_truncate_count;/* truncate_count or restart_addr */
>
> + atomic_t cache_access;
> + wait_queue_head_t cache_wait;
> +
> #ifndef CONFIG_MMU
> struct vm_region *vm_region; /* NOMMU mapping region */
> #endif
> Index: mmotm-mm-accessor/mm/memory.c
> ===================================================================
> --- mmotm-mm-accessor.orig/mm/memory.c
> +++ mmotm-mm-accessor/mm/memory.c
> @@ -145,6 +145,14 @@ void pmd_clear_bad(pmd_t *pmd)
> pmd_clear(pmd);
> }
>

Let's put the note here. "The caller needs to hold the pte lock"

> +static void update_vma_cache(pmd_t *pmd, struct vm_area_struct *vma)
> +{
> + struct page *page;
> + /* ptelock is held */
> + page = pmd_page(*pmd);
> + page->cache = vma;
> +}
> +
> /*
> * Note: this doesn't free the actual pages themselves. That
> * has been handled earlier when unmapping all the memory regions.
> @@ -2118,6 +2126,7 @@ reuse:
> if (ptep_set_access_flags(vma, address, page_table, entry,1))
> update_mmu_cache(vma, address, entry);
> ret |= VM_FAULT_WRITE;
> + update_vma_cache(pmd, vma);
> goto unlock;
> }
>
..
<snip>
..

> Index: mmotm-mm-accessor/mm/page_alloc.c
> ===================================================================
> --- mmotm-mm-accessor.orig/mm/page_alloc.c
> +++ mmotm-mm-accessor/mm/page_alloc.c
> @@ -698,6 +698,7 @@ static int prep_new_page(struct page *pa
>
> set_page_private(page, 0);
> set_page_refcounted(page);
> + page->cache = NULL;

Is here is proper place to initialize page->cache?
It cause unnecessary overhead about not pmd page.

How about pmd_alloc?

>
> arch_alloc_page(page, order);
> kernel_map_pages(page, 1 << order, 1);
>