Re: [PATCH v3 1/8] mm: make PTE_MARKER_SWAPIN_ERROR more general

Peter Xu
2023-07-07T13:11:30Z

On Thu, Jul 06, 2023 at 03:50:29PM -0700, Axel Rasmussen wrote:
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 886f06066622..59e954586e2a 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -660,7 +660,7 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> free_swap_and_cache(entry);
> pte_clear_not_present_full(mm, addr, pte, tlb->fullmm);
> } else if (is_hwpoison_entry(entry) ||
> - is_swapin_error_entry(entry)) {
> + is_error_swp_entry(entry)) {

is_error_swp_entry() made me think whether we should call this marker as
ERROR at all - it's too generic, is_error_swp_entry() can be easily read as
"this swap entry is not legal". Can we come up with a less generic one?

PTE_MARKER_PAGE_POISONED / PTE_MARKER_POISONED / PTE_MARKER_PAGE_ERR / ...?

We do use poisoned only in real bad physical pages before, but I think we
can still keep using it when applying it to a pte. I think it's fine to
call a pte poisoned if it's not legal to access, just like a poisoned page.

> pte_clear_not_present_full(mm, addr, pte, tlb->fullmm);
> }
> continue;

The code change across the whole patch look good to me otherwise, thanks.

--
Peter Xu