linux-kernel - Re: [PATCH v6 2/4] mm: Add functions to support extra actions on swap in/out

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8665482b-f808-e995-cda1-99011be6ee34@linux.intel.com>
Date:   Fri, 24 Mar 2017 11:45:55 -0700
From:   Dave Hansen <dave.hansen@...ux.intel.com>
To:     Khalid Aziz <khalid.aziz@...cle.com>, akpm@...ux-foundation.org,
        davem@...emloft.net, arnd@...db.de
Cc:     kirill.shutemov@...ux.intel.com, mhocko@...e.com,
        jmarchan@...hat.com, vbabka@...e.cz, dan.j.williams@...el.com,
        lstoakes@...il.com, hannes@...xchg.org, mgorman@...e.de,
        hughd@...gle.com, vdavydov.dev@...il.com, minchan@...nel.org,
        namit@...are.com, linux-arch@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        sparclinux@...r.kernel.org, Khalid Aziz <khalid@...ehiking.org>
Subject: Re: [PATCH v6 2/4] mm: Add functions to support extra actions on swap
 in/out

On 02/28/2017 10:35 AM, Khalid Aziz wrote:
> diff --git a/mm/memory.c b/mm/memory.c
> index 6bf2b47..b086c76 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2658,6 +2658,7 @@ int do_swap_page(struct vm_fault *vmf)
>  	if (pte_swp_soft_dirty(vmf->orig_pte))
>  		pte = pte_mksoft_dirty(pte);
>  	set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte);
> +	arch_do_swap_page(vma->vm_mm, vmf->address, pte, vmf->orig_pte);
>  	vmf->orig_pte = pte;
>  	if (page == swapcache) {
>  		do_page_add_anon_rmap(page, vma, vmf->address, exclusive);
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 91619fd..192c41a 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1538,6 +1538,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  		swp_pte = swp_entry_to_pte(entry);
>  		if (pte_soft_dirty(pteval))
>  			swp_pte = pte_swp_mksoft_dirty(swp_pte);
> +		arch_unmap_one(mm, address, swp_pte, pteval);
>  		set_pte_at(mm, address, pte, swp_pte);
>  	} else if (PageAnon(page)) {
>  		swp_entry_t entry = { .val = page_private(page) };
> @@ -1571,6 +1572,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  		swp_pte = swp_entry_to_pte(entry);
>  		if (pte_soft_dirty(pteval))
>  			swp_pte = pte_swp_mksoft_dirty(swp_pte);
> +		arch_unmap_one(mm, address, swp_pte, pteval);
>  		set_pte_at(mm, address, pte, swp_pte);
>  	} else
>  		dec_mm_counter(mm, mm_counter_file(page));

>From a core VM perspective, I'm fine with these hooks.  It's minimally
invasive.  It is missing some explanation in the *code* of why sparc is
doing this and when/why other architectures might want to use these
hooks.  I think that would be awfully nice.

I still think the _current_ SPARC implementation of these hooks is
pretty broken because it doesn't allow more than one ADI tag within a
given page.  But, fixing that is confined to sparc code and shouldn't
affect the core VM or these hooks.

I suspect these hooks are still quite incomplete.  For instance, I do
not think KSM goes through these paths.  Couldn't a process *lose* its
ADI tags when KSM merges an underlying physical page?

I think you need to resolve your outstanding issues (from your 0/4
patch) before anyone can really ack these.  I suspect solving your
issues will change the number and placement of these hooks.

There is no mention in these patches of the effectively reduced virtual
address space.  Why?