Re: [PATCH v1 1/3] mm: split vm_normal_pages for LRU and non-LRU handling

From: Felix Kuehling <felix.kuehling@amd.com>
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
Subject: Re: [PATCH v1 1/3] mm: split vm_normal_pages for LRU and non-LRU handling
Date: Fri, 11 Mar 2022 12:08:05 -0500	[thread overview]
Message-ID: <1747447c-202d-9195-9d44-57f299be48c4@amd.com> (raw)
In-Reply-To: <07401a0a-6878-6af2-f663-9f0c3c1d88e5@redhat.com>

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").
>
>