From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757085Ab2IDPVU (ORCPT ); Tue, 4 Sep 2012 11:21:20 -0400 Received: from mga02.intel.com ([134.134.136.20]:21030 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754198Ab2IDPVT (ORCPT ); Tue, 4 Sep 2012 11:21:19 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.80,367,1344236400"; d="scan'208";a="196564792" Subject: Re: [RFC PATCH 2/2] mm: Batch page_check_references in shrink_page_list sharing the same i_mmap_mutex From: Tim Chen To: Matthew Wilcox Cc: Mel Gorman , Andrew Morton , Minchan Kim , Johannes Weiner , KAMEZAWA Hiroyuki , "Kirill A. Shutemov" , Andi Kleen , linux-mm@kvack.org, linux-kernel , Alex Shi In-Reply-To: <1345596500.13492.264.camel@schen9-DESK> References: <1345251998.13492.235.camel@schen9-DESK> <1345480982.13492.239.camel@schen9-DESK> <20120821132129.GC6960@linux.intel.com> <1345596500.13492.264.camel@schen9-DESK> Content-Type: text/plain; charset="UTF-8" Date: Tue, 04 Sep 2012 08:21:17 -0700 Message-ID: <1346772077.13492.267.camel@schen9-DESK> Mime-Version: 1.0 X-Mailer: Evolution 2.32.3 (2.32.3-1.fc14) Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2012-08-21 at 17:48 -0700, Tim Chen wrote: > > Thanks to Matthew's suggestions on improving the patch. Here's the > updated version. It seems to be sane when I booted my machine up. I > will put it through more testing when I get a chance. > > Tim > Matthew, The new patch seems to be causing some of the workloads with mmaped file read to seg fault. Will need to dig further to find out why. Tim > --- > Signed-off-by: Tim Chen > --- > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index fd07c45..f1320b1 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -156,8 +156,11 @@ static inline void page_dup_rmap(struct page *page) > /* > * Called from mm/vmscan.c to handle paging out > */ > -int page_referenced(struct page *, int is_locked, > - struct mem_cgroup *memcg, unsigned long *vm_flags); > +int needs_page_mmap_mutex(struct page *page); > +int page_referenced(struct page *, int is_locked, struct mem_cgroup *memcg, > + unsigned long *vm_flags); > +int __page_referenced(struct page *, int is_locked, struct mem_cgroup *memcg, > + unsigned long *vm_flags); > int page_referenced_one(struct page *, struct vm_area_struct *, > unsigned long address, unsigned int *mapcount, unsigned long *vm_flags); > > @@ -176,6 +179,7 @@ enum ttu_flags { > bool is_vma_temporary_stack(struct vm_area_struct *vma); > > int try_to_unmap(struct page *, enum ttu_flags flags); > +int __try_to_unmap(struct page *, enum ttu_flags flags); > int try_to_unmap_one(struct page *, struct vm_area_struct *, > unsigned long address, enum ttu_flags flags); > > diff --git a/mm/rmap.c b/mm/rmap.c > index 5b5ad58..ca8cd21 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -843,8 +843,7 @@ static int page_referenced_file(struct page *page, > * so we can safely take mapping->i_mmap_mutex. > */ > BUG_ON(!PageLocked(page)); > - > - mutex_lock(&mapping->i_mmap_mutex); > + BUG_ON(!mutex_is_locked(&mapping->i_mmap_mutex)); > > /* > * i_mmap_mutex does not stabilize mapcount at all, but mapcount > @@ -869,21 +868,16 @@ static int page_referenced_file(struct page *page, > break; > } > > - mutex_unlock(&mapping->i_mmap_mutex); > return referenced; > } > > -/** > - * page_referenced - test if the page was referenced > - * @page: the page to test > - * @is_locked: caller holds lock on the page > - * @memcg: target memory cgroup > - * @vm_flags: collect encountered vma->vm_flags who actually referenced the page > - * > - * Quick test_and_clear_referenced for all mappings to a page, > - * returns the number of ptes which referenced the page. > - */ > -int page_referenced(struct page *page, > +int needs_page_mmap_mutex(struct page *page) > +{ > + return page->mapping && page_mapped(page) && page_rmapping(page) && > + !PageKsm(page) && !PageAnon(page); > +} > + > +int __page_referenced(struct page *page, > int is_locked, > struct mem_cgroup *memcg, > unsigned long *vm_flags) > @@ -919,6 +913,32 @@ out: > return referenced; > } > > +/** > + * page_referenced - test if the page was referenced > + * @page: the page to test > + * @is_locked: caller holds lock on the page > + * @memcg: target memory cgroup > + * @vm_flags: collect encountered vma->vm_flags who actually referenced the page > + * > + * Quick test_and_clear_referenced for all mappings to a page, > + * returns the number of ptes which referenced the page. > + */ > +int page_referenced(struct page *page, > + int is_locked, > + struct mem_cgroup *memcg, > + unsigned long *vm_flags) > +{ > + int result, needs_lock; > + > + needs_lock = needs_page_mmap_mutex(page); > + if (needs_lock) > + mutex_lock(&page->mapping->i_mmap_mutex); > + result = __page_referenced(page, is_locked, memcg, vm_flags); > + if (needs_lock) > + mutex_unlock(&page->mapping->i_mmap_mutex); > + return result; > +} > + > static int page_mkclean_one(struct page *page, struct vm_area_struct *vma, > unsigned long address) > { > @@ -1560,7 +1580,7 @@ static int try_to_unmap_file(struct page *page, enum ttu_flags flags) > unsigned long max_nl_size = 0; > unsigned int mapcount; > > - mutex_lock(&mapping->i_mmap_mutex); > + BUG_ON(!mutex_is_locked(&mapping->i_mmap_mutex)); > vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) { > unsigned long address = vma_address(page, vma); > if (address == -EFAULT) > @@ -1640,7 +1660,24 @@ static int try_to_unmap_file(struct page *page, enum ttu_flags flags) > list_for_each_entry(vma, &mapping->i_mmap_nonlinear, shared.vm_set.list) > vma->vm_private_data = NULL; > out: > - mutex_unlock(&mapping->i_mmap_mutex); > + return ret; > +} > + > +int __try_to_unmap(struct page *page, enum ttu_flags flags) > +{ > + int ret; > + > + BUG_ON(!PageLocked(page)); > + VM_BUG_ON(!PageHuge(page) && PageTransHuge(page)); > + > + if (unlikely(PageKsm(page))) > + ret = try_to_unmap_ksm(page, flags); > + else if (PageAnon(page)) > + ret = try_to_unmap_anon(page, flags); > + else > + ret = try_to_unmap_file(page, flags); > + if (ret != SWAP_MLOCK && !page_mapped(page)) > + ret = SWAP_SUCCESS; > return ret; > } > > @@ -1660,20 +1697,27 @@ out: > */ > int try_to_unmap(struct page *page, enum ttu_flags flags) > { > - int ret; > + int result, needs_lock; > + > + needs_lock = needs_page_mmap_mutex(page); > + if (needs_lock) > + mutex_lock(&page->mapping->i_mmap_mutex); > + result = __try_to_unmap(page, flags); > + if (needs_lock) > + mutex_unlock(&page->mapping->i_mmap_mutex); > + return result; > +} > > - BUG_ON(!PageLocked(page)); > - VM_BUG_ON(!PageHuge(page) && PageTransHuge(page)); > +int __try_to_munlock(struct page *page) > +{ > + VM_BUG_ON(!PageLocked(page) || PageLRU(page)); > > if (unlikely(PageKsm(page))) > - ret = try_to_unmap_ksm(page, flags); > + return try_to_unmap_ksm(page, TTU_MUNLOCK); > else if (PageAnon(page)) > - ret = try_to_unmap_anon(page, flags); > + return try_to_unmap_anon(page, TTU_MUNLOCK); > else > - ret = try_to_unmap_file(page, flags); > - if (ret != SWAP_MLOCK && !page_mapped(page)) > - ret = SWAP_SUCCESS; > - return ret; > + return try_to_unmap_file(page, TTU_MUNLOCK); > } > > /** > @@ -1693,14 +1737,15 @@ int try_to_unmap(struct page *page, enum ttu_flags flags) > */ > int try_to_munlock(struct page *page) > { > - VM_BUG_ON(!PageLocked(page) || PageLRU(page)); > - > - if (unlikely(PageKsm(page))) > - return try_to_unmap_ksm(page, TTU_MUNLOCK); > - else if (PageAnon(page)) > - return try_to_unmap_anon(page, TTU_MUNLOCK); > - else > - return try_to_unmap_file(page, TTU_MUNLOCK); > + int result, needs_lock; > + > + needs_lock = needs_page_mmap_mutex(page); > + if (needs_lock) > + mutex_lock(&page->mapping->i_mmap_mutex); > + result = __try_to_munlock(page); > + if (needs_lock) > + mutex_unlock(&page->mapping->i_mmap_mutex); > + return result; > } > > void __put_anon_vma(struct anon_vma *anon_vma) > diff --git a/mm/vmscan.c b/mm/vmscan.c > index d4ab646..74a5fd0 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -27,6 +27,7 @@ > buffer_heads_over_limit */ > #include > #include > +#include > #include > #include > #include > @@ -786,7 +787,7 @@ static enum page_references page_check_references(struct page *page, > int referenced_ptes, referenced_page; > unsigned long vm_flags; > > - referenced_ptes = page_referenced(page, 1, mz->mem_cgroup, &vm_flags); > + referenced_ptes = __page_referenced(page, 1, mz->mem_cgroup, &vm_flags); > referenced_page = TestClearPageReferenced(page); > > /* Lumpy reclaim - ignore references */ > @@ -856,6 +857,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, > unsigned long nr_congested = 0; > unsigned long nr_reclaimed = 0; > unsigned long nr_writeback = 0; > + struct mutex *i_mmap_mutex = NULL; > > cond_resched(); > > @@ -909,7 +911,15 @@ static unsigned long shrink_page_list(struct list_head *page_list, > } > } > > + if (needs_page_mmap_mutex(page) && > + i_mmap_mutex != &page->mapping->i_mmap_mutex) { > + if (i_mmap_mutex) > + mutex_unlock(i_mmap_mutex); > + i_mmap_mutex = &page->mapping->i_mmap_mutex; > + mutex_lock(i_mmap_mutex); > + } > references = page_check_references(page, mz, sc); > + > switch (references) { > case PAGEREF_ACTIVATE: > goto activate_locked; > @@ -939,7 +949,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, > * processes. Try to unmap it here. > */ > if (page_mapped(page) && mapping) { > - switch (try_to_unmap(page, TTU_UNMAP)) { > + switch (__try_to_unmap(page, TTU_UNMAP)) { > case SWAP_FAIL: > goto activate_locked; > case SWAP_AGAIN: > @@ -1090,6 +1100,8 @@ keep_lumpy: > VM_BUG_ON(PageLRU(page) || PageUnevictable(page)); > } > > + if (i_mmap_mutex) > + mutex_unlock(i_mmap_mutex); > nr_reclaimed += __remove_mapping_batch(&unmap_pages, &ret_pages, > &free_pages); > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/