Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932472AbbB0VCh (ORCPT ); Fri, 27 Feb 2015 16:02:37 -0500 Received: from mail-wg0-f47.google.com ([74.125.82.47]:36751 "EHLO mail-wg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753016AbbB0VCg (ORCPT ); Fri, 27 Feb 2015 16:02:36 -0500 Date: Fri, 27 Feb 2015 22:02:33 +0100 From: Michal Hocko To: "Wang, Yalin" Cc: "'Minchan Kim'" , Andrew Morton , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , Rik van Riel , Johannes Weiner , Mel Gorman , Shaohua Li Subject: Re: [RFC] mm: change mm_advise_free to clear page dirty Message-ID: <20150227210233.GA29002@dhcp22.suse.cz> References: <1424765897-27377-1-git-send-email-minchan@kernel.org> <20150224154318.GA14939@dhcp22.suse.cz> <20150225000809.GA6468@blaptop> <35FD53F367049845BC99AC72306C23D10458D6173BDC@CNBJMBX05.corpusers.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <35FD53F367049845BC99AC72306C23D10458D6173BDC@CNBJMBX05.corpusers.net> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2119 Lines: 71 On Fri 27-02-15 11:37:18, Wang, Yalin wrote: > This patch add ClearPageDirty() to clear AnonPage dirty flag, > the Anonpage mapcount must be 1, so that this page is only used by > the current process, not shared by other process like fork(). > if not clear page dirty for this anon page, the page will never be > treated as freeable. Very well spotted! I haven't noticed that during the review. > Signed-off-by: Yalin Wang > --- > mm/madvise.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 6d0fcb8..257925a 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -297,22 +297,17 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr, > continue; > > page = vm_normal_page(vma, addr, ptent); > - if (!page) > + if (!page || !PageAnon(page) || !trylock_page(page)) > continue; PageAnon check seems to be redundant because we are not allowing MADV_FREE on any !anon private mappings AFAIR. > > if (PageSwapCache(page)) { > - if (!trylock_page(page)) > + if (!try_to_free_swap(page)) > continue; You need to unlock the page here. > - > - if (!try_to_free_swap(page)) { > - unlock_page(page); > - continue; > - } > - > - ClearPageDirty(page); > - unlock_page(page); > } > > + if (page_mapcount(page) == 1) > + ClearPageDirty(page); Please add a comment about why we need to ClearPageDirty even !PageSwapCache. Anon pages are usually not marked dirty AFAIR. The reason seem to be racing try_to_free_swap which sets the page that way (although I do not seem to remember why are we doing that in the first place...) > + unlock_page(page); > /* > * Some of architecture(ex, PPC) don't update TLB > * with set_pte_at and tlb_remove_tlb_entry so for > -- > 2.2.2 -- Michal Hocko SUSE Labs -- 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/