Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756039Ab3FQHYt (ORCPT ); Mon, 17 Jun 2013 03:24:49 -0400 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:49492 "EHLO LGEMRELSE6Q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754191Ab3FQHYs (ORCPT ); Mon, 17 Jun 2013 03:24:48 -0400 X-AuditID: 9c930179-b7c30ae0000047e1-1a-51beb9beb54f Date: Mon, 17 Jun 2013 16:24:46 +0900 From: Minchan Kim To: John Stultz Cc: LKML , Andrew Morton , Android Kernel Team , Robert Love , Mel Gorman , Hugh Dickins , Dave Hansen , Rik van Riel , Dmitry Adamushko , Dave Chinner , Neil Brown , Andrea Righi , Andrea Arcangeli , "Aneesh Kumar K.V" , Mike Hommey , Taras Glek , Dhaval Giani , Jan Kara , KOSAKI Motohiro , Michel Lespinasse , "linux-mm@kvack.org" Subject: Re: [PATCH 7/8] vrange: Add method to purge volatile ranges Message-ID: <20130617072446.GB3251@bbox> References: <1371010971-15647-1-git-send-email-john.stultz@linaro.org> <1371010971-15647-8-git-send-email-john.stultz@linaro.org> <20130617071331.GA3251@bbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130617071331.GA3251@bbox> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: AAAAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4920 Lines: 130 On Mon, Jun 17, 2013 at 04:13:31PM +0900, Minchan Kim wrote: > Hello John, > > I am rewriting purging path and found a bug from this patch. > I might forget it so I will send this comment for recording. > > On Tue, Jun 11, 2013 at 09:22:50PM -0700, John Stultz wrote: > > From: Minchan Kim > > > > This patch adds discarding function to purge volatile ranges under > > memory pressure. Logic is as following: > > > > 1. Memory pressure happens > > 2. VM start to reclaim pages > > 3. Check the page is in volatile range. > > 4. If so, zap the page from the process's page table. > > (By semantic vrange(2), we should mark it with another one to > > make page fault when you try to access the address. It will > > be introduced later patch) > > 5. If page is unmapped from all processes, discard it instead of swapping. > > > > This patch does not address the case where there is no swap, which > > keeps anonymous pages from being aged off the LRUs. Minchan has > > additional patches that add support for purging anonymous pages > > > > XXX: First pass at file purging. Seems to work, but is likely broken > > and needs close review. > > > > Cc: Andrew Morton > > Cc: Android Kernel Team > > Cc: Robert Love > > Cc: Mel Gorman > > Cc: Hugh Dickins > > Cc: Dave Hansen > > Cc: Rik van Riel > > Cc: Dmitry Adamushko > > Cc: Dave Chinner > > Cc: Neil Brown > > Cc: Andrea Righi > > Cc: Andrea Arcangeli > > Cc: Aneesh Kumar K.V > > Cc: Mike Hommey > > Cc: Taras Glek > > Cc: Dhaval Giani > > Cc: Jan Kara > > Cc: KOSAKI Motohiro > > Cc: Michel Lespinasse > > Cc: Minchan Kim > > Cc: linux-mm@kvack.org > > Signed-off-by: Minchan Kim > > [jstultz: Reworked to add purging of file pages, commit log tweaks] > > Signed-off-by: John Stultz > > --- > > include/linux/rmap.h | 12 +- > > include/linux/swap.h | 1 + > > include/linux/vrange.h | 7 ++ > > mm/ksm.c | 2 +- > > mm/rmap.c | 30 +++-- > > mm/swapfile.c | 36 ++++++ > > mm/vmscan.c | 16 ++- > > mm/vrange.c | 332 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 8 files changed, 420 insertions(+), 16 deletions(-) > > > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > > index 6dacb93..6432dfb 100644 > > --- a/include/linux/rmap.h > > +++ b/include/linux/rmap.h > > @@ -83,6 +83,8 @@ enum ttu_flags { > > }; > > > > < snip > > > > @@ -662,7 +663,7 @@ int page_mapped_in_vma(struct page *page, struct vm_area_struct *vma) > > */ > > int page_referenced_one(struct page *page, struct vm_area_struct *vma, > > unsigned long address, unsigned int *mapcount, > > - unsigned long *vm_flags) > > + unsigned long *vm_flags, int *is_vrange) > > { > > struct mm_struct *mm = vma->vm_mm; > > int referenced = 0; > > @@ -724,6 +725,9 @@ int page_referenced_one(struct page *page, struct vm_area_struct *vma, > > referenced++; > > } > > pte_unmap_unlock(pte, ptl); > > + if (is_vrange && > > + vrange_address(mm, address, address + PAGE_SIZE - 1)) > > + *is_vrange = 1; > > < snip > > > > +static bool __vrange_address(struct vrange_root *vroot, > > + unsigned long start, unsigned long end) > > +{ > > + struct interval_tree_node *node; > > + > > + node = interval_tree_iter_first(&vroot->v_rb, start, end); > > + return node ? true : false; > > +} > > + > > +bool vrange_address(struct mm_struct *mm, > > + unsigned long start, unsigned long end) > > +{ > > + struct vrange_root *vroot; > > + unsigned long vstart_idx, vend_idx; > > + struct vm_area_struct *vma; > > + bool ret; > > + > > + vma = find_vma(mm, start); > > It seems to be tweaked by you while you are refactoring with file-vrange > The problem of the code is that you couldn't use vma without holding > the lock of mmap_sem and you couldn't use the lock in purging path > because you couldn't know other tasks's state so it might be a dealock > if you try to hold a lock. It was sent by mistake with not completing. :( Exactly speaking, the find_vma is a problem and we don't need it. Couldn't we pass just vma, NOT mm? Have you seen any problem? -- Kind regards, Minchan Kim -- 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/