linux-kernel - Re: [PATCH v3] mm: memory: fix /proc/meminfo reporting for MLOCK

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: <01045F3D-B141-42E8-86C9-FA17B0E5EEE2@fb.com>
Date:   Mon, 16 Sep 2019 21:34:50 +0000
From:   Lucian Grijincu <lucian@...com>
To:     Michal Hocko <mhocko@...nel.org>
CC:     "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Souptick Joarder <jrdr.linux@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Rik van Riel <riel@...com>, Roman Gushchin <guro@...com>,
        Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH v3] mm: memory: fix /proc/meminfo reporting for
 MLOCK_ONFAULT



> On 9/16/19, 04:35, "Michal Hocko" <mhocko@...nel.org> wrote:
>     > diff --git a/mm/memory.c b/mm/memory.c
>     > index e0c232fe81d9..55da24f33bc4 100644
>     > --- a/mm/memory.c
>     > +++ b/mm/memory.c
>     > @@ -3311,6 +3311,8 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
>     >  	} else {
>     >  		inc_mm_counter_fast(vma->vm_mm, mm_counter_file(page));
>     >  		page_add_file_rmap(page, false);
>     > +		if (vma->vm_flags & VM_LOCKED && !PageTransCompound(page))
>     > +			mlock_vma_page(page);
>     >  	}
>     >  	set_pte_at(vma->vm_mm, vmf->address, vmf->pte, entry);
    
>     I dunno. Handling it here in alloc_set_pte sounds a bit weird to me.
>     Altough we already do mlock for CoW pages there, I thought this was more
>     of an exception.
>     Is there any real reason why this cannot be done in the standard #PF
>     path? finish_fault for example?

alloc_set_pte is called from finish_fault https://github.com/torvalds/linux/blob/v5.2/mm/memory.c#L3400

   vm_fault_t finish_fault(struct vm_fault *vmf)
     ...
	if (!ret)
		ret = alloc_set_pte(vmf, vmf->memcg, page);

and inside alloc_set_pte one of the branches of the if-clause already handled mlocked pages:
https://github.com/torvalds/linux/blob/v5.2/mm/memory.c#L3348-L3356

I added it to the else-branch as that seemed like the least intrusive change, but I will move this to finish_fault, probably like this (after I'm done testing):

   vm_fault_t finish_fault(struct vm_fault *vmf)
    ...
        if (!ret)
                ret = alloc_set_pte(vmf, vmf->memcg, page);
+       if (!ret && (vmf->vma->vm_flags & VM_LOCKED) && !PageTransCompound(page))
+                       mlock_vma_page(page);

Thanks for the review and suggestions!

--
Lucian