All of lore.kernel.org
 help / color / mirror / Atom feed
* mm: ptl is not bloated if it fits in pointer
@ 2013-12-16  9:04 ` Hugh Dickins
  0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2013-12-16  9:04 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Peter Zijlstra, Linus Torvalds, Andrew Morton, linux-kernel, linux-mm

It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
to kmalloc eight bytes for an indirect page table lock: the lock needs
to fit in the space that a pointer to it would occupy, not into an int.

Signed-off-by: Hugh Dickins <hughd@google.com>
---

 kernel/bounds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 3.13-rc4/kernel/bounds.c	2013-11-22 15:40:37.452192638 -0800
+++ linux/kernel/bounds.c	2013-12-15 14:34:36.304485959 -0800
@@ -22,6 +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));
+	DEFINE(BLOATED_SPINLOCKS, sizeof(spinlock_t) > sizeof(spinlock_t *));
 	/* End of constants */
 }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* mm: ptl is not bloated if it fits in pointer
@ 2013-12-16  9:04 ` Hugh Dickins
  0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2013-12-16  9:04 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Peter Zijlstra, Linus Torvalds, Andrew Morton, linux-kernel, linux-mm

It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
to kmalloc eight bytes for an indirect page table lock: the lock needs
to fit in the space that a pointer to it would occupy, not into an int.

Signed-off-by: Hugh Dickins <hughd@google.com>
---

 kernel/bounds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 3.13-rc4/kernel/bounds.c	2013-11-22 15:40:37.452192638 -0800
+++ linux/kernel/bounds.c	2013-12-15 14:34:36.304485959 -0800
@@ -22,6 +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));
+	DEFINE(BLOATED_SPINLOCKS, sizeof(spinlock_t) > sizeof(spinlock_t *));
 	/* End of constants */
 }

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
  2013-12-16  9:04 ` Hugh Dickins
@ 2013-12-16 10:04   ` Peter Zijlstra
  -1 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2013-12-16 10:04 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Kirill A. Shutemov, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm

On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> to kmalloc eight bytes for an indirect page table lock: the lock needs
> to fit in the space that a pointer to it would occupy, not into an int.

Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
bigger than that is bloated.

For the page-frame case we do indeed not care about the strict 32bit but
more about not being larger than a pointer, however there are already
other users.

See for instance include/linux/lockref.h and lib/lockref.c, they very
much require the spinlock to be 32bit and the below would break that.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
@ 2013-12-16 10:04   ` Peter Zijlstra
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2013-12-16 10:04 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Kirill A. Shutemov, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm

On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> to kmalloc eight bytes for an indirect page table lock: the lock needs
> to fit in the space that a pointer to it would occupy, not into an int.

Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
bigger than that is bloated.

For the page-frame case we do indeed not care about the strict 32bit but
more about not being larger than a pointer, however there are already
other users.

See for instance include/linux/lockref.h and lib/lockref.c, they very
much require the spinlock to be 32bit and the below would break that.

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
  2013-12-16  9:04 ` Hugh Dickins
@ 2013-12-16 10:12   ` Peter Zijlstra
  -1 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2013-12-16 10:12 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Kirill A. Shutemov, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm

On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures

So yes that's unfortunate, but why are people using that
GENERIC_LOCKBREAK stuff to begin with? Its atrocious, a much better path
would be to remove it.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
@ 2013-12-16 10:12   ` Peter Zijlstra
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2013-12-16 10:12 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Kirill A. Shutemov, Linus Torvalds, Andrew Morton, linux-kernel,
	linux-mm

On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures

So yes that's unfortunate, but why are people using that
GENERIC_LOCKBREAK stuff to begin with? Its atrocious, a much better path
would be to remove it.

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
  2013-12-16 10:04   ` Peter Zijlstra
@ 2013-12-16 13:50     ` Kirill A. Shutemov
  -1 siblings, 0 replies; 10+ messages in thread
From: Kirill A. Shutemov @ 2013-12-16 13:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Hugh Dickins, Kirill A. Shutemov, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm

