From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7CF2C43381 for ; Fri, 22 Feb 2019 04:41:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A9EC12086C for ; Fri, 22 Feb 2019 04:41:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726684AbfBVElW (ORCPT ); Thu, 21 Feb 2019 23:41:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49010 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726247AbfBVElW (ORCPT ); Thu, 21 Feb 2019 23:41:22 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 40FDD81F11; Fri, 22 Feb 2019 04:41:21 +0000 (UTC) Received: from xz-x1 (ovpn-12-57.pek2.redhat.com [10.72.12.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4704567676; Fri, 22 Feb 2019 04:41:10 +0000 (UTC) Date: Fri, 22 Feb 2019 12:41:05 +0800 From: Peter Xu To: Jerome Glisse Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, David Hildenbrand , Hugh Dickins , Maya Gokhale , Pavel Emelyanov , Johannes Weiner , Martin Cracauer , Shaohua Li , Marty McFadden , Andrea Arcangeli , Mike Kravetz , Denis Plotnikov , Mike Rapoport , Mel Gorman , "Kirill A . Shutemov" , "Dr . David Alan Gilbert" Subject: Re: [PATCH v2 05/26] mm: gup: allow VM_FAULT_RETRY for multiple times Message-ID: <20190222044105.GE8904@xz-x1> References: <20190212025632.28946-1-peterx@redhat.com> <20190212025632.28946-6-peterx@redhat.com> <20190221160612.GE2813@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190221160612.GE2813@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 22 Feb 2019 04:41:21 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 21, 2019 at 11:06:55AM -0500, Jerome Glisse wrote: > On Tue, Feb 12, 2019 at 10:56:11AM +0800, Peter Xu wrote: > > This is the gup counterpart of the change that allows the VM_FAULT_RETRY > > to happen for more than once. > > > > Signed-off-by: Peter Xu > > Reviewed-by: Jérôme Glisse Thanks for the r-b, Jerome! Though I plan to change this patch a bit because I just noticed that I didn't touch up the hugetlbfs path for GUP. Though it was not needed for now because hugetlbfs is not yet supported but I think maybe I'd better do that as well in this same patch to make follow up works easier on hugetlb, and the patch will be more self contained. The new version will simply squash below change into current patch: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index e3c738bde72e..a8eace2d5296 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -4257,8 +4257,10 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT; if (flags & FOLL_TRIED) { - VM_WARN_ON_ONCE(fault_flags & - FAULT_FLAG_ALLOW_RETRY); + /* + * Note: FAULT_FLAG_ALLOW_RETRY and + * FAULT_FLAG_TRIED can co-exist + */ fault_flags |= FAULT_FLAG_TRIED; } ret = hugetlb_fault(mm, vma, vaddr, fault_flags); I'd say this change is straightforward (it's the same as the faultin_page below but just for hugetlbfs). Please let me know if you still want to offer the r-b with above change squashed (I'll be more than glad to take it!), or I'll just wait for your review comment when I post the next version. Thanks, > > > --- > > mm/gup.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index fa75a03204c1..ba387aec0d80 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -528,7 +528,10 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, > > if (*flags & FOLL_NOWAIT) > > fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT; > > if (*flags & FOLL_TRIED) { > > - VM_WARN_ON_ONCE(fault_flags & FAULT_FLAG_ALLOW_RETRY); > > + /* > > + * Note: FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_TRIED > > + * can co-exist > > + */ > > fault_flags |= FAULT_FLAG_TRIED; > > } > > > > @@ -943,17 +946,23 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk, > > /* VM_FAULT_RETRY triggered, so seek to the faulting offset */ > > pages += ret; > > start += ret << PAGE_SHIFT; > > + lock_dropped = true; > > > > +retry: > > /* > > * Repeat on the address that fired VM_FAULT_RETRY > > - * without FAULT_FLAG_ALLOW_RETRY but with > > + * with both FAULT_FLAG_ALLOW_RETRY and > > * FAULT_FLAG_TRIED. > > */ > > *locked = 1; > > - lock_dropped = true; > > down_read(&mm->mmap_sem); > > ret = __get_user_pages(tsk, mm, start, 1, flags | FOLL_TRIED, > > - pages, NULL, NULL); > > + pages, NULL, locked); > > + if (!*locked) { > > + /* Continue to retry until we succeeded */ > > + BUG_ON(ret != 0); > > + goto retry; > > + } > > if (ret != 1) { > > BUG_ON(ret > 1); > > if (!pages_done) > > -- > > 2.17.1 > > -- Peter Xu