subject:"\[PATCH 08\/10\] locking\/qspinlock\: Merge struct _

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

2018-04-06 Thread Boqun Feng

On Thu, Apr 05, 2018 at 05:59:05PM +0100, Will Deacon wrote:
> struct __qspinlock provides a handy union of fields so that
> subcomponents of the lockword can be accessed by name, without having to
> manage shifts and masks explicitly and take endianness into account.
> 
> This is useful in qspinlock.h and also potentially in arch headers, so
> move the struct __qspinlock into struct qspinlock and kill the extra
> definition.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 

As I said in the IRC, it's glad to see such a merge ;-)

Acked-by: Boqun Feng 

Regards,
Boqun

> ---
>  arch/x86/include/asm/qspinlock.h  |  2 +-
>  arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
>  include/asm-generic/qspinlock_types.h | 32 +++--
>  kernel/locking/qspinlock.c| 46 
> ++-
>  kernel/locking/qspinlock_paravirt.h   | 34 ---
>  5 files changed, 46 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/x86/include/asm/qspinlock.h 
> b/arch/x86/include/asm/qspinlock.h
> index 5e16b5d40d32..90b0b0ed8161 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -16,7 +16,7 @@
>   */
>  static inline void native_queued_spin_unlock(struct qspinlock *lock)
>  {
> - smp_store_release((u8 *)lock, 0);
> + smp_store_release(>locked, 0);
>  }
>  
>  #ifdef CONFIG_PARAVIRT_SPINLOCKS
> diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
> b/arch/x86/include/asm/qspinlock_paravirt.h
> index 923307ea11c7..9ef5ee03d2d7 100644
> --- a/arch/x86/include/asm/qspinlock_paravirt.h
> +++ b/arch/x86/include/asm/qspinlock_paravirt.h
> @@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
>   *
>   * void __pv_queued_spin_unlock(struct qspinlock *lock)
>   * {
> - *   struct __qspinlock *l = (void *)lock;
> - *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
> + *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
>   *
>   *   if (likely(lockval == _Q_LOCKED_VAL))
>   *   return;
> diff --git a/include/asm-generic/qspinlock_types.h 
> b/include/asm-generic/qspinlock_types.h
> index 034acd0c4956..0763f065b975 100644
> --- a/include/asm-generic/qspinlock_types.h
> +++ b/include/asm-generic/qspinlock_types.h
> @@ -29,13 +29,41 @@
>  #endif
>  
>  typedef struct qspinlock {
> - atomic_tval;
> + union {
> + atomic_t val;
> +
> + /*
> +  * By using the whole 2nd least significant byte for the
> +  * pending bit, we can allow better optimization of the lock
> +  * acquisition for the pending bit holder.
> +  */
> +#ifdef __LITTLE_ENDIAN
> + struct {
> + u8  locked;
> + u8  pending;
> + };
> + struct {
> + u16 locked_pending;
> + u16 tail;
> + };
> +#else
> + struct {
> + u16 tail;
> + u16 locked_pending;
> + };
> + struct {
> + u8  reserved[2];
> + u8  pending;
> + u8  locked;
> + };
> +#endif
> + };
>  } arch_spinlock_t;
>  
>  /*
>   * Initializier
>   */
> -#define  __ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
> +#define  __ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
>  
>  /*
>   * Bitfields in the atomic value:
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index c8b57d375b49..3ad8786a47e2 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock 
> *decode_tail(u32 tail)
>  
>  #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
>  
> -/*
> - * By using the whole 2nd least significant byte for the pending bit, we
> - * can allow better optimization of the lock acquisition for the pending
> - * bit holder.
> - *
> - * This internal structure is also used by the set_locked function which
> - * is not restricted to _Q_PENDING_BITS == 8.
> - */
> -struct __qspinlock {
> - union {
> - atomic_t val;
> -#ifdef __LITTLE_ENDIAN
> - struct {
> - u8  locked;
> - u8  pending;
> - };
> - struct {
> - u16 locked_pending;
> - u16 tail;
> - };
> -#else
> - struct {
> - u16 tail;
> - u16 locked_pending;
> - };
> - struct {
> - u8  reserved[2];
> - u8  pending;
> - u8  locked;
> - };
> -#endif
> - };
> -};
> -
>

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

2018-04-06 Thread Boqun Feng

On Thu, Apr 05, 2018 at 05:59:05PM +0100, Will Deacon wrote:
> struct __qspinlock provides a handy union of fields so that
> subcomponents of the lockword can be accessed by name, without having to
> manage shifts and masks explicitly and take endianness into account.
> 
> This is useful in qspinlock.h and also potentially in arch headers, so
> move the struct __qspinlock into struct qspinlock and kill the extra
> definition.
> 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Signed-off-by: Will Deacon 

As I said in the IRC, it's glad to see such a merge ;-)

Acked-by: Boqun Feng 

Regards,
Boqun

> ---
>  arch/x86/include/asm/qspinlock.h  |  2 +-
>  arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
>  include/asm-generic/qspinlock_types.h | 32 +++--
>  kernel/locking/qspinlock.c| 46 
> ++-
>  kernel/locking/qspinlock_paravirt.h   | 34 ---
>  5 files changed, 46 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/x86/include/asm/qspinlock.h 
> b/arch/x86/include/asm/qspinlock.h
> index 5e16b5d40d32..90b0b0ed8161 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -16,7 +16,7 @@
>   */
>  static inline void native_queued_spin_unlock(struct qspinlock *lock)
>  {
> - smp_store_release((u8 *)lock, 0);
> + smp_store_release(>locked, 0);
>  }
>  
>  #ifdef CONFIG_PARAVIRT_SPINLOCKS
> diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
> b/arch/x86/include/asm/qspinlock_paravirt.h
> index 923307ea11c7..9ef5ee03d2d7 100644
> --- a/arch/x86/include/asm/qspinlock_paravirt.h
> +++ b/arch/x86/include/asm/qspinlock_paravirt.h
> @@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
>   *
>   * void __pv_queued_spin_unlock(struct qspinlock *lock)
>   * {
> - *   struct __qspinlock *l = (void *)lock;
> - *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
> + *   u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
>   *
>   *   if (likely(lockval == _Q_LOCKED_VAL))
>   *   return;
> diff --git a/include/asm-generic/qspinlock_types.h 
> b/include/asm-generic/qspinlock_types.h
> index 034acd0c4956..0763f065b975 100644
> --- a/include/asm-generic/qspinlock_types.h
> +++ b/include/asm-generic/qspinlock_types.h
> @@ -29,13 +29,41 @@
>  #endif
>  
>  typedef struct qspinlock {
> - atomic_tval;
> + union {
> + atomic_t val;
> +
> + /*
> +  * By using the whole 2nd least significant byte for the
> +  * pending bit, we can allow better optimization of the lock
> +  * acquisition for the pending bit holder.
> +  */
> +#ifdef __LITTLE_ENDIAN
> + struct {
> + u8  locked;
> + u8  pending;
> + };
> + struct {
> + u16 locked_pending;
> + u16 tail;
> + };
> +#else
> + struct {
> + u16 tail;
> + u16 locked_pending;
> + };
> + struct {
> + u8  reserved[2];
> + u8  pending;
> + u8  locked;
> + };
> +#endif
> + };
>  } arch_spinlock_t;
>  
>  /*
>   * Initializier
>   */
> -#define  __ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
> +#define  __ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
>  
>  /*
>   * Bitfields in the atomic value:
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index c8b57d375b49..3ad8786a47e2 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock 
> *decode_tail(u32 tail)
>  
>  #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
>  
> -/*
> - * By using the whole 2nd least significant byte for the pending bit, we
> - * can allow better optimization of the lock acquisition for the pending
> - * bit holder.
> - *
> - * This internal structure is also used by the set_locked function which
> - * is not restricted to _Q_PENDING_BITS == 8.
> - */
> -struct __qspinlock {
> - union {
> - atomic_t val;
> -#ifdef __LITTLE_ENDIAN
> - struct {
> - u8  locked;
> - u8  pending;
> - };
> - struct {
> - u16 locked_pending;
> - u16 tail;
> - };
> -#else
> - struct {
> - u16 tail;
> - u16 locked_pending;
> - };
> - struct {
> - u8  reserved[2];
> - u8  pending;
> - u8  locked;
> - };
> -#endif
> - };
> -};
> -
>  #if _Q_PENDING_BITS == 8
>  /**
>   * clear_pending_set_locked - take ownership and

[PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

2018-04-05 Thread Will Deacon

struct __qspinlock provides a handy union of fields so that
subcomponents of the lockword can be accessed by name, without having to
manage shifts and masks explicitly and take endianness into account.

This is useful in qspinlock.h and also potentially in arch headers, so
move the struct __qspinlock into struct qspinlock and kill the extra
definition.

Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Signed-off-by: Will Deacon 
---
 arch/x86/include/asm/qspinlock.h  |  2 +-
 arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
 include/asm-generic/qspinlock_types.h | 32 +++--
 kernel/locking/qspinlock.c| 46 ++-
 kernel/locking/qspinlock_paravirt.h   | 34 ---
 5 files changed, 46 insertions(+), 71 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 5e16b5d40d32..90b0b0ed8161 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -16,7 +16,7 @@
  */
 static inline void native_queued_spin_unlock(struct qspinlock *lock)
 {
-   smp_store_release((u8 *)lock, 0);
+   smp_store_release(>locked, 0);
 }
 
 #ifdef CONFIG_PARAVIRT_SPINLOCKS
diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
b/arch/x86/include/asm/qspinlock_paravirt.h
index 923307ea11c7..9ef5ee03d2d7 100644
--- a/arch/x86/include/asm/qspinlock_paravirt.h
+++ b/arch/x86/include/asm/qspinlock_paravirt.h
@@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
  *
  * void __pv_queued_spin_unlock(struct qspinlock *lock)
  * {
- * struct __qspinlock *l = (void *)lock;
- * u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
+ * u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
  *
  * if (likely(lockval == _Q_LOCKED_VAL))
  * return;
diff --git a/include/asm-generic/qspinlock_types.h 
b/include/asm-generic/qspinlock_types.h
index 034acd0c4956..0763f065b975 100644
--- a/include/asm-generic/qspinlock_types.h
+++ b/include/asm-generic/qspinlock_types.h
@@ -29,13 +29,41 @@
 #endif
 
 typedef struct qspinlock {
-   atomic_tval;
+   union {
+   atomic_t val;
+
+   /*
+* By using the whole 2nd least significant byte for the
+* pending bit, we can allow better optimization of the lock
+* acquisition for the pending bit holder.
+*/
+#ifdef __LITTLE_ENDIAN
+   struct {
+   u8  locked;
+   u8  pending;
+   };
+   struct {
+   u16 locked_pending;
+   u16 tail;
+   };
+#else
+   struct {
+   u16 tail;
+   u16 locked_pending;
+   };
+   struct {
+   u8  reserved[2];
+   u8  pending;
+   u8  locked;
+   };
+#endif
+   };
 } arch_spinlock_t;
 
 /*
  * Initializier
  */
-#define__ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
+#define__ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
 
 /*
  * Bitfields in the atomic value:
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index c8b57d375b49..3ad8786a47e2 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock *decode_tail(u32 
tail)
 
 #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
 
-/*
- * By using the whole 2nd least significant byte for the pending bit, we
- * can allow better optimization of the lock acquisition for the pending
- * bit holder.
- *
- * This internal structure is also used by the set_locked function which
- * is not restricted to _Q_PENDING_BITS == 8.
- */
-struct __qspinlock {
-   union {
-   atomic_t val;
-#ifdef __LITTLE_ENDIAN
-   struct {
-   u8  locked;
-   u8  pending;
-   };
-   struct {
-   u16 locked_pending;
-   u16 tail;
-   };
-#else
-   struct {
-   u16 tail;
-   u16 locked_pending;
-   };
-   struct {
-   u8  reserved[2];
-   u8  pending;
-   u8  locked;
-   };
-#endif
-   };
-};
-
 #if _Q_PENDING_BITS == 8
 /**
  * clear_pending_set_locked - take ownership and clear the pending bit.
@@ -159,9 +125,7 @@ struct __qspinlock {
  */
 static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
 {
-   struct __qspinlock *l = (void *)lock;
-
-   WRITE_ONCE(l->locked_pending, _Q_LOCKED_VAL);
+

[PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

2018-04-05 Thread Will Deacon

struct __qspinlock provides a handy union of fields so that
subcomponents of the lockword can be accessed by name, without having to
manage shifts and masks explicitly and take endianness into account.

This is useful in qspinlock.h and also potentially in arch headers, so
move the struct __qspinlock into struct qspinlock and kill the extra
definition.

Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Signed-off-by: Will Deacon 
---
 arch/x86/include/asm/qspinlock.h  |  2 +-
 arch/x86/include/asm/qspinlock_paravirt.h |  3 +-
 include/asm-generic/qspinlock_types.h | 32 +++--
 kernel/locking/qspinlock.c| 46 ++-
 kernel/locking/qspinlock_paravirt.h   | 34 ---
 5 files changed, 46 insertions(+), 71 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 5e16b5d40d32..90b0b0ed8161 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -16,7 +16,7 @@
  */
 static inline void native_queued_spin_unlock(struct qspinlock *lock)
 {
-   smp_store_release((u8 *)lock, 0);
+   smp_store_release(>locked, 0);
 }
 
 #ifdef CONFIG_PARAVIRT_SPINLOCKS
diff --git a/arch/x86/include/asm/qspinlock_paravirt.h 
b/arch/x86/include/asm/qspinlock_paravirt.h
index 923307ea11c7..9ef5ee03d2d7 100644
--- a/arch/x86/include/asm/qspinlock_paravirt.h
+++ b/arch/x86/include/asm/qspinlock_paravirt.h
@@ -22,8 +22,7 @@ PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath);
  *
  * void __pv_queued_spin_unlock(struct qspinlock *lock)
  * {
- * struct __qspinlock *l = (void *)lock;
- * u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
+ * u8 lockval = cmpxchg(>locked, _Q_LOCKED_VAL, 0);
  *
  * if (likely(lockval == _Q_LOCKED_VAL))
  * return;
diff --git a/include/asm-generic/qspinlock_types.h 
b/include/asm-generic/qspinlock_types.h
index 034acd0c4956..0763f065b975 100644
--- a/include/asm-generic/qspinlock_types.h
+++ b/include/asm-generic/qspinlock_types.h
@@ -29,13 +29,41 @@
 #endif
 
 typedef struct qspinlock {
-   atomic_tval;
+   union {
+   atomic_t val;
+
+   /*
+* By using the whole 2nd least significant byte for the
+* pending bit, we can allow better optimization of the lock
+* acquisition for the pending bit holder.
+*/
+#ifdef __LITTLE_ENDIAN
+   struct {
+   u8  locked;
+   u8  pending;
+   };
+   struct {
+   u16 locked_pending;
+   u16 tail;
+   };
+#else
+   struct {
+   u16 tail;
+   u16 locked_pending;
+   };
+   struct {
+   u8  reserved[2];
+   u8  pending;
+   u8  locked;
+   };
+#endif
+   };
 } arch_spinlock_t;
 
 /*
  * Initializier
  */
-#define__ARCH_SPIN_LOCK_UNLOCKED   { ATOMIC_INIT(0) }
+#define__ARCH_SPIN_LOCK_UNLOCKED   { .val = ATOMIC_INIT(0) }
 
 /*
  * Bitfields in the atomic value:
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index c8b57d375b49..3ad8786a47e2 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -114,40 +114,6 @@ static inline __pure struct mcs_spinlock *decode_tail(u32 
tail)
 
 #define _Q_LOCKED_PENDING_MASK (_Q_LOCKED_MASK | _Q_PENDING_MASK)
 
-/*
- * By using the whole 2nd least significant byte for the pending bit, we
- * can allow better optimization of the lock acquisition for the pending
- * bit holder.
- *
- * This internal structure is also used by the set_locked function which
- * is not restricted to _Q_PENDING_BITS == 8.
- */
-struct __qspinlock {
-   union {
-   atomic_t val;
-#ifdef __LITTLE_ENDIAN
-   struct {
-   u8  locked;
-   u8  pending;
-   };
-   struct {
-   u16 locked_pending;
-   u16 tail;
-   };
-#else
-   struct {
-   u16 tail;
-   u16 locked_pending;
-   };
-   struct {
-   u8  reserved[2];
-   u8  pending;
-   u8  locked;
-   };
-#endif
-   };
-};
-
 #if _Q_PENDING_BITS == 8
 /**
  * clear_pending_set_locked - take ownership and clear the pending bit.
@@ -159,9 +125,7 @@ struct __qspinlock {
  */
 static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
 {
-   struct __qspinlock *l = (void *)lock;
-
-   WRITE_ONCE(l->locked_pending, _Q_LOCKED_VAL);
+   WRITE_ONCE(lock->locked_pending, _Q_LOCKED_VAL);
 }
 
 /*
@@ -176,13 +140,11

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

Re: [PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

[PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

[PATCH 08/10] locking/qspinlock: Merge struct __qspinlock into struct qspinlock

4 matches

Site Navigation

Mail list logo

Footer information