linux-kernel - Re: [PATCH 01/20] mm: mmu

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:	Wed, 20 Apr 2011 10:47:28 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Andrea Arcangeli <aarcange@...hat.com>,
	Avi Kivity <avi@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Rik van Riel <riel@...hat.com>, Ingo Molnar <mingo@...e.hu>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-mm@...ck.org,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	David Miller <davem@...emloft.net>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Mel Gorman <mel@....ul.ie>, Nick Piggin <npiggin@...nel.dk>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Yanmin Zhang <yanmin_zhang@...ux.intel.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Russell King <rmk@....linux.org.uk>,
	Paul Mundt <lethal@...ux-sh.org>,
	Jeff Dike <jdike@...toit.com>, Tony Luck <tony.luck@...el.com>,
	Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH 01/20] mm: mmu_gather rework

On Tue, 2011-04-19 at 13:06 -0700, Andrew Morton wrote:
> On Fri, 01 Apr 2011 14:12:59 +0200
> Peter Zijlstra <a.p.zijlstra@...llo.nl> wrote:
> 
> > Remove the first obstackle towards a fully preemptible mmu_gather.
> > 
> > The current scheme assumes mmu_gather is always done with preemption
> > disabled and uses per-cpu storage for the page batches. Change this to
> > try and allocate a page for batching and in case of failure, use a
> > small on-stack array to make some progress.
> > 
> > Preemptible mmu_gather is desired in general and usable once
> > i_mmap_lock becomes a mutex. Doing it before the mutex conversion
> > saves us from having to rework the code by moving the mmu_gather
> > bits inside the pte_lock.
> > 
> > Also avoid flushing the tlb batches from under the pte lock,
> > this is useful even without the i_mmap_lock conversion as it
> > significantly reduces pte lock hold times.
> 
> There doesn't seem much point in reviewing this closely, as a lot of it
> gets tossed away later in the series..

That's a result of breaking patches along concept boundaries :/

> >  		free_pages_and_swap_cache(tlb->pages, tlb->nr);
> 
> It seems inappropriate that this code uses
> free_page[s]_and_swap_cache().  It should go direct to put_page() and
> release_pages()?  Please review this code's implicit decision to pass
> "cold==0" into release_pages().

Well, that isn't new with this patch, however it does look to be
correct. We're freeing user pages, those could indeed still be part of
the swapcache. Furthermore, the PAGEVEC_SIZE split in
free_pages_and_swap_cache() alone makes it worth calling that over
release_pages().

As to the cold==0, I think that too is correct since we don't actually
touch the pages themselves and we have no inkling as to their cache
state, we're simply wiping out user pages.

> > -static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page)
> > +static inline int __tlb_remove_page(struct mmu_gather *tlb, struct page *page)
> 
> I wonder if all the inlining which remains in this code is needed and
> desirable.

Probably not, the big plan was to make everybody use the generic code
and then move it into mm/memory.c or so.

But I guess I can have asm-generic/tlb.h define HAVE_GENERIC_MMU_GATHER
and make the compilation in mm/memory.c conditional on that (or generate
lots of Kconfig churn).
--
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/