linux-kernel - Re: [PATCH] mm/vma: Do not register private-anon mappings with khugepaged during mmap

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] [day] [month] [year] [list]

Message-ID: <91f91b18-2188-4328-a458-60531c588186@lucifer.local>
Date: Thu, 6 Mar 2025 16:10:54 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Dev Jain <dev.jain@....com>
Cc: akpm@...ux-foundation.org, Liam.Howlett@...cle.com, vbabka@...e.cz,
        jannh@...gle.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        ryan.roberts@....com, anshuman.khandual@....com,
        aneesh.kumar@...nel.org, yang@...amperecomputing.com, david@...hat.com,
        willy@...radead.org, hughd@...gle.com, ziy@...dia.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] mm/vma: Do not register private-anon mappings with
 khugepaged during mmap

+cc Greg

On Thu, Mar 06, 2025 at 08:02:10PM +0530, Dev Jain wrote:
>
>
> On 06/03/25 1:48 pm, Lorenzo Stoakes wrote:
> > On Thu, Mar 06, 2025 at 12:00:37PM +0530, Dev Jain wrote:
> > > We already are registering private-anon VMAs with khugepaged during fault
> > > time, in do_huge_pmd_anonymous_page(). Commit "register suitable readonly
> > > file vmas for khugepaged" moved the khugepaged registration logic from
> > > shmem_mmap to the generic mmap path. Make this logic specific for non-anon
> > > mappings.
> >
> > This does sound reasonable, thanks! Though does need to be expanded as
> > Andrew says for user-visible effects just to be crystal clear.
>
> Sure.

Thanks!

>
> >
> > >
> > > Fixes: 613bec092fe7 ("mm: mmap: register suitable readonly file vmas for khugepaged")
> > > Signed-off-by: Dev Jain <dev.jain@....com>
> > > ---
> > >   mm/vma.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/vma.c b/mm/vma.c
> > > index af1d549b179c..730a26bf14a5 100644
> > > --- a/mm/vma.c
> > > +++ b/mm/vma.c
> > > @@ -2377,7 +2377,8 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap)
> > >   	 * vma_merge_new_range() calls khugepaged_enter_vma() too, the below
> > >   	 * call covers the non-merge case.
> > >   	 */
> > > -	khugepaged_enter_vma(vma, map->flags);
> > > +	if (!vma_is_anonymous(vma))
> > > +		khugepaged_enter_vma(vma, map->flags);
> >
> > This really needs a comment :) as a Joe Bloggs coder coming to this my
> > immediate thought would be 'huh? Why?'
> >
> > Something like:
> >
> > 	/* Just added so khugepaged has nothing to do. We call again on fault. */
> >
> > Would be great.
> >
> > Thanks!
>
> Sure.

Thanks!

>
> BTW does this patch merit a CC:stable? I am not aware of the general
> practice but I am not sure if this is a *serious bug*, as per
> submitting-patches.rst.

I think it's fine to send 'not serious' bugs too :P I mean this is a perf
regression right? We don't want that in our stable kernels.

Unless Greg suggests otherwise (cc'd) I'd say it does merit it as there seems to
be no risk, only benefit and reduces overhead, so from my point of view seems
sensible.

>
> >
> >
> > >   	ksm_add_vma(vma);
> > >   	*vmap = vma;
> > >   	return 0;
> > > --
> > > 2.30.2
> > >
>