From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752151AbcFTDMR (ORCPT ); Sun, 19 Jun 2016 23:12:17 -0400 Received: from out4133-18.mail.aliyun.com ([42.120.133.18]:25130 "EHLO out4133-18.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751579AbcFTDMI (ORCPT ); Sun, 19 Jun 2016 23:12:08 -0400 X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R131e4;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e02c03284;MF=hillf.zj@alibaba-inc.com;NM=1;PH=DS;RN=5;SR=0;TI=SMTPD_----4wcn.fY_1466391085; Reply-To: "Hillf Danton" From: "Hillf Danton" To: "'Ebru Akagunduz'" , "'Kirill A. Shutemov'" Cc: , , "Andrew Morton" References: <04f701d1c797$1ebe6b80$5c3b4280$@alibaba-inc.com> <04f801d1c79b$b46744a0$1d35cde0$@alibaba-inc.com> <20160616100854.GB18137@node.shutemov.name> <20160618190951.GA11151@debian> In-Reply-To: <20160618190951.GA11151@debian> Subject: Re: [PATCHv9-rebased2 01/37] mm, thp: make swapin readahead under down_read of mmap_sem Date: Mon, 20 Jun 2016 10:51:25 +0800 Message-ID: <05f001d1ca9e$a3ac5a00$eb050e00$@alibaba-inc.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQGJxWVrkyqmG9ZdjrZwyGwwgyB3NwIEF0hjAk8pzvwCVSzj8qBMa/xw Content-Language: zh-cn Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > > > @@ -2401,11 +2430,18 @@ static void __collapse_huge_page_swapin(struct mm_struct *mm, > > > > continue; > > > > swapped_in++; > > > > ret = do_swap_page(mm, vma, _address, pte, pmd, > > > > - FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_RETRY_NOWAIT, > > > > + FAULT_FLAG_ALLOW_RETRY, > > > > > > Add a description in change log for it please. > > > > Ebru, would you address it? > > > This changelog really seems poor. > Is there a way to update only changelog of the commit? > I tried to use git rebase to amend commit, however > I could not rebase. This patch only needs better changelog. > > I would like to update it as follows, if you would like to too: > > " > Currently khugepaged makes swapin readahead under down_write. This patch > supplies to make swapin readahead under down_read instead of down_write. > > Along swapin, we can need to drop and re-take mmap_sem. Therefore we > have to be sure vma is consistent. This patch adds a helper function > to validate vma and also supplies that async swapin should not be > performed without waiting. > > The patch was tested with a test program that allocates 800MB of memory, > writes to it, and then sleeps. The system was forced to swap out all. > Afterwards, the test program touches the area by writing, it skips a page > in each 20 pages of the area. > " > I like to ask again, why is FAULT_FLAG_RETRY_NOWAIT dropped? > Could you please suggest me a way to replace above changelog with the old? > We can ask Andrew for some advices. > > > > > > They are cleaned up in subsequent darns? > > > Yes, that is reported and fixed here: > http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=fc7038a69cee6b817261f7cd805e9663fdc1075c > > However, the above comment inconsistency still there. > I've added a fix patch: > > From 404438ff1b0617cbf7434cba0c5a08f79ccb8a5d Mon Sep 17 00:00:00 2001 > From: Ebru Akagunduz > Date: Sat, 18 Jun 2016 21:07:22 +0300 > Subject: [PATCH] mm, thp: fix comment inconsistency for swapin readahead > functions > Fill in change log please. thanks Hillf > Signed-off-by: Ebru Akagunduz > --- > mm/huge_memory.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > index acd374e..f0d528e 100644 > --- a/mm/huge_memory.c > +++ b/mm/huge_memory.c > @@ -2436,9 +2436,10 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm, > /* do_swap_page returns VM_FAULT_RETRY with released mmap_sem */ > if (ret & VM_FAULT_RETRY) { > down_read(&mm->mmap_sem); > - /* vma is no longer available, don't continue to swapin */ > - if (hugepage_vma_revalidate(mm, address)) > + if (hugepage_vma_revalidate(mm, address)) { > + /* vma is no longer available, don't continue to swapin */ > return false; > + } > } > if (ret & VM_FAULT_ERROR) { > trace_mm_collapse_huge_page_swapin(mm, swapped_in, 0); > @@ -2513,8 +2514,8 @@ static void collapse_huge_page(struct mm_struct *mm, > if (allocstall == curr_allocstall && swap != 0) { > /* > * __collapse_huge_page_swapin always returns with mmap_sem > - * locked. If it fails, release mmap_sem and jump directly > - * out. Continuing to collapse causes inconsistency. > + * locked. If it fails, we release mmap_sem and jump out_nolock. > + * Continuing to collapse causes inconsistency. > */ > if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) { > mem_cgroup_cancel_charge(new_page, memcg, true); > -- > 1.9.1