Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.160 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C
Message-ID: <e1c01800-a5ba-ea69-c9d8-19b2cbe05d4f@nvidia.com>
Date:   Thu, 17 Nov 2022 15:15:40 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [RFC PATCH 1/4] mm: Move FOLL_* defs to mm_types.h
Content-Language: en-US
To:     David Howells <dhowells@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>
CC:     Matthew Wilcox <willy@infradead.org>, <linux-mm@kvack.org>,
        <linux-fsdevel@vger.kernel.org>,
        Christoph Hellwig <hch@infradead.org>,
        "Jeff Layton" <jlayton@kernel.org>, <linux-kernel@vger.kernel.org>
References: <166869687556.3723671.10061142538708346995.stgit@warthog.procyon.org.uk>
 <166869688542.3723671.10243929000823258622.stgit@warthog.procyon.org.uk>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <166869688542.3723671.10243929000823258622.stgit@warthog.procyon.org.uk>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2022 23:15:55.3501
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 83f8df6f-0fd8-4497-74e2-08dac8f1add6
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: BN8NAM11FT046.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7695
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/17/22 06:54, David Howells wrote:
> Move FOLL_* definitions to linux/mm_types.h to make them more accessible
> without having to drag in all of linux/mm.h and everything that drags in
> too[1].
> 
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: John Hubbard <jhubbard@nvidia.com>
> cc: Al Viro <viro@zeniv.linux.org.uk>
> cc: linux-mm@kvack.org
> cc: linux-fsdevel@vger.kernel.org
> Link: https://lore.kernel.org/linux-fsdevel/Y1%2FhSO+7kAJhGShG@casper.infradead.org/ [1]
> ---
> 
>   include/linux/mm.h       |   74 ----------------------------------------------
>   include/linux/mm_types.h |   73 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 74 deletions(-)


OK, I've verified that this is a "mostly identical" movement: the only
thing that changes is that the comments now come before the defines.

And because mm.h includes mm_types.h, it is unlikely that moving a
define from mm.h to mm_types.h would cause build failures. It's not
completely impossible: ordering issues are sometimes involved in this
sort of change. But unlikely.

Anyway, this is a good move. The users of various mm APIs should not
have to pull in quite so much of the internals of mm, and this is a step
in that direction. FOLL_* items are used by filesystems and other
subsystems that definitely do not need all of mm.h.


Reviewed-by: John Hubbard <jhubbard@nvidia.com>


thanks,
-- 
John Hubbard
NVIDIA

> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 8bbcccbc5565..7a7a287818ad 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2941,80 +2941,6 @@ static inline vm_fault_t vmf_error(int err)
>   struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
>   			 unsigned int foll_flags);
>   
> -#define FOLL_WRITE	0x01	/* check pte is writable */
> -#define FOLL_TOUCH	0x02	/* mark page accessed */
> -#define FOLL_GET	0x04	/* do get_page on page */
> -#define FOLL_DUMP	0x08	/* give error on hole if it would be zero */
> -#define FOLL_FORCE	0x10	/* get_user_pages read/write w/o permission */
> -#define FOLL_NOWAIT	0x20	/* if a disk transfer is needed, start the IO
> -				 * and return without waiting upon it */
> -#define FOLL_NOFAULT	0x80	/* do not fault in pages */
> -#define FOLL_HWPOISON	0x100	/* check page is hwpoisoned */
> -#define FOLL_MIGRATION	0x400	/* wait for page to replace migration entry */
> -#define FOLL_TRIED	0x800	/* a retry, previous pass started an IO */
> -#define FOLL_REMOTE	0x2000	/* we are working on non-current tsk/mm */
> -#define FOLL_ANON	0x8000	/* don't do file mappings */
> -#define FOLL_LONGTERM	0x10000	/* mapping lifetime is indefinite: see below */
> -#define FOLL_SPLIT_PMD	0x20000	/* split huge pmd before returning */
> -#define FOLL_PIN	0x40000	/* pages must be released via unpin_user_page */
> -#define FOLL_FAST_ONLY	0x80000	/* gup_fast: prevent fall-back to slow gup */
> -
> -/*
> - * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
> - * other. Here is what they mean, and how to use them:
> - *
> - * FOLL_LONGTERM indicates that the page will be held for an indefinite time
> - * period _often_ under userspace control.  This is in contrast to
> - * iov_iter_get_pages(), whose usages are transient.
> - *
> - * FIXME: For pages which are part of a filesystem, mappings are subject to the
> - * lifetime enforced by the filesystem and we need guarantees that longterm
> - * users like RDMA and V4L2 only establish mappings which coordinate usage with
> - * the filesystem.  Ideas for this coordination include revoking the longterm
> - * pin, delaying writeback, bounce buffer page writeback, etc.  As FS DAX was
> - * added after the problem with filesystems was found FS DAX VMAs are
> - * specifically failed.  Filesystem pages are still subject to bugs and use of
> - * FOLL_LONGTERM should be avoided on those pages.
> - *
> - * FIXME: Also NOTE that FOLL_LONGTERM is not supported in every GUP call.
> - * Currently only get_user_pages() and get_user_pages_fast() support this flag
> - * and calls to get_user_pages_[un]locked are specifically not allowed.  This
> - * is due to an incompatibility with the FS DAX check and
> - * FAULT_FLAG_ALLOW_RETRY.
> - *
> - * In the CMA case: long term pins in a CMA region would unnecessarily fragment
> - * that region.  And so, CMA attempts to migrate the page before pinning, when
> - * FOLL_LONGTERM is specified.
> - *
> - * FOLL_PIN indicates that a special kind of tracking (not just page->_refcount,
> - * but an additional pin counting system) will be invoked. This is intended for
> - * anything that gets a page reference and then touches page data (for example,
> - * Direct IO). This lets the filesystem know that some non-file-system entity is
> - * potentially changing the pages' data. In contrast to FOLL_GET (whose pages
> - * are released via put_page()), FOLL_PIN pages must be released, ultimately, by
> - * a call to unpin_user_page().
> - *
> - * FOLL_PIN is similar to FOLL_GET: both of these pin pages. They use different
> - * and separate refcounting mechanisms, however, and that means that each has
> - * its own acquire and release mechanisms:
> - *
> - *     FOLL_GET: get_user_pages*() to acquire, and put_page() to release.
> - *
> - *     FOLL_PIN: pin_user_pages*() to acquire, and unpin_user_pages to release.
> - *
> - * FOLL_PIN and FOLL_GET are mutually exclusive for a given function call.
> - * (The underlying pages may experience both FOLL_GET-based and FOLL_PIN-based
> - * calls applied to them, and that's perfectly OK. This is a constraint on the
> - * callers, not on the pages.)
> - *
> - * FOLL_PIN should be set internally by the pin_user_pages*() APIs, never
> - * directly by the caller. That's in order to help avoid mismatches when
> - * releasing pages: get_user_pages*() pages must be released via put_page(),
> - * while pin_user_pages*() pages must be released via unpin_user_page().
> - *
> - * Please see Documentation/core-api/pin_user_pages.rst for more information.
> - */
> -
>   static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
>   {
>   	if (vm_fault & VM_FAULT_OOM)
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 500e536796ca..0c80a5ad6e6a 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1003,4 +1003,77 @@ enum fault_flag {
>   
>   typedef unsigned int __bitwise zap_flags_t;
>   
> +/*
> + * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
> + * other. Here is what they mean, and how to use them:
> + *
> + * FOLL_LONGTERM indicates that the page will be held for an indefinite time
> + * period _often_ under userspace control.  This is in contrast to
> + * iov_iter_get_pages(), whose usages are transient.
> + *
> + * FIXME: For pages which are part of a filesystem, mappings are subject to the
> + * lifetime enforced by the filesystem and we need guarantees that longterm
> + * users like RDMA and V4L2 only establish mappings which coordinate usage with
> + * the filesystem.  Ideas for this coordination include revoking the longterm
> + * pin, delaying writeback, bounce buffer page writeback, etc.  As FS DAX was
> + * added after the problem with filesystems was found FS DAX VMAs are
> + * specifically failed.  Filesystem pages are still subject to bugs and use of
> + * FOLL_LONGTERM should be avoided on those pages.
> + *
> + * FIXME: Also NOTE that FOLL_LONGTERM is not supported in every GUP call.
> + * Currently only get_user_pages() and get_user_pages_fast() support this flag
> + * and calls to get_user_pages_[un]locked are specifically not allowed.  This
> + * is due to an incompatibility with the FS DAX check and
> + * FAULT_FLAG_ALLOW_RETRY.
> + *
> + * In the CMA case: long term pins in a CMA region would unnecessarily fragment
> + * that region.  And so, CMA attempts to migrate the page before pinning, when
> + * FOLL_LONGTERM is specified.
> + *
> + * FOLL_PIN indicates that a special kind of tracking (not just page->_refcount,
> + * but an additional pin counting system) will be invoked. This is intended for
> + * anything that gets a page reference and then touches page data (for example,
> + * Direct IO). This lets the filesystem know that some non-file-system entity is
> + * potentially changing the pages' data. In contrast to FOLL_GET (whose pages
> + * are released via put_page()), FOLL_PIN pages must be released, ultimately, by
> + * a call to unpin_user_page().
> + *
> + * FOLL_PIN is similar to FOLL_GET: both of these pin pages. They use different
> + * and separate refcounting mechanisms, however, and that means that each has
> + * its own acquire and release mechanisms:
> + *
> + *     FOLL_GET: get_user_pages*() to acquire, and put_page() to release.
> + *
> + *     FOLL_PIN: pin_user_pages*() to acquire, and unpin_user_pages to release.
> + *
> + * FOLL_PIN and FOLL_GET are mutually exclusive for a given function call.
> + * (The underlying pages may experience both FOLL_GET-based and FOLL_PIN-based
> + * calls applied to them, and that's perfectly OK. This is a constraint on the
> + * callers, not on the pages.)
> + *
> + * FOLL_PIN should be set internally by the pin_user_pages*() APIs, never
> + * directly by the caller. That's in order to help avoid mismatches when
> + * releasing pages: get_user_pages*() pages must be released via put_page(),
> + * while pin_user_pages*() pages must be released via unpin_user_page().
> + *
> + * Please see Documentation/core-api/pin_user_pages.rst for more information.
> + */
> +#define FOLL_WRITE	0x01	/* check pte is writable */
> +#define FOLL_TOUCH	0x02	/* mark page accessed */
> +#define FOLL_GET	0x04	/* do get_page on page */
> +#define FOLL_DUMP	0x08	/* give error on hole if it would be zero */
> +#define FOLL_FORCE	0x10	/* get_user_pages read/write w/o permission */
> +#define FOLL_NOWAIT	0x20	/* if a disk transfer is needed, start the IO
> +				 * and return without waiting upon it */
> +#define FOLL_NOFAULT	0x80	/* do not fault in pages */
> +#define FOLL_HWPOISON	0x100	/* check page is hwpoisoned */
> +#define FOLL_MIGRATION	0x400	/* wait for page to replace migration entry */
> +#define FOLL_TRIED	0x800	/* a retry, previous pass started an IO */
> +#define FOLL_REMOTE	0x2000	/* we are working on non-current tsk/mm */
> +#define FOLL_ANON	0x8000	/* don't do file mappings */
> +#define FOLL_LONGTERM	0x10000	/* mapping lifetime is indefinite: see below */
> +#define FOLL_SPLIT_PMD	0x20000	/* split huge pmd before returning */
> +#define FOLL_PIN	0x40000	/* pages must be released via unpin_user_page */
> +#define FOLL_FAST_ONLY	0x80000	/* gup_fast: prevent fall-back to slow gup */
> +
>   #endif /* _LINUX_MM_TYPES_H */
> 
>