From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=8z1G=7R=kvack.org=owner-linux-mm@kernel.org>
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=-5.4 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1
	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 8866DC433E0
	for <linux-mm@archiver.kernel.org>; Thu,  4 Jun 2020 01:51:53 +0000 (UTC)
Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17])
	by mail.kernel.org (Postfix) with ESMTP id 3BC1120738
	for <linux-mm@archiver.kernel.org>; Thu,  4 Jun 2020 01:51:53 +0000 (UTC)
Authentication-Results: mail.kernel.org;
	dkim=pass (2048-bit key) header.d=nvidia.com header.i=@nvidia.com header.b="JfpT4OTs"
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3BC1120738
Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com
Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org
Received: by kanga.kvack.org (Postfix)
	id CB712280001; Wed,  3 Jun 2020 21:51:52 -0400 (EDT)
Received: by kanga.kvack.org (Postfix, from userid 40)
	id C67AE8000C; Wed,  3 Jun 2020 21:51:52 -0400 (EDT)
X-Delivered-To: int-list-linux-mm@kvack.org
Received: by kanga.kvack.org (Postfix, from userid 63042)
	id B567B280001; Wed,  3 Jun 2020 21:51:52 -0400 (EDT)
X-Delivered-To: linux-mm@kvack.org
Received: from forelay.hostedemail.com (smtprelay0111.hostedemail.com [216.40.44.111])
	by kanga.kvack.org (Postfix) with ESMTP id 9E13B8000C
	for <linux-mm@kvack.org>; Wed,  3 Jun 2020 21:51:52 -0400 (EDT)
Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251])
	by forelay04.hostedemail.com (Postfix) with ESMTP id 60F931EF2
	for <linux-mm@kvack.org>; Thu,  4 Jun 2020 01:51:52 +0000 (UTC)
X-FDA: 76889853264.20.boat55_4a772c630ef63
Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251])
	by smtpin20.hostedemail.com (Postfix) with ESMTP id 389E6180C07AB
	for <linux-mm@kvack.org>; Thu,  4 Jun 2020 01:51:52 +0000 (UTC)
X-HE-Tag: boat55_4a772c630ef63
X-Filterd-Recvd-Size: 10786
Received: from hqnvemgate25.nvidia.com (hqnvemgate25.nvidia.com [216.228.121.64])
	by imf19.hostedemail.com (Postfix) with ESMTP
	for <linux-mm@kvack.org>; Thu,  4 Jun 2020 01:51:51 +0000 (UTC)
Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqnvemgate25.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA)
	id <B5ed8535e0000>; Wed, 03 Jun 2020 18:50:22 -0700
Received: from hqmail.nvidia.com ([172.20.161.6])
  by hqpgpgate101.nvidia.com (PGP Universal service);
  Wed, 03 Jun 2020 18:51:50 -0700
X-PGP-Universal: processed;
	by hqpgpgate101.nvidia.com on Wed, 03 Jun 2020 18:51:50 -0700
