linux-kernel - Re: [PATCH] mremap: enforce rmap src/dst vma ordering in case of vma_merge succeeding in copy

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]

Date:	Mon, 7 Nov 2011 16:28:08 +0000
From:	Mel Gorman <mgorman@...e.de>
To:	Andrea Arcangeli <aarcange@...hat.com>
Cc:	Nai Xia <nai.xia@...il.com>, Hugh Dickins <hughd@...gle.com>,
	Pawel Sikora <pluto@...k.net>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	jpiszcz@...idpixels.com, arekm@...-linux.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mremap: enforce rmap src/dst vma ordering in case of
 vma_merge succeeding in copy_vma

On Mon, Nov 07, 2011 at 04:42:35PM +0100, Andrea Arcangeli wrote:
> On Mon, Nov 07, 2011 at 01:14:13PM +0000, Mel Gorman wrote:
> > I'm tending to agree. The number of cases that must be kept in mind
> > is getting too tricky. Taking the anon_vma lock may be slower but at
> > the risk of sounding chicken, it's easier to understand.
> > 
> > > But I think Mel indicated that anon_vma_locking might be
> > > harmful to JVM SMP performance.
> > > How severe you expect this to be, Mel ?
> > > 
> > 
> > I would only expect it to be a problem during garbage collection when
> > there is a greater likelihood that mremap is heavily used. While it
> > would have been nice to avoid additional overhead in mremap, I don't
> > think the JVM GC case on its own is sufficient justification to avoid
> > taking the anon_vma lock.
> 
> Adding one liner in the error path and a bugcheck in the *vmap case,
> doesn't seem the end of the world compared to my previous fix that you
> acked.

Note that I didn't suddenly turn that ack into a nack although

  1) A small comment on why we need to call anon_vma_moveto_tail in the
     error path would be nice

  2) It is unfortunate that we need the faulted_in_anon_vma just
     for a VM_BUG_ON check that only exists for CONFIG_DEBUG_VM
     but not earth shatting

What I said was taking the anon_vma lock may be slower but it was
generally easier to understand. I'm happy with the new patch too
particularly as it keeps the "ordering game" consistent for fork
and mremap but I previously missed move_page_tables in the error
path so was worried if there was something else I managed to miss
particularly in light of the "Allocating a new vma, copy first and
merge later" direction.

I'm also prefectly happy with my human meat brain and do not expect
to replace it with an aliens.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/