Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <1747447c-202d-9195-9d44-57f299be48c4@amd.com>
Date:   Fri, 11 Mar 2022 12:08:05 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v1 1/3] mm: split vm_normal_pages for LRU and non-LRU
 handling
Content-Language: en-US
To:     David Hildenbrand <david@redhat.com>,
        Alex Sierra <alex.sierra@amd.com>, jgg@nvidia.com
Cc:     linux-mm@kvack.org, rcampbell@nvidia.com,
        linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        hch@lst.de, jglisse@redhat.com, apopple@nvidia.com,
        willy@infradead.org, akpm@linux-foundation.org
References: <20220310172633.9151-1-alex.sierra@amd.com>
 <20220310172633.9151-2-alex.sierra@amd.com>
 <07401a0a-6878-6af2-f663-9f0c3c1d88e5@redhat.com>
From:   Felix Kuehling <felix.kuehling@amd.com>
Organization: AMD Inc.
In-Reply-To: <07401a0a-6878-6af2-f663-9f0c3c1d88e5@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YWsxMXRLeXZHQVAxbEZnRjlRNU5PWksxWVE5aUFiV1dHWnhFVUh0TnlYakUz?=
 =?utf-8?B?ekZka3VDYXFyaVBtYjJkb00vUEFwNUVFUjBuZzBwenNzM1hBNFRETVFkU28z?=
 =?utf-8?B?OVRCYWxZVmdQNkV3ZU1nMStsVERTbU40dTYvOVZSNjF6RkYyN3I1Tmo1dDhk?=
 =?utf-8?B?UkVZZzBVZ2FnV3ZmdW41aGZ1OFhjbW9SZFFSak9jOCtiZGkrY0lFTFo3Lzky?=
 =?utf-8?B?L0ZxNWo5c1lQUnZETCs0MjMrR3h4bm5VSHl5UmZhZUJvdWptVTRrTXEwKzRa?=
 =?utf-8?B?dmZENHlTVTA4Rk53ZW5LWDV0OFNFUVhpQkJxNTFOWmt1cXZWWXZ3dlNpZTVL?=
 =?utf-8?B?M3RJL1VyK1Jnd2RjZTJxeFY1SGVHSTJ1QnJUVmVEWmJvTU5qZWZPTlp5endi?=
 =?utf-8?B?N1lFUU1IYjAvUXVzSVhwV1VRTEE0dDJCaUVxbDhCQThKS3lFVkkwZWRTTEVl?=
 =?utf-8?B?L285NzVDWHRIRU9ZSStqU095dG1zL01KbE0vcDVjbE9NY0hnZzNXMVMyUlFt?=
 =?utf-8?B?K0N3dGIyUWNNSFJFalpyekpTN0JHVGNFNVhTQ2s1ZGk3bDFkK2F6MmpNalFY?=
 =?utf-8?B?UG96NkV2ZGtCOW11ZENqRzltNC9uTGF3aWFMZGtYZkR5WGFuZkJlN2prUzVY?=
 =?utf-8?B?cVZnMXZqaGljQXVETExZc3lzZ05IOEc5OUtjcVB2cUJpNUtqRWlFM3o5Y3Yz?=
 =?utf-8?B?VFNLeVdIYXlhRkw0ZHJETU9UN3lBa2xMMHJuOWZSdTlkRmdtV2ZsRG9wVGJI?=
 =?utf-8?B?bkR3NG10bXJQZTl2dHhTYnFRWVJXOVBJSFJFekljTzY3MlJENXlaUzZ6dVRD?=
 =?utf-8?B?SmtsaEtmR3NGdzJkcHBQcW9yV1VHWjZ3MVE5N0Qrc2pMY0k3MUMvTjAxVll3?=
 =?utf-8?B?VnRRMnV1WWJjTnFkVWN6RWlqa2MxSmxXSVZXMktYaW04eGJBckZqWGhyZnNT?=
 =?utf-8?B?RUJyaGkyeWRDaUFDbnkrNHRReXlWbUhUM0kzVjk4M1NBdGJIQkFQR3hZVkty?=
 =?utf-8?B?dWltTXZTakRVR1A3YlNGeWVseEl1L1JPaE14aGhvMEEwRWtKZmJiaURoeHha?=
 =?utf-8?B?WW5PaklYNnFwN0gwRVA4YWxZM1M3RzA5R0l2RVdQQ2R6VHBNTTJwalQ5R2Qz?=
 =?utf-8?B?OERUODdlSERZRFhNdzJaUEdZVy9xMUtKTUY4UHFSS2NJMWtEeGJZL3JST2F6?=
 =?utf-8?B?THJQOUVld0VNRjlkcGpueWRWMVdlazN4NXUrL1A2Tmp4YzNIYkhoR0dZM1Vk?=
 =?utf-8?B?RnhrS1VZTFBJandJdzVKbG9aWXU3YWdtblpBME9nZlNVSzF0R3NyMHBRSGQ0?=
 =?utf-8?B?YVdENmF2SUZObC9lZG93b3dreExjRndJOUorWGxoZEE0VlBmaEZ1SHMxYUtI?=
 =?utf-8?B?bDN2djFmNm5qSUpneXNqQVQwTHhLSi9YaTNLYU52aXBFOFE5RWlJWG5UdEhY?=
 =?utf-8?B?S3EzSmF3ZlNRTlBQSlZXT2l3UDAxS251V2ZzS3VxTXJiaDBxd2ZJK0daSE0w?=
 =?utf-8?B?eXkyeldmdzA0RlZIdjd4SHFvSG14Wi9JUkJmeWNXdmpHL0JGUHlBYkRhRDFz?=
 =?utf-8?B?eDVHOERVSHNDbjExUmE1bVlnSkVCTTRiSUhxd0Q0bCtJNTN3ZWVCVmNLRytL?=
 =?utf-8?B?RjY1ZGp4cDhjSEFyN0NjelA0dm5xK0tBVEFEMktjamw2WDlVRGQ4Skh6MW5P?=
 =?utf-8?B?SnY4QkhRRTFlejkxUkNtaFlFdUVzdVVsdGVTcU1qS0pYejFYWmhSTFJYaHZB?=
 =?utf-8?B?Y3oxdnpRTVk4bGkwWlZJU200ZSt4cllVZnRvUlhLZW1VNCszbjFwUjdZeXZ4?=
 =?utf-8?B?c25udUcvazhoeUtqVVYvcGlNZGl4QnJlcjFpckZTRFBLY290a2ZQYUhUV29Q?=
 =?utf-8?B?VWd3TVFHN0hTSTlEbUZ0TUZaMksyMTlsTnoyNm9QVTl4WVN4UklZcU5kZVdC?=
 =?utf-8?B?RXA2TDNRNjFubU1PUjdWaU9NOHE3M3FmYXlZY2JLTGlpeTJxRHdnZjdTeHcx?=
 =?utf-8?B?VkRHYzQ4OGpkanNaNGFPR2w5bzhZbEZGemRWQXZnd01jRVd4SnNxNm1Mek5R?=
 =?utf-8?B?cWFqeXlwSG1Xa2I4alJIYWFLcFpjbU9oaUpvTVhwSGNLUVZZZmFLVmQ4a0k3?=
 =?utf-8?B?QUJvTHJaTGZiRHNwSXlxOUVRMDJCTm13Y1RLeVF3djVSUDFlMS9ZcVYxRWJ3?=
 =?utf-8?Q?sJcHsxWcTKC4jsBeWNMcnzo=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7f18a97a-9e6b-4e2f-7f88-08da0381b6e9
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2022 17:08:08.3422
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: I632W1stFcQAOzdfTAgrYz8frGjYHvaPp4PTPbqDCq2WRlgsPtrzXLW06cL0WUsWhr7mUKIEcucwdgyvtMcADw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR12MB5082
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE 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-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On 2022-03-11 04:16, David Hildenbrand wrote:
> On 10.03.22 18:26, Alex Sierra wrote:
>> DEVICE_COHERENT pages introduce a subtle distinction in the way
>> "normal" pages can be used by various callers throughout the kernel.
>> They behave like normal pages for purposes of mapping in CPU page
>> tables, and for COW. But they do not support LRU lists, NUMA
>> migration or THP. Therefore we split vm_normal_page into two
>> functions vm_normal_any_page and vm_normal_lru_page. The latter will
>> only return pages that can be put on an LRU list and that support
>> NUMA migration, KSM and THP.
>>
>> We also introduced a FOLL_LRU flag that adds the same behaviour to
>> follow_page and related APIs, to allow callers to specify that they
>> expect to put pages on an LRU list.
>>
> I still don't see the need for s/vm_normal_page/vm_normal_any_page/. And
> as this patch is dominated by that change, I'd suggest (again) to just
> drop it as I don't see any value of that renaming. No specifier implies any.

OK. If nobody objects, we can adopts that naming convention.


>
> The general idea of this change LGTM.
>
>
> I wonder how this interacts with the actual DEVICE_COHERENT coherent
> series. Is this a preparation? Should it be part of the DEVICE_COHERENT
> series?

Yes, it should be part of that series. Alex developed it on top of the 
series for now. But I think eventually it would need to be spliced into it.

Patch1 would need to go somewhere before the other DEVICE_COHERENT 
patches (with minor modifications). Patch 2 could be squashed into 
"tools: add hmm gup test for long term pinned device pages" or go next 
to it. Patch 3 doesn't have a direct dependency on device-coherent 
pages. It only mentions them in comments.


>
> IOW, should this patch start with
>
> "With DEVICE_COHERENT, we'll soon have vm_normal_pages() return
> device-managed anonymous pages that are not LRU pages. Although they
> behave like normal pages for purposes of mapping in CPU page, and for
> COW, they do not support LRU lists, NUMA migration or THP. [...]"

Yes, that makes sense.

Regards,
   Felix


>
> But then, I'm confused by patch 2 and 3, because it feels more like we'd
> already have DEVICE_COHERENT then ("hmm_is_coherent_type").
>
>