Received: from [10.2.89.40] (10.124.1.5) by HQMAIL107.nvidia.com
 (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Thu, 4 Jun
 2020 01:51:44 +0000
Subject: Re: [patch 003/131] mm/gup: move __get_user_pages_fast() down a few
 lines in gup.c
To: Andrew Morton <akpm@linux-foundation.org>, <airlied@linux.ie>,
	<chris@chris-wilson.co.uk>, <daniel@ffwll.ch>, <jani.nikula@linux.intel.com>,
	<joonas.lahtinen@linux.intel.com>, <jrdr.linux@gmail.com>,
	<linux-mm@kvack.org>, <matthew.auld@intel.com>, <mm-commits@vger.kernel.org>,
	<rodrigo.vivi@intel.com>, <torvalds@linux-foundation.org>,
	<tvrtko.ursulin@intel.com>, <willy@infradead.org>
References: <20200603225627.KaTOZsw5O%akpm@linux-foundation.org>
X-Nvconfidentiality: public
From: John Hubbard <jhubbard@nvidia.com>
Message-ID: <2f010100-7d0f-e7bd-073e-e7f6baa205b2@nvidia.com>
Date: Wed, 3 Jun 2020 18:51:44 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <20200603225627.KaTOZsw5O%akpm@linux-foundation.org>
X-Originating-IP: [10.124.1.5]
X-ClientProxiedBy: HQMAIL111.nvidia.com (172.20.187.18) To
 HQMAIL107.nvidia.com (172.20.187.13)
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1;
	t=1591235422; bh=fE9HF9vcpJxI4siN1xHvqJ6kIfNjD/Fly43ApTQj514=;
	h=X-PGP-Universal:Subject:To:References:X-Nvconfidentiality:From:
	 Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:
	 X-Originating-IP:X-ClientProxiedBy:Content-Type:Content-Language:
	 Content-Transfer-Encoding;
	b=JfpT4OTsOuhFShqetOWS6P/T9jIK5nJnmiqetBfK2wEBnUAaNhYARQUQ1B9st5yZQ
	 vboNG2DEWzwceTpYq5pprDFOMsxp7fLBIG1Ig29qkHViRFGc0HQXEMOEpNJ9GupaW/
	 7Gv5fmIETIIv+AoGdLOj6NxNWFvh8OMHnnO+WktR2zLebT4cxE32p/bgwRkgwkE/SD
	 uct9CLcHxeJSeU17rC9AeW/f9ky45OpBHIqV/j+EsFaczKVBI2laVIzg/+DqByAKlC
	 qZdnrSA8Qka3r8akfIT+aCtPxgAAxdNDs0gSI4H03JRhMdsCOT3VsUO08gP956wT7t
	 3mp2+S/RgtWmg==
X-Rspamd-Queue-Id: 389E6180C07AB
X-Spamd-Result: default: False [0.00 / 100.00]
X-Rspamd-Server: rspam02
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
Sender: owner-linux-mm@kvack.org
Precedence: bulk
X-Loop: owner-majordomo@kvack.org
List-ID: <linux-mm.kvack.org>

On 2020-06-03 15:56, Andrew Morton wrote:
> From: John Hubbard <jhubbard@nvidia.com>
> Subject: mm/gup: move __get_user_pages_fast() down a few lines in gup.c
> 
> Patch series "mm/gup, drm/i915: refactor gup_fast, convert to pin_user_pages()", v2.

These patches 003 through 007 (gup refactoring and pin_user_pages stuff) all
look good.

Thanks for fixing up the merge conflicts with commit
17839856fd58 ("gup: document and work around "COW can break either way" issue").
I wasn't aware of that commit until the -next conflict email showed up in
my inbox this morning.

thanks,
-- 
John Hubbard
NVIDIA

> 
> In order to convert the drm/i915 driver from get_user_pages() to
> pin_user_pages(), a FOLL_PIN equivalent of __get_user_pages_fast() was
> required.  That led to refactoring __get_user_pages_fast(), with the
> following goals:
> 
> 1) As above: provide a pin_user_pages*() routine for drm/i915 to call,
>     in place of __get_user_pages_fast(),
> 
> 2) Get rid of the gup.c duplicate code for walking page tables with
>     interrupts disabled. This duplicate code is a minor maintenance
>     problem anyway.
> 
> 3) Make it easy for an upcoming patch from Souptick, which aims to
>     convert __get_user_pages_fast() to use a gup_flags argument, instead
>     of a bool writeable arg.  Also, if this series looks good, we can
>     ask Souptick to change the name as well, to whatever the consensus
>     is. My initial recommendation is: get_user_pages_fast_only(), to
>     match the new pin_user_pages_only().
> 
> 
> This patch (of 4):
> 
> This is in order to avoid a forward declaration of
> internal_get_user_pages_fast(), in the next patch.
> 
> This is code movement only--all generated code should be identical.
> 
> Link: http://lkml.kernel.org/r/20200522051931.54191-1-jhubbard@nvidia.com
> Link: http://lkml.kernel.org/r/20200519002124.2025955-1-jhubbard@nvidia.com
> Link: http://lkml.kernel.org/r/20200519002124.2025955-2-jhubbard@nvidia.com
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: "Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Souptick Joarder <jrdr.linux@gmail.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>   mm/gup.c |  132 ++++++++++++++++++++++++++---------------------------
>   1 file changed, 66 insertions(+), 66 deletions(-)
> 
> --- a/mm/gup.c~mm-gup-move-__get_user_pages_fast-down-a-few-lines-in-gupc
> +++ a/mm/gup.c
> @@ -2703,72 +2703,6 @@ static bool gup_fast_permitted(unsigned
>   }
>   #endif
>   
> -/*
> - * Like get_user_pages_fast() except it's IRQ-safe in that it won't fall back to
> - * the regular GUP.
> - * Note a difference with get_user_pages_fast: this always returns the
> - * number of pages pinned, 0 if no pages were pinned.
> - *
> - * If the architecture does not support this function, simply return with no
> - * pages pinned.
> - *
> - * Careful, careful! COW breaking can go either way, so a non-write
> - * access can get ambiguous page results. If you call this function without
> - * 'write' set, you'd better be sure that you're ok with that ambiguity.
> - */
> -int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> -			  struct page **pages)
> -{
> -	unsigned long len, end;
> -	unsigned long flags;
> -	int nr_pinned = 0;
> -	/*
> -	 * Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
> -	 * because gup fast is always a "pin with a +1 page refcount" request.
> -	 */
> -	unsigned int gup_flags = FOLL_GET;
> -
> -	if (write)
> -		gup_flags |= FOLL_WRITE;
> -
> -	start = untagged_addr(start) & PAGE_MASK;
> -	len = (unsigned long) nr_pages << PAGE_SHIFT;
> -	end = start + len;
> -
> -	if (end <= start)
> -		return 0;
> -	if (unlikely(!access_ok((void __user *)start, len)))
> -		return 0;
> -
> -	/*
> -	 * Disable interrupts.  We use the nested form as we can already have
> -	 * interrupts disabled by get_futex_key.
> -	 *
> -	 * With interrupts disabled, we block page table pages from being
> -	 * freed from under us. See struct mmu_table_batch comments in
> -	 * include/asm-generic/tlb.h for more details.
> -	 *
> -	 * We do not adopt an rcu_read_lock(.) here as we also want to
> -	 * block IPIs that come from THPs splitting.
> -	 *
> -	 * NOTE! We allow read-only gup_fast() here, but you'd better be
> -	 * careful about possible COW pages. You'll get _a_ COW page, but
> -	 * not necessarily the one you intended to get depending on what
> -	 * COW event happens after this. COW may break the page copy in a
> -	 * random direction.
> -	 */
> -
> -	if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
> -	    gup_fast_permitted(start, end)) {
> -		local_irq_save(flags);
> -		gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
> -		local_irq_restore(flags);
> -	}
> -
> -	return nr_pinned;
> -}
> -EXPORT_SYMBOL_GPL(__get_user_pages_fast);
> -
>   static int __gup_longterm_unlocked(unsigned long start, int nr_pages,
>   				   unsigned int gup_flags, struct page **pages)
>   {
> @@ -2848,6 +2782,72 @@ static int internal_get_user_pages_fast(
>   	return ret;
>   }
>   
> +/*
> + * Like get_user_pages_fast() except it's IRQ-safe in that it won't fall back to
> + * the regular GUP.
> + * Note a difference with get_user_pages_fast: this always returns the
> + * number of pages pinned, 0 if no pages were pinned.
> + *
> + * If the architecture does not support this function, simply return with no
> + * pages pinned.
> + *
> + * Careful, careful! COW breaking can go either way, so a non-write
> + * access can get ambiguous page results. If you call this function without
> + * 'write' set, you'd better be sure that you're ok with that ambiguity.
> + */
> +int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> +			  struct page **pages)
> +{
> +	unsigned long len, end;
> +	unsigned long flags;
> +	int nr_pinned = 0;
> +	/*
> +	 * Internally (within mm/gup.c), gup fast variants must set FOLL_GET,
> +	 * because gup fast is always a "pin with a +1 page refcount" request.
> +	 */
> +	unsigned int gup_flags = FOLL_GET;
> +
> +	if (write)
> +		gup_flags |= FOLL_WRITE;
> +
> +	start = untagged_addr(start) & PAGE_MASK;
> +	len = (unsigned long) nr_pages << PAGE_SHIFT;
> +	end = start + len;
> +
> +	if (end <= start)
> +		return 0;
> +	if (unlikely(!access_ok((void __user *)start, len)))
> +		return 0;
> +
> +	/*
> +	 * Disable interrupts.  We use the nested form as we can already have
> +	 * interrupts disabled by get_futex_key.
> +	 *
> +	 * With interrupts disabled, we block page table pages from being
> +	 * freed from under us. See struct mmu_table_batch comments in
> +	 * include/asm-generic/tlb.h for more details.
> +	 *
> +	 * We do not adopt an rcu_read_lock(.) here as we also want to
> +	 * block IPIs that come from THPs splitting.
> +	 *
> +	 * NOTE! We allow read-only gup_fast() here, but you'd better be
> +	 * careful about possible COW pages. You'll get _a_ COW page, but
> +	 * not necessarily the one you intended to get depending on what
> +	 * COW event happens after this. COW may break the page copy in a
> +	 * random direction.
> +	 */
> +
> +	if (IS_ENABLED(CONFIG_HAVE_FAST_GUP) &&
> +	    gup_fast_permitted(start, end)) {
> +		local_irq_save(flags);
> +		gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
> +		local_irq_restore(flags);
> +	}
> +
> +	return nr_pinned;
> +}
> +EXPORT_SYMBOL_GPL(__get_user_pages_fast);
> +
>   /**
>    * get_user_pages_fast() - pin user pages in memory
>    * @start:      starting user address
> _
>