On 21.10.24 19:26, Vlastimil Babka wrote:

On 10/21/24 19:14, Lorenzo Stoakes wrote:

quoted

On Mon, Oct 21, 2024 at 07:00:53PM +0200, David Hildenbrand wrote:

quoted

quoted

quoted

quoted

Also the existing logic is that existing markers (HW poison, uffd-simulated HW
poison, uffd wp marker) are retained and no error raised on MADV_DONTNEED, and
no error on MADV_FREE either, so it'd be consistent with existing behaviour.

HW poison / uffd-simulated HW poison are expected to be zapped: it's just
like a mapped page with HWPOISON. So that is correct.

Well, poison is _not_ zapped on MADV_DONTNEED but _is_ on MADV_FREE :) anyway, I

Huh?

madvise_dontneed_single_vma()->zap_page_range_single(details=NULL)->unmap_single_vma(details=NULL)
... zap_pte_range()

} else if (is_hwpoison_entry(entry) ||
	   is_poisoned_swp_entry(entry)) {
	if (!should_zap_cows(details))
		continue;
	...

Should just zap them.

What am I missing?

Yeah ok it's me who's missing something here, I hadn't noticed details == NULL
so should_zap_cows() is true, my mistake!

Well, good to know it's consistent then. As I've explained I see why zapping
actual hwpoison makes sense for MADV_DONTNEED/MADV_FREE. That it's done also
for uffd poison is not completely clear, but maybe it was just easier to
implement. 

Note that in VM context "uffd poison" really just is "this was hwpoison 
on the source VM, so we mimic that on the destination VM, because the 
data *is* lost" -- so you want the exact same behavior.

For example, when a VM reboots you might just want to ZAP these hwpoison 
entries, and get fresh pages on next access.

So to me it makes sense that they are treated equally.

-- 
Cheers,

David / dhildenb