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

[Felix Kuehling <felix.kuehling@amd.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").
>
>