Peter Zijlstra wrote:
> On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> > It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> > to kmalloc eight bytes for an indirect page table lock: the lock needs
> > to fit in the space that a pointer to it would occupy, not into an int.
> 
> Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
> bigger than that is bloated.
> 
> For the page-frame case we do indeed not care about the strict 32bit but
> more about not being larger than a pointer, however there are already
> other users.
> 
> See for instance include/linux/lockref.h and lib/lockref.c, they very
> much require the spinlock to be 32bit and the below would break that.

What about this instead? Smoke-tested.

>From d0243ba6bf462a7c0ae4290e1c0dbdae22eaa2a6 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Mon, 16 Dec 2013 15:36:31 +0200
Subject: [PATCH] mm: do not allocate page->ptl dynamically, if spinlock_t fits
 to long

In struct page we have enough space to fit long-size page->ptl
there, but we use dynamically-allocated page->ptl if
size(spinlock_t) > sizeof(int). It hurts 64-bit architectures with
CONFIG_GENERIC_LOCKBREAK, where sizeof(spinlock_t) == 8, but it
easily fits into struct page.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 include/linux/lockref.h  | 2 +-
 include/linux/mm.h       | 6 +++---
 include/linux/mm_types.h | 3 ++-
 kernel/bounds.c          | 2 +-
 mm/memory.c              | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/lockref.h b/include/linux/lockref.h
index c8929c3832db..4bfde0e99ed5 100644
--- a/include/linux/lockref.h
+++ b/include/linux/lockref.h
@@ -19,7 +19,7 @@
 
 #define USE_CMPXCHG_LOCKREF \
 	(IS_ENABLED(CONFIG_ARCH_USE_CMPXCHG_LOCKREF) && \
-	 IS_ENABLED(CONFIG_SMP) && !BLOATED_SPINLOCKS)
+	 IS_ENABLED(CONFIG_SMP) && SPINLOCK_SIZE <= 4)
 
 struct lockref {
 	union {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1cedd000cf29..35527173cf50 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1317,7 +1317,7 @@ 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
-#if BLOATED_SPINLOCKS
+#if ALLOC_SPLIT_PTLOCKS
 extern bool ptlock_alloc(struct page *page);
 extern void ptlock_free(struct page *page);
 
@@ -1325,7 +1325,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
 {
 	return page->ptl;
 }
-#else /* BLOATED_SPINLOCKS */
+#else /* ALLOC_SPLIT_PTLOCKS */
 static inline bool ptlock_alloc(struct page *page)
 {
 	return true;
@@ -1339,7 +1339,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
 {
 	return &page->ptl;
 }
-#endif /* BLOATED_SPINLOCKS */
+#endif /* ALLOC_SPLIT_PTLOCKS */
 
 static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
 {
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index bd299418a934..494b328c2a61 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -26,6 +26,7 @@ struct address_space;
 #define USE_SPLIT_PTE_PTLOCKS	(NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
 #define USE_SPLIT_PMD_PTLOCKS	(USE_SPLIT_PTE_PTLOCKS && \
 		IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
+#define ALLOC_SPLIT_PTLOCKS	(SPINLOCK_SIZE > BITS_PER_LONG/8)
 
 /*
  * Each physical page in the system has a struct page associated with
@@ -155,7 +156,7 @@ struct page {
 						 * system if PG_buddy is set.
 						 */
 #if USE_SPLIT_PTE_PTLOCKS
-#if BLOATED_SPINLOCKS
+#if ALLOC_SPLIT_PTLOCKS
 		spinlock_t *ptl;
 #else
 		spinlock_t ptl;
diff --git a/kernel/bounds.c b/kernel/bounds.c
index 5253204afdca..9fd4246b04b8 100644
--- a/kernel/bounds.c
+++ b/kernel/bounds.c
@@ -22,6 +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));
+	DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
 	/* End of constants */
 }
diff --git a/mm/memory.c b/mm/memory.c
index 5d9025f3b3e1..b6e211b779d0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4271,7 +4271,7 @@ void copy_user_huge_page(struct page *dst, struct page *src,
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
 
-#if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
+#if ALLOC_SPLIT_PTLOCKS
 bool ptlock_alloc(struct page *page)
 {
 	spinlock_t *ptl;
-- 
 Kirill A. Shutemov

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
@ 2013-12-16 13:50     ` Kirill A. Shutemov
  0 siblings, 0 replies; 10+ messages in thread
From: Kirill A. Shutemov @ 2013-12-16 13:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Hugh Dickins, Kirill A. Shutemov, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm

Peter Zijlstra wrote:
> On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> > It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> > to kmalloc eight bytes for an indirect page table lock: the lock needs
> > to fit in the space that a pointer to it would occupy, not into an int.
> 
> Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
> bigger than that is bloated.
> 
> For the page-frame case we do indeed not care about the strict 32bit but
> more about not being larger than a pointer, however there are already
> other users.
> 
> See for instance include/linux/lockref.h and lib/lockref.c, they very
> much require the spinlock to be 32bit and the below would break that.

What about this instead? Smoke-tested.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
  2013-12-16 13:50     ` Kirill A. Shutemov
@ 2013-12-17  0:29       ` Hugh Dickins
  -1 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2013-12-17  0:29 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Peter Zijlstra, Hugh Dickins, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm

On Mon, 16 Dec 2013, Kirill A. Shutemov wrote:
> Peter Zijlstra wrote:
> > On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> > > It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> > > to kmalloc eight bytes for an indirect page table lock: the lock needs
> > > to fit in the space that a pointer to it would occupy, not into an int.
> > 
> > Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
> > bigger than that is bloated.
> > 
> > For the page-frame case we do indeed not care about the strict 32bit but
> > more about not being larger than a pointer, however there are already
> > other users.
> > 
> > See for instance include/linux/lockref.h and lib/lockref.c, they very
> > much require the spinlock to be 32bit and the below would break that.

Whoops, yes indeed, I completely missed that BLOATED_SPINLOCKS is being
used for two different purposes, which have two different constraints.

> 
> What about this instead? Smoke-tested.

Excellent: I haven't tested it at all, but this patch looks to me
exactly how it should be done - thanks.

> 
> From d0243ba6bf462a7c0ae4290e1c0dbdae22eaa2a6 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Mon, 16 Dec 2013 15:36:31 +0200
> Subject: [PATCH] mm: do not allocate page->ptl dynamically, if spinlock_t fits
>  to long
> 
> In struct page we have enough space to fit long-size page->ptl
> there, but we use dynamically-allocated page->ptl if
> size(spinlock_t) > sizeof(int). It hurts 64-bit architectures with
> CONFIG_GENERIC_LOCKBREAK, where sizeof(spinlock_t) == 8, but it
> easily fits into struct page.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Acked-by: Hugh Dickins <hughd@google.com>

> ---
>  include/linux/lockref.h  | 2 +-
>  include/linux/mm.h       | 6 +++---
>  include/linux/mm_types.h | 3 ++-
>  kernel/bounds.c          | 2 +-
>  mm/memory.c              | 2 +-
>  5 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/lockref.h b/include/linux/lockref.h
> index c8929c3832db..4bfde0e99ed5 100644
> --- a/include/linux/lockref.h
> +++ b/include/linux/lockref.h
> @@ -19,7 +19,7 @@
>  
>  #define USE_CMPXCHG_LOCKREF \
>  	(IS_ENABLED(CONFIG_ARCH_USE_CMPXCHG_LOCKREF) && \
> -	 IS_ENABLED(CONFIG_SMP) && !BLOATED_SPINLOCKS)
> +	 IS_ENABLED(CONFIG_SMP) && SPINLOCK_SIZE <= 4)
>  
>  struct lockref {
>  	union {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1cedd000cf29..35527173cf50 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1317,7 +1317,7 @@ 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
> -#if BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  extern bool ptlock_alloc(struct page *page);
>  extern void ptlock_free(struct page *page);
>  
> @@ -1325,7 +1325,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
>  {
>  	return page->ptl;
>  }
> -#else /* BLOATED_SPINLOCKS */
> +#else /* ALLOC_SPLIT_PTLOCKS */
>  static inline bool ptlock_alloc(struct page *page)
>  {
>  	return true;
> @@ -1339,7 +1339,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
>  {
>  	return &page->ptl;
>  }
> -#endif /* BLOATED_SPINLOCKS */
> +#endif /* ALLOC_SPLIT_PTLOCKS */
>  
>  static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
>  {
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index bd299418a934..494b328c2a61 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -26,6 +26,7 @@ struct address_space;
>  #define USE_SPLIT_PTE_PTLOCKS	(NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
>  #define USE_SPLIT_PMD_PTLOCKS	(USE_SPLIT_PTE_PTLOCKS && \
>  		IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
> +#define ALLOC_SPLIT_PTLOCKS	(SPINLOCK_SIZE > BITS_PER_LONG/8)
>  
>  /*
>   * Each physical page in the system has a struct page associated with
> @@ -155,7 +156,7 @@ struct page {
>  						 * system if PG_buddy is set.
>  						 */
>  #if USE_SPLIT_PTE_PTLOCKS
> -#if BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  		spinlock_t *ptl;
>  #else
>  		spinlock_t ptl;
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index 5253204afdca..9fd4246b04b8 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -22,6 +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));
> +	DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
>  	/* End of constants */
>  }
> diff --git a/mm/memory.c b/mm/memory.c
> index 5d9025f3b3e1..b6e211b779d0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4271,7 +4271,7 @@ void copy_user_huge_page(struct page *dst, struct page *src,
>  }
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
>  
> -#if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  bool ptlock_alloc(struct page *page)
>  {
>  	spinlock_t *ptl;
> -- 
>  Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: mm: ptl is not bloated if it fits in pointer
@ 2013-12-17  0:29       ` Hugh Dickins
  0 siblings, 0 replies; 10+ messages in thread
From: Hugh Dickins @ 2013-12-17  0:29 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Peter Zijlstra, Hugh Dickins, Linus Torvalds, Andrew Morton,
	linux-kernel, linux-mm

On Mon, 16 Dec 2013, Kirill A. Shutemov wrote:
> Peter Zijlstra wrote:
> > On Mon, Dec 16, 2013 at 01:04:13AM -0800, Hugh Dickins wrote:
> > > It's silly to force the 64-bit CONFIG_GENERIC_LOCKBREAK architectures
> > > to kmalloc eight bytes for an indirect page table lock: the lock needs
> > > to fit in the space that a pointer to it would occupy, not into an int.
> > 
> > Ah, no. A spinlock is very much assumed to be 32bit, any spinlock that's
> > bigger than that is bloated.
> > 
> > For the page-frame case we do indeed not care about the strict 32bit but
> > more about not being larger than a pointer, however there are already
> > other users.
> > 
> > See for instance include/linux/lockref.h and lib/lockref.c, they very
> > much require the spinlock to be 32bit and the below would break that.

Whoops, yes indeed, I completely missed that BLOATED_SPINLOCKS is being
used for two different purposes, which have two different constraints.

> 
> What about this instead? Smoke-tested.

Excellent: I haven't tested it at all, but this patch looks to me
exactly how it should be done - thanks.

> 
> From d0243ba6bf462a7c0ae4290e1c0dbdae22eaa2a6 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Mon, 16 Dec 2013 15:36:31 +0200
> Subject: [PATCH] mm: do not allocate page->ptl dynamically, if spinlock_t fits
>  to long
> 
> In struct page we have enough space to fit long-size page->ptl
> there, but we use dynamically-allocated page->ptl if
> size(spinlock_t) > sizeof(int). It hurts 64-bit architectures with
> CONFIG_GENERIC_LOCKBREAK, where sizeof(spinlock_t) == 8, but it
> easily fits into struct page.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Acked-by: Hugh Dickins <hughd@google.com>

> ---
>  include/linux/lockref.h  | 2 +-
>  include/linux/mm.h       | 6 +++---
>  include/linux/mm_types.h | 3 ++-
>  kernel/bounds.c          | 2 +-
>  mm/memory.c              | 2 +-
>  5 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/lockref.h b/include/linux/lockref.h
> index c8929c3832db..4bfde0e99ed5 100644
> --- a/include/linux/lockref.h
> +++ b/include/linux/lockref.h
> @@ -19,7 +19,7 @@
>  
>  #define USE_CMPXCHG_LOCKREF \
>  	(IS_ENABLED(CONFIG_ARCH_USE_CMPXCHG_LOCKREF) && \
> -	 IS_ENABLED(CONFIG_SMP) && !BLOATED_SPINLOCKS)
> +	 IS_ENABLED(CONFIG_SMP) && SPINLOCK_SIZE <= 4)
>  
>  struct lockref {
>  	union {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1cedd000cf29..35527173cf50 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1317,7 +1317,7 @@ 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
> -#if BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  extern bool ptlock_alloc(struct page *page);
>  extern void ptlock_free(struct page *page);
>  
> @@ -1325,7 +1325,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
>  {
>  	return page->ptl;
>  }
> -#else /* BLOATED_SPINLOCKS */
> +#else /* ALLOC_SPLIT_PTLOCKS */
>  static inline bool ptlock_alloc(struct page *page)
>  {
>  	return true;
> @@ -1339,7 +1339,7 @@ static inline spinlock_t *ptlock_ptr(struct page *page)
>  {
>  	return &page->ptl;
>  }
> -#endif /* BLOATED_SPINLOCKS */
> +#endif /* ALLOC_SPLIT_PTLOCKS */
>  
>  static inline spinlock_t *pte_lockptr(struct mm_struct *mm, pmd_t *pmd)
>  {
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index bd299418a934..494b328c2a61 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -26,6 +26,7 @@ struct address_space;
>  #define USE_SPLIT_PTE_PTLOCKS	(NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
>  #define USE_SPLIT_PMD_PTLOCKS	(USE_SPLIT_PTE_PTLOCKS && \
>  		IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
> +#define ALLOC_SPLIT_PTLOCKS	(SPINLOCK_SIZE > BITS_PER_LONG/8)
>  
>  /*
>   * Each physical page in the system has a struct page associated with
> @@ -155,7 +156,7 @@ struct page {
>  						 * system if PG_buddy is set.
>  						 */
>  #if USE_SPLIT_PTE_PTLOCKS
> -#if BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  		spinlock_t *ptl;
>  #else
>  		spinlock_t ptl;
> diff --git a/kernel/bounds.c b/kernel/bounds.c
> index 5253204afdca..9fd4246b04b8 100644
> --- a/kernel/bounds.c
> +++ b/kernel/bounds.c
> @@ -22,6 +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));
> +	DEFINE(SPINLOCK_SIZE, sizeof(spinlock_t));
>  	/* End of constants */
>  }
> diff --git a/mm/memory.c b/mm/memory.c
> index 5d9025f3b3e1..b6e211b779d0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4271,7 +4271,7 @@ void copy_user_huge_page(struct page *dst, struct page *src,
>  }
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
>  
> -#if USE_SPLIT_PTE_PTLOCKS && BLOATED_SPINLOCKS
> +#if ALLOC_SPLIT_PTLOCKS
>  bool ptlock_alloc(struct page *page)
>  {
>  	spinlock_t *ptl;
> -- 
>  Kirill A. Shutemov

--
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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-12-17  0:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16  9:04 mm: ptl is not bloated if it fits in pointer Hugh Dickins
2013-12-16  9:04 ` Hugh Dickins
2013-12-16 10:04 ` Peter Zijlstra
2013-12-16 10:04   ` Peter Zijlstra
2013-12-16 13:50   ` Kirill A. Shutemov
2013-12-16 13:50     ` Kirill A. Shutemov
2013-12-17  0:29     ` Hugh Dickins
2013-12-17  0:29       ` Hugh Dickins
2013-12-16 10:12 ` Peter Zijlstra
2013-12-16 10:12   ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.