linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Piggin <npiggin@suse.de>,
	Linux Memory Management <linux-mm@kvack.org>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Hugh Dickins <hugh@tiscali.co.uk>, ralf <ralf@linux-mips.org>,
	Paul Mundt <lethal@linux-sh.org>
Subject: Re: [RFC/PATCH] mm: Pass virtual address to [__]p{te,ud,md}_free_tlb()
Date: Tue, 28 Jul 2009 10:17:40 +1000	[thread overview]
Message-ID: <1248740260.30993.26.camel@pasglop> (raw)
In-Reply-To: <alpine.LFD.2.01.0907271210210.25224@localhost.localdomain>

On Mon, 2009-07-27 at 12:11 -0700, Linus Torvalds wrote:
> 
> On Thu, 23 Jul 2009, Benjamin Herrenschmidt wrote:
> > 
> > Hrm... my powerpc-next branch will contain stuff that depend on it, so
> > I'll probably have to pull it in though, unless I tell all my
> > sub-maintainers to also pull from that other branch first :-)
> 
> Ok, I'll just apply the patch. It does look obvious enough.

There seem to be a MIPS and SH breakage as a result but I can't see
how my patch would have broken it, ie, it looks like the bug was
already in those two archs. The error is that it complains about a
duplicate definition of __pmd_free_tlb() between those arch pgalloc.h
and pgtable-nopmd.h

For MIPS, when CONFIG_32BIT is set, asm/pgalloc.h redefines
__pmd_free_tlb despite the fact that it's already defined by
asm-generic/pgtable-nopmd.h (via via pgtable.h via linux/mm.h).

I -suspect- what happens is that the compiler, before, would ignore the
double definition (or maybe just warn) due to the definition being
strictly identical. With the new argument added, it's no longer the case
as it's called "a" in asm-generic and "addr" in mips... oops.

In any case, can Ralf and Paul check if the following patch is correct ?

From 41928c7945d855ae0eb053eadad590ab6876847e Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 28 Jul 2009 10:16:48 +1000
Subject: [PATCH] mm: Remove duplicate definitions in MIPS and SH

Those definitions are already provided by asm-generic

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/mips/include/asm/pgalloc.h |   11 -----------
 arch/sh/include/asm/pgalloc.h   |    8 --------
 2 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h
index f705735..3738f4b 100644
--- a/arch/mips/include/asm/pgalloc.h
+++ b/arch/mips/include/asm/pgalloc.h
@@ -104,17 +104,6 @@ do {							\
 	tlb_remove_page((tlb), pte);			\
 } while (0)
 
-#ifdef CONFIG_32BIT
-
-/*
- * allocating and freeing a pmd is trivial: the 1-entry pmd is
- * inside the pgd, so has no extra memory associated with it.
- */
-#define pmd_free(mm, x)			do { } while (0)
-#define __pmd_free_tlb(tlb, x, addr)	do { } while (0)
-
-#endif
-
 #ifdef CONFIG_64BIT
 
 static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h
index 89a4827..63ca37b 100644
--- a/arch/sh/include/asm/pgalloc.h
+++ b/arch/sh/include/asm/pgalloc.h
@@ -79,14 +79,6 @@ do {							\
 	tlb_remove_page((tlb), (pte));			\
 } while (0)
 
-/*
- * allocating and freeing a pmd is trivial: the 1-entry pmd is
- * inside the pgd, so has no extra memory associated with it.
- */
-
-#define pmd_free(mm, x)			do { } while (0)
-#define __pmd_free_tlb(tlb,x,addr)	do { } while (0)
-
 static inline void check_pgt_cache(void)
 {
 	quicklist_trim(QUICK_PGD, NULL, 25, 16);
-- 
1.6.1.2.14.gf26b5



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Piggin <npiggin@suse.de>,
	Linux Memory Management <linux-mm@kvack.org>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Hugh Dickins <hugh@tiscali.co.uk>, ralf <ralf@linux-mips.org>,
	Paul Mundt <lethal@linux-sh.org>
Subject: Re: [RFC/PATCH] mm: Pass virtual address to [__]p{te,ud,md}_free_tlb()
Date: Tue, 28 Jul 2009 10:17:40 +1000	[thread overview]
Message-ID: <1248740260.30993.26.camel@pasglop> (raw)
Message-ID: <20090728001740.hx7CcO-lOteuUTZqUPrITQQXM-hcRoUzVmV5YEgAaRM@z> (raw)
In-Reply-To: <alpine.LFD.2.01.0907271210210.25224@localhost.localdomain>

On Mon, 2009-07-27 at 12:11 -0700, Linus Torvalds wrote:
> 
> On Thu, 23 Jul 2009, Benjamin Herrenschmidt wrote:
> > 
> > Hrm... my powerpc-next branch will contain stuff that depend on it, so
> > I'll probably have to pull it in though, unless I tell all my
> > sub-maintainers to also pull from that other branch first :-)
> 
> Ok, I'll just apply the patch. It does look obvious enough.

There seem to be a MIPS and SH breakage as a result but I can't see
how my patch would have broken it, ie, it looks like the bug was
already in those two archs. The error is that it complains about a
duplicate definition of __pmd_free_tlb() between those arch pgalloc.h
and pgtable-nopmd.h

For MIPS, when CONFIG_32BIT is set, asm/pgalloc.h redefines
__pmd_free_tlb despite the fact that it's already defined by
asm-generic/pgtable-nopmd.h (via via pgtable.h via linux/mm.h).

I -suspect- what happens is that the compiler, before, would ignore the
double definition (or maybe just warn) due to the definition being
strictly identical. With the new argument added, it's no longer the case
as it's called "a" in asm-generic and "addr" in mips... oops.

In any case, can Ralf and Paul check if the following patch is correct ?

From 41928c7945d855ae0eb053eadad590ab6876847e Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 28 Jul 2009 10:16:48 +1000
Subject: [PATCH] mm: Remove duplicate definitions in MIPS and SH

Those definitions are already provided by asm-generic

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/mips/include/asm/pgalloc.h |   11 -----------
 arch/sh/include/asm/pgalloc.h   |    8 --------
 2 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h
index f705735..3738f4b 100644
--- a/arch/mips/include/asm/pgalloc.h
+++ b/arch/mips/include/asm/pgalloc.h
@@ -104,17 +104,6 @@ do {							\
 	tlb_remove_page((tlb), pte);			\
 } while (0)
 
-#ifdef CONFIG_32BIT
-
-/*
- * allocating and freeing a pmd is trivial: the 1-entry pmd is
- * inside the pgd, so has no extra memory associated with it.
- */
-#define pmd_free(mm, x)			do { } while (0)
-#define __pmd_free_tlb(tlb, x, addr)	do { } while (0)
-
-#endif
-
 #ifdef CONFIG_64BIT
 
 static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address)
diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h
index 89a4827..63ca37b 100644
--- a/arch/sh/include/asm/pgalloc.h
+++ b/arch/sh/include/asm/pgalloc.h
@@ -79,14 +79,6 @@ do {							\
 	tlb_remove_page((tlb), (pte));			\
 } while (0)
 
-/*
- * allocating and freeing a pmd is trivial: the 1-entry pmd is
- * inside the pgd, so has no extra memory associated with it.
- */
-
-#define pmd_free(mm, x)			do { } while (0)
-#define __pmd_free_tlb(tlb,x,addr)	do { } while (0)
-
 static inline void check_pgt_cache(void)
 {
 	quicklist_trim(QUICK_PGD, NULL, 25, 16);
-- 
1.6.1.2.14.gf26b5




  parent reply	other threads:[~2009-07-28  0:17 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-15  7:49 [RFC/PATCH] mm: Pass virtual address to [__]p{te,ud,md}_free_tlb() Benjamin Herrenschmidt
2009-07-15 13:56 ` [RFC/PATCH] mm: Pass virtual address to [__]p{te, ud, md}_free_tlb() Nick Piggin
2009-07-15 13:56   ` [RFC/PATCH] mm: Pass virtual address to [__]p{te,ud,md}_free_tlb() Nick Piggin
2009-07-16  1:54   ` Benjamin Herrenschmidt
2009-07-16  1:54     ` Benjamin Herrenschmidt
2009-07-20  8:10     ` Nick Piggin
2009-07-20 10:00       ` Benjamin Herrenschmidt
2009-07-20 10:00         ` Benjamin Herrenschmidt
2009-07-20 10:38         ` Nick Piggin
2009-07-21  0:02           ` Benjamin Herrenschmidt
2009-07-21  0:02             ` Benjamin Herrenschmidt
2009-07-21  7:05             ` Nick Piggin
2009-07-20  7:11   ` Benjamin Herrenschmidt
2009-07-20  7:11     ` Benjamin Herrenschmidt
2009-07-20  7:48     ` Martin Schwidefsky
2009-07-20  7:48       ` Martin Schwidefsky
2009-07-20  8:05     ` Nick Piggin
2009-07-20  8:05       ` Nick Piggin
2009-07-20  9:59       ` Benjamin Herrenschmidt
2009-07-20  9:59         ` Benjamin Herrenschmidt
2009-07-20 10:39         ` Nick Piggin
2009-07-22 16:31     ` Linus Torvalds
2009-07-23  0:53       ` Benjamin Herrenschmidt
2009-07-23  0:59         ` Kumar Gala
2009-07-27 19:11         ` Linus Torvalds
2009-07-27 19:11           ` Linus Torvalds
2009-07-27 21:35           ` Benjamin Herrenschmidt
2009-07-27 21:35             ` Benjamin Herrenschmidt
2009-07-28  0:17           ` Benjamin Herrenschmidt [this message]
2009-07-28  0:17             ` Benjamin Herrenschmidt
2009-07-28  0:25             ` Paul Mundt
2009-07-28  0:41               ` Linus Torvalds
2009-07-28  0:41                 ` Linus Torvalds
2009-07-16  1:36 ` Michael Ellerman
2009-07-16  1:56   ` Benjamin Herrenschmidt
2009-07-16  1:56     ` Benjamin Herrenschmidt
2009-07-20 12:46 ` David Howells
2009-07-20 12:46   ` David Howells

Reply instructions:

You may reply publicly 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=1248740260.30993.26.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=hugh@tiscali.co.uk \
    --cc=lethal@linux-sh.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=npiggin@suse.de \
    --cc=ralf@linux-mips.org \
    --cc=torvalds@linux-foundation.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).