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: <31f7d71d-0eb9-2250-78c0-2e8f31023c66@nvidia.com>
Date:   Mon, 23 Jan 2023 19:08:39 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v8 10/10] mm: Renumber FOLL_PIN and FOLL_GET down
Content-Language: en-US
To:     David Howells <dhowells@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Christoph Hellwig <hch@infradead.org>
CC:     Matthew Wilcox <willy@infradead.org>, Jens Axboe <axboe@kernel.dk>,
        "Jan Kara" <jack@suse.cz>, Jeff Layton <jlayton@kernel.org>,
        Logan Gunthorpe <logang@deltatee.com>,
        <linux-fsdevel@vger.kernel.org>, <linux-block@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, "Christoph Hellwig" <hch@lst.de>,
        <linux-mm@kvack.org>
References: <20230123173007.325544-1-dhowells@redhat.com>
 <20230123173007.325544-11-dhowells@redhat.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20230123173007.325544-11-dhowells@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jan 2023 03:08:50.2194
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: e00a5585-07fb-4ab7-3b2e-08dafdb85121
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: CO1NAM11FT065.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4189
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/23/23 09:30, David Howells wrote:
> Renumber FOLL_PIN and FOLL_GET down to bit 0 and 1 respectively so that
> they are coincidentally the same as BIO_PAGE_PINNED and BIO_PAGE_REFFED and
> also so that they can be stored in the bottom two bits of a page pointer
> (something I'm looking at for zerocopy socket fragments).
> 
> (Note that BIO_PAGE_REFFED should probably be got rid of at some point,
> hence why FOLL_PIN is at 0.)
> 
> Also renumber down the other FOLL_* flags to close the gaps.

Should we also get these sorted into internal-to-mm and public sets?
Because Jason (+Cc) again was about to split them apart into
mm/internal.h [1] and that might make that a little cleaner.

Also, I don't think that there is any large readability difference
either way between 0x and <<1, so whatever you and Christophe settle on
there seems fine.

So either way with those points,

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

thanks,
-- 
John Hubbard
NVIDIA

> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Al Viro <viro@zeniv.linux.org.uk>
> cc: Christoph Hellwig <hch@lst.de>
> cc: Matthew Wilcox <willy@infradead.org>
> cc: linux-fsdevel@vger.kernel.org
> cc: linux-mm@kvack.org
> ---
> 
> Notes:
>      ver #8)
>       - Put FOLL_PIN at bit 0 and FOLL_GET at bit 1 to match BIO_PAGE_*.
>       - Renumber the remaining flags down to fill in the gap.
> 
>   include/linux/mm.h | 32 +++++++++++++++++---------------
>   1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 3de9d88f8524..c95bc4f77e8f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3074,26 +3074,28 @@ 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
> +#define FOLL_PIN	0x01	/* pages must be released via unpin_user_page */
> +#define FOLL_GET	0x02	/* do get_page on page (equivalent to BIO_FOLL_GET) */
> +#define FOLL_WRITE	0x04	/* check pte is writable */
> +#define FOLL_TOUCH	0x08	/* mark page accessed */
> +#define FOLL_DUMP	0x10	/* give error on hole if it would be zero */
> +#define FOLL_FORCE	0x20	/* get_user_pages read/write w/o permission */
> +#define FOLL_NOWAIT	0x40	/* 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_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 */
> -#define FOLL_PCI_P2PDMA	0x100000 /* allow returning PCI P2PDMA pages */
> -#define FOLL_INTERRUPTIBLE  0x200000 /* allow interrupts from generic signals */
> +#define FOLL_TRIED	0x200	/* a retry, previous pass started an IO */
> +#define FOLL_REMOTE	0x400	/* we are working on non-current tsk/mm */
> +#define FOLL_ANON	0x800	/* don't do file mappings */
> +#define FOLL_LONGTERM	0x1000	/* mapping lifetime is indefinite: see below */
> +#define FOLL_SPLIT_PMD	0x2000	/* split huge pmd before returning */
> +#define FOLL_FAST_ONLY	0x4000	/* gup_fast: prevent fall-back to slow gup */
> +#define FOLL_PCI_P2PDMA	0x8000 /* allow returning PCI P2PDMA pages */
> +#define FOLL_INTERRUPTIBLE  0x10000 /* allow interrupts from generic signals */
>   
>   /*
> + * Note that FOLL_PIN is sorted to bit 0 to be coincident with BIO_PAGE_PINNED.
> + *
>    * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each
>    * other. Here is what they mean, and how to use them:
>    *
> 
>