x86: spinlocks: define dummy __raw_spin_is_contended

x86: spinlocks: define dummy __raw_spin_is_contended

[Kyle McMartin]

From: Kyle McMartin <kyle@redhat.com>

Architectures other than mips and x86 are not using ticket
spinlocks. Therefore, the contention on the lock is meaningless,
since there is nobody known to be waiting on it (arguably /fairly/
unfair locks) Dummy it out to return 0 on other architectures.

Signed-off-by: Kyle McMartin <kyle@redhat.com>
---
 arch/mips/include/asm/spinlock.h |    1 +
 arch/x86/include/asm/paravirt.h  |    1 +
 arch/x86/include/asm/spinlock.h  |    1 +
 include/linux/spinlock.h         |    5 +++++
 4 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
index 1a1f320..0884947 100644
--- a/arch/mips/include/asm/spinlock.h
+++ b/arch/mips/include/asm/spinlock.h
@@ -51,6 +51,7 @@ static inline int __raw_spin_is_contended(raw_spinlock_t *lock)
 
 	return (((counters >> 14) - counters) & 0x1fff) > 1;
 }
+#define __raw_spin_is_contended	__raw_spin_is_contended
 
 static inline void __raw_spin_lock(raw_spinlock_t *lock)
 {
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index ba3e2ff..c09a141 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -1402,6 +1402,7 @@ static inline int __raw_spin_is_contended(struct raw_spinlock *lock)
 {
 	return PVOP_CALL1(int, pv_lock_ops.spin_is_contended, lock);
 }
+#define __raw_spin_is_contended	__raw_spin_is_contended
 
 static __always_inline void __raw_spin_lock(struct raw_spinlock *lock)
 {
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index d17c919..8247e94 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -245,6 +245,7 @@ static inline int __raw_spin_is_contended(raw_spinlock_t *lock)
 {
 	return __ticket_spin_is_contended(lock);
 }
+#define __raw_spin_is_contended	__raw_spin_is_contended
 
 static __always_inline void __raw_spin_lock(raw_spinlock_t *lock)
 {
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index e0c0fcc..a0c66a2 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -124,7 +124,12 @@ do {								\
 #ifdef CONFIG_GENERIC_LOCKBREAK
 #define spin_is_contended(lock) ((lock)->break_lock)
 #else
+
+#ifdef __raw_spin_is_contended
 #define spin_is_contended(lock)	__raw_spin_is_contended(&(lock)->raw_lock)
+#else
+#define spin_is_contended(lock)	(((void)(lock), 0))
+#endif /*__raw_spin_is_contended*/
 #endif
 
 /**
-- 
1.6.0.6

Re: x86: spinlocks: define dummy __raw_spin_is_contended

[Kyle McMartin]

CC-ing Linus, since he just released 2.6.29-rc4, which will fail to
build btrfs on anything besides mips or x86.

regards, Kyle

On Sun, Feb 08, 2009 at 01:03:41PM -0500, Kyle McMartin wrote:
> From: Kyle McMartin <kyle@redhat.com>
> 
> Architectures other than mips and x86 are not using ticket
> spinlocks. Therefore, the contention on the lock is meaningless,
> since there is nobody known to be waiting on it (arguably /fairly/
> unfair locks) Dummy it out to return 0 on other architectures.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> ---
>  arch/mips/include/asm/spinlock.h |    1 +
>  arch/x86/include/asm/paravirt.h  |    1 +
>  arch/x86/include/asm/spinlock.h  |    1 +
>  include/linux/spinlock.h         |    5 +++++
>  4 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/mips/include/asm/spinlock.h b/arch/mips/include/asm/spinlock.h
> index 1a1f320..0884947 100644
> --- a/arch/mips/include/asm/spinlock.h
> +++ b/arch/mips/include/asm/spinlock.h
> @@ -51,6 +51,7 @@ static inline int __raw_spin_is_contended(raw_spinlock_t *lock)
>  
>  	return (((counters >> 14) - counters) & 0x1fff) > 1;
>  }
> +#define __raw_spin_is_contended	__raw_spin_is_contended
>  
>  static inline void __raw_spin_lock(raw_spinlock_t *lock)
>  {
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index ba3e2ff..c09a141 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -1402,6 +1402,7 @@ static inline int __raw_spin_is_contended(struct raw_spinlock *lock)
>  {
>  	return PVOP_CALL1(int, pv_lock_ops.spin_is_contended, lock);
>  }
> +#define __raw_spin_is_contended	__raw_spin_is_contended
>  
>  static __always_inline void __raw_spin_lock(struct raw_spinlock *lock)
>  {
> diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
> index d17c919..8247e94 100644
> --- a/arch/x86/include/asm/spinlock.h
> +++ b/arch/x86/include/asm/spinlock.h
> @@ -245,6 +245,7 @@ static inline int __raw_spin_is_contended(raw_spinlock_t *lock)
>  {
>  	return __ticket_spin_is_contended(lock);
>  }
> +#define __raw_spin_is_contended	__raw_spin_is_contended
>  
>  static __always_inline void __raw_spin_lock(raw_spinlock_t *lock)
>  {
> diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
> index e0c0fcc..a0c66a2 100644
> --- a/include/linux/spinlock.h
> +++ b/include/linux/spinlock.h
> @@ -124,7 +124,12 @@ do {								\
>  #ifdef CONFIG_GENERIC_LOCKBREAK
>  #define spin_is_contended(lock) ((lock)->break_lock)
>  #else
> +
> +#ifdef __raw_spin_is_contended
>  #define spin_is_contended(lock)	__raw_spin_is_contended(&(lock)->raw_lock)
> +#else
> +#define spin_is_contended(lock)	(((void)(lock), 0))
> +#endif /*__raw_spin_is_contended*/
>  #endif
>  
>  /**
> -- 
> 1.6.0.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: x86: spinlocks: define dummy __raw_spin_is_contended

[Ingo Molnar]

* Kyle McMartin <kyle@infradead.org> wrote:

> CC-ing Linus, since he just released 2.6.29-rc4, which will fail to
> build btrfs on anything besides mips or x86.

Hm, i'm not particularly fond of the spin_is_contended() trickery done in:

/*
 * btrfs_search_slot uses this to decide if it should drop its locks
 * before doing something expensive like allocating free blocks for cow.
 */
int btrfs_path_lock_waiting(struct btrfs_path *path, int level)
{
        int i;
        struct extent_buffer *eb;

        for (i = level; i <= level + 1 && i < BTRFS_MAX_LEVEL; i++) {
                eb = path->nodes[i];
                if (!eb)
                        break;
                smp_mb();
                if (spin_is_contended(&eb->lock) ||
                    waitqueue_active(&eb->lock_wq))
                        return 1;
        }
        return 0;
}

It does not look particularly cheap either, if the number of levels is high.

I'd suggest we remove that hack please and live or die by what performance
the generic locking code gives us - and fix it when it causes problems,
instead of working it around locally?

As spin-mutexes have shown (queued up for .30), working on that level is far
more rewarding in terms of improving Linux as a whole.

	Ingo

Re: x86: spinlocks: define dummy __raw_spin_is_contended

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Kyle McMartin <kyle@infradead.org> wrote:
> 
> > CC-ing Linus, since he just released 2.6.29-rc4, which will fail to
> > build btrfs on anything besides mips or x86.
> 
> Hm, i'm not particularly fond of the spin_is_contended() trickery done in:

having said that, Kyle's patch is obviously correct nevertheless - the above
is not to be taken as an objection.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: x86: spinlocks: define dummy __raw_spin_is_contended

[Ralf Baechle]

On Sun, Feb 08, 2009 at 01:03:41PM -0500, Kyle McMartin wrote:

> From: Kyle McMartin <kyle@redhat.com>
> 
> Architectures other than mips and x86 are not using ticket
> spinlocks. Therefore, the contention on the lock is meaningless,
> since there is nobody known to be waiting on it (arguably /fairly/
> unfair locks) Dummy it out to return 0 on other architectures.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

Looks good.

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf