Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Will Deacon <will.deacon@arm.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	Linux MM <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Rik van Riel <riel@surriel.com>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	Linux-sh list <linux-sh@vger.kernel.org>,
	Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather
Date: Wed, 4 Dec 2019 17:41:43 +0100
Message-ID: <20191204164143.GB2810@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <CAMuHMdVnhNFBqPQXKYCQbCnoQjZPSXRkuxbsbaguZ7_TcXXmVg@mail.gmail.com>

On Wed, Dec 04, 2019 at 04:07:53PM +0100, Geert Uytterhoeven wrote:
> On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <peterz@infradead.org> wrote:

> > Does this fare better?
> 
> Yes. Migo-R is happy again.
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/sh/include/asm/pgalloc.h
> > +++ b/arch/sh/include/asm/pgalloc.h
> > @@ -36,9 +36,7 @@ do {                                                  \
> >  #if CONFIG_PGTABLE_LEVELS > 2
> >  #define __pmd_free_tlb(tlb, pmdp, addr)                        \
> >  do {                                                   \
> > -       struct page *page = virt_to_page(pmdp);         \
> > -       pgtable_pmd_page_dtor(page);                    \
> > -       tlb_remove_page((tlb), page);                   \
> > +       pmd_free((tlb)->mm, (pmdp));                    \
> >  } while (0);
> >  #endif

OK, so I was going to write a Changelog to go with that, but then I
realized that while this works and is similar to before the patch, I'm
not sure this is in fact correct.

With this on (and also before) we're freeing the PMD before we've done
the TLB invalidate, that seems wrong!

Looking at the size of that pmd_cache, that looks to be 30-(12+12-3)+3
== 12, which is exactly 1 page, for PAGE_SIZE_4K, less for the larger
pages.

I'm thinking perhaps we should do something like the below instead?


---
 arch/sh/mm/pgtable.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/arch/sh/mm/pgtable.c b/arch/sh/mm/pgtable.c
index 5c8f9247c3c2..fac7e822fd0c 100644
--- a/arch/sh/mm/pgtable.c
+++ b/arch/sh/mm/pgtable.c
@@ -5,9 +5,6 @@
 #define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO
 
 static struct kmem_cache *pgd_cachep;
-#if PAGETABLE_LEVELS > 2
-static struct kmem_cache *pmd_cachep;
-#endif
 
 void pgd_ctor(void *x)
 {
@@ -23,11 +20,6 @@ void pgtable_cache_init(void)
 	pgd_cachep = kmem_cache_create("pgd_cache",
 				       PTRS_PER_PGD * (1<<PTE_MAGNITUDE),
 				       PAGE_SIZE, SLAB_PANIC, pgd_ctor);
-#if PAGETABLE_LEVELS > 2
-	pmd_cachep = kmem_cache_create("pmd_cache",
-				       PTRS_PER_PMD * (1<<PTE_MAGNITUDE),
-				       PAGE_SIZE, SLAB_PANIC, NULL);
-#endif
 }
 
 pgd_t *pgd_alloc(struct mm_struct *mm)
@@ -48,11 +40,7 @@ void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
 
 pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
 {
-	return kmem_cache_alloc(pmd_cachep, PGALLOC_GFP);
-}
-
-void pmd_free(struct mm_struct *mm, pmd_t *pmd)
-{
-	kmem_cache_free(pmd_cachep, pmd);
+	BUILD_BUG_ON(PTRS_PER_PMD * (1<<PTE_MAGNITUDE) <= PAGE_SIZE);
+	return (pmd_t *)__get_free_page(PGALLOC_GFP);
 }
 #endif /* PAGETABLE_LEVELS > 2 */


  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19 10:31 [PATCH v6 00/18] generic mmu_gather patches Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 01/18] asm-generic/tlb: Provide a comment Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 02/18] asm-generic/tlb: Provide HAVE_MMU_GATHER_PAGE_SIZE Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 03/18] asm-generic/tlb: Provide generic VIPT cache flush Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 04/18] asm-generic/tlb: Provide generic tlb_flush() based on flush_tlb_range() Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 05/18] asm-generic/tlb: Provide generic tlb_flush() based on flush_tlb_mm() Peter Zijlstra
