* [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case
@ 2013-11-07 14:14 Kirill A. Shutemov
2013-11-07 14:14 ` [PATCH 2/2] mm: create a separate slab for page->ptl allocation Kirill A. Shutemov
2013-11-15 22:01 ` [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Tony Luck
0 siblings, 2 replies; 8+ messages in thread
From: Kirill A. Shutemov @ 2013-11-07 14:14 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, linux-mm, Kirill A. Shutemov
From: Peter Zijlstra <peterz@infradead.org>
Use kernel/bounds.c to convert build-time spinlock_t size check into
a preprocessor symbol and apply that to properly separate the page::ptl
situation.
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/mm.h | 24 +++++++++++++-----------
include/linux/mm_types.h | 9 +++++----
kernel/bounds.c | 2 ++
mm/memory.c | 11 +++++------
4 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index d0339741b6ce..1cedd000cf29 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1317,27 +1317,29 @@ static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long a
#endif /* CONFIG_MMU && !__ARCH_HAS_4LEVEL_HACK */
#if USE_SPLIT_PTE_PTLOCKS
-bool __ptlock_alloc(struct page *page);
-void __ptlock_free(struct page *page);
+#if BLOATED_SPINLOCKS
+extern bool ptlock_alloc(struct page *page);
+extern void ptlock_free(struct page *page);
+
+static inline spinlock_t *ptlock_ptr(struct page *page)
+{
+ return page->ptl;
+}
+#else /* BLOATED_SPINLOCKS */
static inline bool ptlock_alloc(struct page *page)
{
- if (sizeof(spinlock_t) > sizeof(page->ptl))
- return __ptlock_alloc(page);
return true;
}
+
static inline void ptlock_free(struct page *page)
{
- if (sizeof(spinlock_t) > sizeof(page->ptl))
- __ptlock_free(page);
}
static inline spinlock_t *ptlock_ptr(struct page *page)
{
- if (sizeof(spinlock_t) > sizeof(page->ptl))
- return (spinlock_t *) page->ptl;
- else
- return (spinlock_t *) &page->ptl;
+ return &page->ptl;
}
+#endif /* BLOATED_SPINLOCKS */
static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
{
@@ -1354,7 +1356,7 @@ static inline bool ptlock_init(struct page *page)
* slab code uses page->slab_cache and page->first_page (for tail
* pages), which share storage with page->ptl.
*/
- VM_BUG_ON(page->ptl);
+ VM_BUG_ON(*(unsigned long *)&page->ptl);
if (!ptlock_alloc(page))
return false;
spin_lock_init(ptlock_ptr(page));
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 752b6d4ee5dc..7ddc3d5c7776 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -156,10 +156,11 @@ struct page {
* system if PG_buddy is set.
*/
#if USE_SPLIT_PTE_PTLOCKS
- unsigned long ptl; /* It's spinlock_t if it fits to long,
- * otherwise it's pointer to dynamicaly
- * allocated spinlock_t.
- */
+#if BLOATED_SPINLOCKS
+ spinlock_t *ptl;
+#else
+ spinlock_t ptl;
+#endif
#endif
struct kmem_cache *slab_cache; /* SL[AU]B: Pointer to slab */
struct page *first_page; /* Compound tail pages */
diff --git a/kernel/bounds.c b/kernel/bounds.c
index e8ca97b5c386..578782ef6ae1 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -11,6 +11,7 @@
#include <linux/kbuild.h>
#include <linux/page_cgroup.h>
#include <linux/log2.h>
+#include <linux/spinlock.h>
void foo(void)
{
@@ -21,5 +22,6 @@ void foo(void)
#ifdef CONFIG_SMP
DEFINE(NR_CPUS_BITS, ilog2(CONFIG_NR_CPUS));
#endif
+ DEFINE(BLOATED_SPINLOCKS, sizeof(spinlock_t) > sizeof(int));
/* End of constants */
}
diff --git a/mm/memory.c b/mm/memory.c
index 6f7bdee617e2..f6cd03e4dec6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4271,21 +4271,20 @@ void copy_user_huge_page(struct page *dst, struct page *src,
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
-#if USE_SPLIT_PTE_PTLOCKS
-bool __ptlock_alloc(struct page *page)
+#if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
+bool ptlock_alloc(struct page *page)
{
spinlock_t *ptl;
ptl = kmalloc(sizeof(spinlock_t), GFP_KERNEL);
if (!ptl)
return false;
- page->ptl = (unsigned long)ptl;
+ page->ptl = ptl;
return true;
}
-void __ptlock_free(struct page *page)
+void ptlock_free(struct page *page)
{
- if (sizeof(spinlock_t) > sizeof(page->ptl))
- kfree((spinlock_t *)page->ptl);
+ kfree(page->ptl);
}
#endif
--
1.8.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] mm: create a separate slab for page->ptl allocation
2013-11-07 14:14 [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Kirill A. Shutemov
@ 2013-11-07 14:14 ` Kirill A. Shutemov
2013-11-16 20:43 ` Geert Uytterhoeven
2013-11-15 22:01 ` [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Tony Luck
1 sibling, 1 reply; 8+ messages in thread
From: Kirill A. Shutemov @ 2013-11-07 14:14 UTC (permalink / raw)
To: Andrew Morton
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, linux-mm, Kirill A. Shutemov
If DEBUG_SPINLOCK and DEBUG_LOCK_ALLOC are enabled spinlock_t on x86_64
is 72 bytes. For page->ptl they will be allocated from kmalloc-96 slab,
so we loose 24 on each. An average system can easily allocate few tens
thousands of page->ptl and overhead is significant.
Let's create a separate slab for page->ptl allocation to solve this.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
include/linux/mm.h | 9 +++++++++
init/main.c | 2 +-
mm/memory.c | 7 +++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1cedd000cf29..0548eb201e05 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1318,6 +1318,7 @@ static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long a
#if USE_SPLIT_PTE_PTLOCKS
#if BLOATED_SPINLOCKS
+void __init ptlock_cache_init(void);
extern bool ptlock_alloc(struct page *page);
extern void ptlock_free(struct page *page);
@@ -1326,6 +1327,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
return page->ptl;
}
#else /* BLOATED_SPINLOCKS */
+static inline void ptlock_cache_init(void) {}
static inline bool ptlock_alloc(struct page *page)
{
return true;
@@ -1378,10 +1380,17 @@ static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
{
return &mm->page_table_lock;
}
+static inline void ptlock_cache_init(void) {}
static inline bool ptlock_init(struct page *page) { return true; }
static inline void pte_lock_deinit(struct page *page) {}
#endif /* USE_SPLIT_PTE_PTLOCKS */
+static inline void pgtable_init(void)
+{
+ ptlock_cache_init();
+ pgtable_cache_init();
+}
+
static inline bool pgtable_page_ctor(struct page *page)
{
inc_zone_page_state(page, NR_PAGETABLE);
diff --git a/init/main.c b/init/main.c
index febc511e078a..01573fdfa186 100644
--- a/init/main.c
+++ b/init/main.c
@@ -476,7 +476,7 @@ static void __init mm_init(void)
mem_init();
kmem_cache_init();
percpu_init_late();
- pgtable_cache_init();
+ pgtable_init();
vmalloc_init();
}
diff --git a/mm/memory.c b/mm/memory.c
index f6cd03e4dec6..8e764473d0ff 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4272,6 +4272,13 @@ void copy_user_huge_page(struct page *dst, struct page *src,
#endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
#if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
+static struct kmem_cache *page_ptl_cachep;
+void __init ptlock_cache_init(void)
+{
+ page_ptl_cachep = kmem_cache_create("page->ptl", sizeof(spinlock_t), 0,
+ SLAB_PANIC, NULL);
+}
+
bool ptlock_alloc(struct page *page)
{
spinlock_t *ptl;
--
1.8.4.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case
2013-11-07 14:14 [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Kirill A. Shutemov
2013-11-07 14:14 ` [PATCH 2/2] mm: create a separate slab for page->ptl allocation Kirill A. Shutemov
@ 2013-11-15 22:01 ` Tony Luck
2013-11-15 22:09 ` Tony Luck
1 sibling, 1 reply; 8+ messages in thread
From: Tony Luck @ 2013-11-15 22:01 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar,
Linux Kernel Mailing List, linux-mm
On Thu, Nov 7, 2013 at 6:14 AM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index e8ca97b5c386..578782ef6ae1 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -11,6 +11,7 @@
> #include <linux/kbuild.h>
> #include <linux/page_cgroup.h>
> #include <linux/log2.h>
> +#include <linux/spinlock.h>
>
> void foo(void)
> {
> @@ -21,5 +22,6 @@ void foo(void)
> #ifdef CONFIG_SMP
> DEFINE(NR_CPUS_BITS, ilog2(CONFIG_NR_CPUS));
> #endif
> + DEFINE(BLOATED_SPINLOCKS, sizeof(spinlock_t) > sizeof(int));
> /* End of constants */
> }
This patch arrived in Linus' tree today - and broke the ia64 build :-(
CC kernel/bounds.s
In file included from
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:9,
from include/linux/thread_info.h:54,
from include/asm-generic/preempt.h:4,
from arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:18,
from include/linux/spinlock.h:50,
from kernel/bounds.c:14:
/home/aegl/generic-smp/arch/ia64/include/asm/asm-offsets.h:1:35:
error: generated/asm-offsets.h: No such file or directory
In file included from include/linux/thread_info.h:54,
from include/asm-generic/preempt.h:4,
from arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:18,
from include/linux/spinlock.h:50,
from kernel/bounds.c:14:
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h: In
function 'set_restore_sigmask':
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:138: error:
'IA64_TASK_SIZE' undeclared (first use in this function)
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:138: error:
(Each undeclared identifier is reported only once
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:138: error:
for each function it appears in.)
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h: In
function 'clear_restore_sigmask':
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:144: error:
'IA64_TASK_SIZE' undeclared (first use in this function)
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h: In
function 'test_restore_sigmask':
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:148: error:
'IA64_TASK_SIZE' undeclared (first use in this function)
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h: In
function 'test_and_clear_restore_sigmask':
/home/aegl/generic-smp/arch/ia64/include/asm/thread_info.h:152: error:
'IA64_TASK_SIZE' undeclared (first use in this function)
In file included from arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:18,
from include/linux/spinlock.h:50,
from kernel/bounds.c:14:
include/asm-generic/preempt.h: In function 'preempt_count':
include/asm-generic/preempt.h:12: error: 'IA64_TASK_SIZE' undeclared
(first use in this function)
include/asm-generic/preempt.h: In function 'preempt_count_ptr':
include/asm-generic/preempt.h:17: error: 'IA64_TASK_SIZE' undeclared
(first use in this function)
make[1]: *** [kernel/bounds.s] Error 1
make: *** [prepare0] Error 2
make: *** Waiting for unfinished jobs....
The problem is somewhat circular: IA64_TASK_SIZE will later be defined
by asm-offsets.h,
but we haven't even tried to generate that yet.
My "grep" skills are failing to find the Makefile that decides it wants to build
kernel/bounds.s so early :-(
-Tony
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case
2013-11-15 22:01 ` [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Tony Luck
@ 2013-11-15 22:09 ` Tony Luck
2013-11-15 23:15 ` Kirill A. Shutemov
0 siblings, 1 reply; 8+ messages in thread
From: Tony Luck @ 2013-11-15 22:09 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar,
Linux Kernel Mailing List, linux-mm
On Fri, Nov 15, 2013 at 2:01 PM, Tony Luck <tony.luck@gmail.com> wrote:
> My "grep" skills are failing to find the Makefile that decides it wants to build
> kernel/bounds.s so early :-(
... and then seconds later I found it in the top-level "Kbuild" file.
But it looks ugly ... comment says we might need this before asm-offsets.h
(which is the reverse of the ia64 case ... we need asm-offsets.h before we
can make bounds.s).
Help!
-Tony
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case
2013-11-15 22:09 ` Tony Luck
@ 2013-11-15 23:15 ` Kirill A. Shutemov
2013-11-15 23:33 ` Tony Luck
0 siblings, 1 reply; 8+ messages in thread
From: Kirill A. Shutemov @ 2013-11-15 23:15 UTC (permalink / raw)
To: Tony Luck
Cc: Kirill A. Shutemov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
Linux Kernel Mailing List, linux-mm
Tony Luck wrote:
> On Fri, Nov 15, 2013 at 2:01 PM, Tony Luck <tony.luck@gmail.com> wrote:
> Help!
Could you try this:
>From 661b6057c435fbae265b394b277a0b16bf02d255 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Sat, 16 Nov 2013 01:09:17 +0200
Subject: [PATCH] kernel/bounds: avoid circular dependencies in generated
headers
<linux/spinlock.h> has havy dependencies on other header files.
It trigger circular dependencies in generated headers on IA64, at least:
CC kernel/bounds.s
In file included from /home/space/kas/git/public/linux/arch/ia64/include/asm/thread_info.h:9:0,
from include/linux/thread_info.h:54,
from include/asm-generic/preempt.h:4,
from arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:18,
from include/linux/spinlock.h:50,
from kernel/bounds.c:14:
/home/space/kas/git/public/linux/arch/ia64/include/asm/asm-offsets.h:1:35: fatal error: generated/asm-offsets.h: No such file or directory
compilation terminated.
Let's replace <linux/spinlock.h> with <linux/spinlock_types.h>, it's
enough to find out size of spinlock_t.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
kernel/bounds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bounds.c b/kernel/bounds.c
index 578782ef6ae1..5253204afdca 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -11,7 +11,7 @@
#include <linux/kbuild.h>
#include <linux/page_cgroup.h>
#include <linux/log2.h>
-#include <linux/spinlock.h>
+#include <linux/spinlock_types.h>
void foo(void)
{
--
Kirill A. Shutemov
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case
2013-11-15 23:15 ` Kirill A. Shutemov
@ 2013-11-15 23:33 ` Tony Luck
0 siblings, 0 replies; 8+ messages in thread
From: Tony Luck @ 2013-11-15 23:33 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar,
Linux Kernel Mailing List, linux-mm
On Fri, Nov 15, 2013 at 3:15 PM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> -#include <linux/spinlock.h>
> +#include <linux/spinlock_types.h>
Awesome!
Tested-by: Tony Luck <tony.luck@intel.com>
-Tony
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: create a separate slab for page->ptl allocation
2013-11-07 14:14 ` [PATCH 2/2] mm: create a separate slab for page->ptl allocation Kirill A. Shutemov
@ 2013-11-16 20:43 ` Geert Uytterhoeven
2013-11-18 3:04 ` Stephen Rothwell
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-11-16 20:43 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar, linux-kernel, Linux MM
On Thu, Nov 7, 2013 at 3:14 PM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> +static inline void pgtable_init(void)
> +{
> + ptlock_cache_init();
> + pgtable_cache_init();
> +}
sparc64defconfig:
include/linux/mm.h:1391:2: error: implicit declaration of function
'pgtable_cache_init' [-Werror=implicit-function-declaration]
arch/sparc/include/asm/pgtable_64.h:978:13: error: conflicting types
for 'pgtable_cache_init' [-Werror]
http://kisskb.ellerman.id.au/kisskb/buildresult/10040274/
Has this been in -next?
Probably it needs <asm/pgtable.h>.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] mm: create a separate slab for page->ptl allocation
2013-11-16 20:43 ` Geert Uytterhoeven
@ 2013-11-18 3:04 ` Stephen Rothwell
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Rothwell @ 2013-11-18 3:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kirill A. Shutemov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
linux-kernel, Linux MM
[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]
Hi Geert,
On Sat, 16 Nov 2013 21:43:32 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On Thu, Nov 7, 2013 at 3:14 PM, Kirill A. Shutemov
> <kirill.shutemov@linux.intel.com> wrote:
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
>
> > +static inline void pgtable_init(void)
> > +{
> > + ptlock_cache_init();
> > + pgtable_cache_init();
> > +}
>
> sparc64defconfig:
>
> include/linux/mm.h:1391:2: error: implicit declaration of function
> 'pgtable_cache_init' [-Werror=implicit-function-declaration]
> arch/sparc/include/asm/pgtable_64.h:978:13: error: conflicting types
> for 'pgtable_cache_init' [-Werror]
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/10040274/
>
> Has this been in -next?
No, it hasn't :-(
> Probably it needs <asm/pgtable.h>.
Actually it is because on sparc64, asm/tlbflush_64.h includes linux/mm.h
(and asm/pgtable.h -> asm/pgtable_64.h -> asm/tlbflush.h ->
asm/tlbflush_64.h)
(see my other error report and I have reverted that commit from
linux-next today.)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-11-18 3:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-07 14:14 [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Kirill A. Shutemov
2013-11-07 14:14 ` [PATCH 2/2] mm: create a separate slab for page->ptl allocation Kirill A. Shutemov
2013-11-16 20:43 ` Geert Uytterhoeven
2013-11-18 3:04 ` Stephen Rothwell
2013-11-15 22:01 ` [PATCH 1/2] mm: Properly separate the bloated ptl from the regular case Tony Luck
2013-11-15 22:09 ` Tony Luck
2013-11-15 23:15 ` Kirill A. Shutemov
2013-11-15 23:33 ` Tony Luck
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).