Re: f45ec5ff16 ("userfaultfd: wp: support swap and page migration"): [ 140.777858] BUG: Bad rss-counter state mm:b278fc66 type:MM_ANONPAGES val:1

[Peter Xu]

On Sun, Apr 12, 2020 at 08:54:08PM +0800, Hillf Danton wrote:
> 
> On Fri, 10 Apr 2020 11:32:34 -0400 Peter Xu wrote:
> > 
> > I'm not sure this is correct.  As I mentioned, the commit wanted to
> > apply the uffd-wp bit even for the swap entries so that even the swap
> > entries got swapped in, the page will still be write protected.  So
> > IIUC think we can't remove that.
> 
> Yes you are right.
> 
> Now both CONFIG_MIGRATION and swap entry are restored after making uffd_wq
> survive migrate the same way as soft_dirty.
> 
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -236,6 +236,8 @@ static bool remove_migration_pte(struct
>  		pte = pte_mkold(mk_pte(new, READ_ONCE(vma->vm_page_prot)));
>  		if (pte_swp_soft_dirty(*pvmw.pte))
>  			pte = pte_mksoft_dirty(pte);
> +		if (pte_swp_uffd_wp(*pvmw.pte))
> +			pte = pte_mkuffd_wp(pte);
>  
>  		/*
>  		 * Recheck VMA as permissions can change since migration started
> @@ -243,15 +245,11 @@ static bool remove_migration_pte(struct
>  		entry = pte_to_swp_entry(*pvmw.pte);
>  		if (is_write_migration_entry(entry))
>  			pte = maybe_mkwrite(pte, vma);
> -		else if (pte_swp_uffd_wp(*pvmw.pte))
> -			pte = pte_mkuffd_wp(pte);
>  
>  		if (unlikely(is_zone_device_page(new))) {
>  			if (is_device_private_page(new)) {
>  				entry = make_device_private_entry(new, pte_write(pte));
>  				pte = swp_entry_to_pte(entry);
> -				if (pte_swp_uffd_wp(*pvmw.pte))
> -					pte = pte_mkuffd_wp(pte);
>  			}
>  		}
>  
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -139,11 +139,13 @@ static unsigned long change_pte_range(st
>  			}
>  			ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent);
>  			pages++;
> -		} else if (is_swap_pte(oldpte)) {
> +		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
>  			swp_entry_t entry = pte_to_swp_entry(oldpte);
>  			pte_t newpte;
>  
> -			if (is_write_migration_entry(entry)) {
> +			if (!non_swap_entry(entry)) {
> +				newpte = oldpte;
> +			} else if (is_write_migration_entry(entry)) {
>  				/*
>  				 * A protection check is difficult so
>  				 * just be safe and disable write
> @@ -164,7 +166,7 @@ static unsigned long change_pte_range(st
>  				if (pte_swp_uffd_wp(oldpte))
>  					newpte = pte_swp_mkuffd_wp(newpte);
>  			} else {
> -				newpte = oldpte;
> +				continue;
>  			}
>  
>  			if (uffd_wp)
> 
> 

Hi, Hillf,

Feel free to have a look at another report, which I think is the same
issue of this:

https://lore.kernel.org/lkml/CA+G9fYsRGvkqtpdGv_aVr+Hn17KgYq04Q=EE=pB774qVxRqOeg@xxxxxxxxxxxxxx/

IMHO this bisected commit is correct itself, it's just that we
shouldn't enable uffd-wp on 32bit system.

Thanks,

-- 
Peter Xu