linux-kernel - Re: [PATCH v2 06/10] mm: avoid using vma

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: <5b641d45-3898-40c9-8288-f2aacae1400d@lucifer.local>
Date: Fri, 30 Aug 2024 16:19:54 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>
Subject: Re: [PATCH v2 06/10] mm: avoid using vma_merge() for new VMAs

On Wed, Aug 28, 2024 at 04:52:07PM GMT, Liam R. Howlett wrote:

[snip]

> > +	/*
> > +	 * Now try to expand adjacent VMA(s). This takes care of removing the
> > +	 * following VMA if we have VMAs on both sides.
> > +	 */
> > +	if (vmg->vma && !vma_expand(vmg)) {
> > +		khugepaged_enter_vma(vmg->vma, vmg->flags);
>
> This might be able to be moved into vma_expand().

Sorry for the multiple emails, but am working my way through.

Can't do this, as relocate_vma_down() (and the original implementation in
fs/exec.c - I checked) does not invoke khugepaged_enter_vma(), sadly.

>
> > +
>
> Extra whitespace

Ack. A lot of these are to subjective taste for clarity, but am happy to
adjust these for the most part...

>
> > +		vmg->state = VMA_MERGE_SUCCESS;
> > +		return vmg->vma;
> > +	}
> > +
> > +	/* If expansion failed, reset state. Allows us to retry merge later. */
> > +	vmg->vma = NULL;
> > +	vmg->start = start;
> > +	vmg->end = end;
> > +	vmg->pgoff = pgoff;
> > +	if (vmg->vma == prev)
> > +		vma_iter_set(vmg->vmi, start);
> > +
> > +	return NULL;
> > +}
> > +
> >  /*
> >   * vma_expand - Expand an existing VMA
> >   *
> > @@ -474,7 +584,11 @@ void validate_mm(struct mm_struct *mm)
> >   * vmg->next->vm_end.  Checking if the vmg->vma can expand and merge with
> >   * vmg->next needs to be handled by the caller.
> >   *
> > - * Returns: 0 on success
> > + * Returns: 0 on success.
> > + *
> > + * ASSUMPTIONS:
> > + * - The caller must hold a WRITE lock on vmg->vma->mm->mmap_lock.
> > + * - The caller must have set @vmg->vma and @vmg->next.
> >   */
> >  int vma_expand(struct vma_merge_struct *vmg)
> >  {
> > @@ -484,6 +598,8 @@ int vma_expand(struct vma_merge_struct *vmg)
> >  	struct vm_area_struct *next = vmg->next;
> >  	struct vma_prepare vp;
> >
> > +	mmap_assert_write_locked(vmg->mm);
> > +
>
> There are a few unnecessary whitespaces here..

...except here :) I like to keep the asserts separate from the rest of the
logic, and local declarations on their own lines.

[snip]