2019-02-19 12:47   ` Will Deacon
2019-02-19 10:31 ` [PATCH v6 06/18] asm-generic/tlb: Conditionally provide tlb_migrate_finish() Peter Zijlstra
2019-02-19 12:47   ` Will Deacon
2019-02-19 13:41     ` Peter Zijlstra
2019-02-20 14:47       ` Will Deacon
2019-02-20 15:02         ` Matthew Wilcox
2019-02-19 10:31 ` [PATCH v6 07/18] asm-generic/tlb: Invert HAVE_RCU_TABLE_INVALIDATE Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 08/18] arm/tlb: Convert to generic mmu_gather Peter Zijlstra
2019-02-19 10:31 ` [PATCH v6 09/18] ia64/tlb: Conver " Peter Zijlstra
2019-02-19 12:47   ` Will Deacon
2019-02-21  2:52   ` Souptick Joarder
2019-02-19 10:31 ` [PATCH v6 10/18] sh/tlb: Convert SH " Peter Zijlstra
2019-12-03 11:19   ` Geert Uytterhoeven
2019-12-04 10:47     ` Peter Zijlstra
2019-12-04 12:32       ` Geert Uytterhoeven
2019-12-04 13:22         ` Guenter Roeck
2019-12-04 15:17           ` Geert Uytterhoeven
2019-12-04 19:03             ` Guenter Roeck
2019-12-04 13:34         ` Peter Zijlstra
2019-12-04 15:07           ` Geert Uytterhoeven
2019-12-04 16:41             ` Peter Zijlstra [this message]
2019-12-05 15:26               ` Geert Uytterhoeven
2019-12-05 19:24       ` Rob Landley
2019-12-05 19:23         ` Rich Felker
2019-12-05 19:30       ` John Paul Adrian Glaubitz
2019-12-05 22:56         ` Guenter Roeck
2019-12-06 13:38           ` John Paul Adrian Glaubitz
2019-12-06 14:03             ` Guenter Roeck
2019-02-19 10:31 ` [PATCH v6 11/18] um/tlb: Convert " Peter Zijlstra
2019-02-19 10:32 ` [PATCH v6 12/18] arch/tlb: Clean up simple architectures Peter Zijlstra
2019-02-19 10:32 ` [PATCH v6 13/18] asm-generic/tlb: Introduce HAVE_MMU_GATHER_NO_GATHER Peter Zijlstra
2019-02-19 12:47   ` Will Deacon
2019-02-19 10:32 ` [PATCH v6 14/18] s390/tlb: convert to generic mmu_gather Peter Zijlstra
2019-02-19 12:47   ` Will Deacon
2019-02-19 10:32 ` [PATCH v6 15/18] asm-generic/tlb: Remove arch_tlb*_mmu() Peter Zijlstra
2019-02-19 10:32 ` [PATCH v6 16/18] asm-generic/tlb: Remove HAVE_GENERIC_MMU_GATHER Peter Zijlstra
2019-02-19 10:32 ` [PATCH v6 17/18] asm-generic/tlb: Remove tlb_flush_mmu_free() Peter Zijlstra
2019-02-19 10:32 ` [PATCH v6 18/18] asm-generic/tlb: Remove tlb_table_flush() Peter Zijlstra

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191204164143.GB2810@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=dalias@libc.org \
    --cc=geert@linux-m68k.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linux@roeck-us.net \
    --cc=npiggin@gmail.com \
    --cc=riel@surriel.com \
    --cc=will.deacon@arm.com \
    --cc=ysato@users.sourceforge.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git