From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-f70.google.com (mail-it0-f70.google.com [209.85.214.70]) by kanga.kvack.org (Postfix) with ESMTP id BA5296B0005 for ; Fri, 17 Jun 2016 01:31:10 -0400 (EDT) Received: by mail-it0-f70.google.com with SMTP id e5so153713915ith.0 for ; Thu, 16 Jun 2016 22:31:10 -0700 (PDT) Received: from lgeamrelo13.lge.com (LGEAMRELO13.lge.com. [156.147.23.53]) by mx.google.com with ESMTP id 199si2360174itx.64.2016.06.16.22.31.09 for ; Thu, 16 Jun 2016 22:31:09 -0700 (PDT) Date: Fri, 17 Jun 2016 14:31:02 +0900 From: Minchan Kim Subject: Re: [PATCH] MADVISE_FREE, THP: Fix madvise_free_huge_pmd return value after splitting Message-ID: <20160617053102.GA2374@bbox> References: <1466132640-18932-1-git-send-email-ying.huang@intel.com> MIME-Version: 1.0 In-Reply-To: <1466132640-18932-1-git-send-email-ying.huang@intel.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-ID: To: "Huang, Ying" Cc: Andrew Morton , "Kirill A. Shutemov" , Vlastimil Babka , Jerome Marchand , Andrea Arcangeli , Ebru Akagunduz , linux-mm@kvack.org, linux-kernel@vger.kernel.org Hi, On Thu, Jun 16, 2016 at 08:03:54PM -0700, Huang, Ying wrote: > From: Huang Ying > > madvise_free_huge_pmd should return 0 if the fallback PTE operations are > required. In madvise_free_huge_pmd, if part pages of THP are discarded, > the THP will be split and fallback PTE operations should be used if > splitting succeeds. But the original code will make fallback PTE > operations skipped, after splitting succeeds. Fix that via make > madvise_free_huge_pmd return 0 after splitting successfully, so that the > fallback PTE operations will be done. You're right. Thanks! > > Know issues: if my understanding were correct, return 1 from > madvise_free_huge_pmd means the following processing for the PMD should > be skipped, while return 0 means the following processing is still > needed. So the function should return 0 only if the THP is split > successfully or the PMD is not trans huge. But the pmd_trans_unstable > after madvise_free_huge_pmd guarantee the following processing will be > skipped for huge PMD. So current code can run properly. But if my > understanding were correct, we can clean up return code of > madvise_free_huge_pmd accordingly. I like your clean up. Just a minor comment below. > > Signed-off-by: "Huang, Ying" > --- > mm/huge_memory.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index 2ad52d5..64dc95d 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c First of all, let's change ret from int to bool. And then, add description in the function entry. /* * Return true if we do MADV_FREE successfully on entire pmd page. * Otherwise, return false. */ And do not set to 1 if it is huge_zero_pmd but just goto out to return false. Thanks! > @@ -1655,14 +1655,9 @@ int madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma, > if (next - addr != HPAGE_PMD_SIZE) { > get_page(page); > spin_unlock(ptl); > - if (split_huge_page(page)) { > - put_page(page); > - unlock_page(page); > - goto out_unlocked; > - } > + split_huge_page(page); > put_page(page); > unlock_page(page); > - ret = 1; > goto out_unlocked; > } > > -- > 2.8.1 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: email@kvack.org -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org