diff mbox series

[1/3] powerpc/64s: Add lockdep for HPTE lock

Message ID 20221013230710.1987253-1-npiggin@gmail.com (mailing list archive)
State Accepted
Headers show
Series [1/3] powerpc/64s: Add lockdep for HPTE lock | expand

Commit Message

Nicholas Piggin Oct. 13, 2022, 11:07 p.m. UTC
Add lockdep annotation for the HPTE bit-spinlock. Modern systems don't
take the tlbie lock, so this shows up some of the same lockdep warnings
that were being reported by the ppc970. And they're not taken in exactly
the same places so this is nice to have in its own right.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/book3s64/hash_native.c | 42 +++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 7 deletions(-)

Comments

Guenter Roeck Oct. 14, 2022, 12:18 a.m. UTC | #1
On Fri, Oct 14, 2022 at 09:07:08AM +1000, Nicholas Piggin wrote:
> Add lockdep annotation for the HPTE bit-spinlock. Modern systems don't
> take the tlbie lock, so this shows up some of the same lockdep warnings
> that were being reported by the ppc970. And they're not taken in exactly
> the same places so this is nice to have in its own right.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  arch/powerpc/mm/book3s64/hash_native.c | 42 +++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
> index 623a7b7ab38b..02d0c210a1ce 100644
> --- a/arch/powerpc/mm/book3s64/hash_native.c
> +++ b/arch/powerpc/mm/book3s64/hash_native.c
> @@ -43,6 +43,29 @@
>  
>  static DEFINE_RAW_SPINLOCK(native_tlbie_lock);
>  
> +#ifdef CONFIG_LOCKDEP
> +static struct lockdep_map hpte_lock_map =
> +        STATIC_LOCKDEP_MAP_INIT("hpte_lock", &hpte_lock_map);
> +
> +static void acquire_hpte_lock(void)
> +{
> +	lock_map_acquire(&hpte_lock_map);
> +}
> +
> +static void release_hpte_lock(void)
> +{
> +	lock_map_release(&hpte_lock_map);
> +}
> +#else
> +static void acquire_hpte_lock(void)
> +{
> +}
> +
> +static void release_hpte_lock(void)
> +{
> +}
> +#endif
> +
>  static inline unsigned long  ___tlbie(unsigned long vpn, int psize,
>  						int apsize, int ssize)
>  {
> @@ -220,6 +243,7 @@ static inline void native_lock_hpte(struct hash_pte *hptep)
>  {
>  	unsigned long *word = (unsigned long *)&hptep->v;
>  
> +	acquire_hpte_lock();
>  	while (1) {
>  		if (!test_and_set_bit_lock(HPTE_LOCK_BIT, word))
>  			break;
> @@ -234,6 +258,7 @@ static inline void native_unlock_hpte(struct hash_pte *hptep)
>  {
>  	unsigned long *word = (unsigned long *)&hptep->v;
>  
> +	release_hpte_lock();
>  	clear_bit_unlock(HPTE_LOCK_BIT, word);
>  }
>  
> @@ -279,6 +304,7 @@ static long native_hpte_insert(unsigned long hpte_group, unsigned long vpn,
>  		hpte_v = hpte_old_to_new_v(hpte_v);
>  	}
>  
> +	release_hpte_lock();
>  	hptep->r = cpu_to_be64(hpte_r);
>  	/* Guarantee the second dword is visible before the valid bit */
>  	eieio();
> @@ -327,6 +353,7 @@ static long native_hpte_remove(unsigned long hpte_group)
>  		return -1;
>  
>  	/* Invalidate the hpte. NOTE: this also unlocks it */
> +	release_hpte_lock();
>  	hptep->v = 0;
>  
>  	return i;
> @@ -517,10 +544,11 @@ static void native_hpte_invalidate(unsigned long slot, unsigned long vpn,
>  		/* recheck with locks held */
>  		hpte_v = hpte_get_old_v(hptep);
>  
> -		if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID))
> +		if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID)) {
>  			/* Invalidate the hpte. NOTE: this also unlocks it */
> +			release_hpte_lock();
>  			hptep->v = 0;
> -		else
> +		} else
>  			native_unlock_hpte(hptep);
>  	}
>  	/*
> @@ -580,10 +608,8 @@ static void native_hugepage_invalidate(unsigned long vsid,
>  			hpte_v = hpte_get_old_v(hptep);
>  
>  			if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID)) {
> -				/*
> -				 * Invalidate the hpte. NOTE: this also unlocks it
> -				 */
> -
> +				/* Invalidate the hpte. NOTE: this also unlocks it */
> +				release_hpte_lock();
>  				hptep->v = 0;
>  			} else
>  				native_unlock_hpte(hptep);
> @@ -765,8 +791,10 @@ static void native_flush_hash_range(unsigned long number, int local)
>  
>  			if (!HPTE_V_COMPARE(hpte_v, want_v) || !(hpte_v & HPTE_V_VALID))
>  				native_unlock_hpte(hptep);
> -			else
> +			else {
> +				release_hpte_lock();
>  				hptep->v = 0;
> +			}
>  
>  		} pte_iterate_hashed_end();
>  	}
> -- 
> 2.37.2
>
Michael Ellerman Oct. 28, 2022, 11:49 a.m. UTC | #2
On Fri, 14 Oct 2022 09:07:08 +1000, Nicholas Piggin wrote:
> Add lockdep annotation for the HPTE bit-spinlock. Modern systems don't
> take the tlbie lock, so this shows up some of the same lockdep warnings
> that were being reported by the ppc970. And they're not taken in exactly
> the same places so this is nice to have in its own right.
> 
> 

Applied to powerpc/fixes.

[1/3] powerpc/64s: Add lockdep for HPTE lock
      https://git.kernel.org/powerpc/c/be83d5485da549d934ec65463ea831709f2827b1
[2/3] powerpc/64s: make HPTE lock and native_tlbie_lock irq-safe
      https://git.kernel.org/powerpc/c/35159b5717fa9c6031fdd6a2193c7a3dc717ce33
[3/3] powerpc/64s: make linear_map_hash_lock a raw spinlock
      https://git.kernel.org/powerpc/c/b12eb279ff552bd67c167b0fe701ae602aa7311e

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
index 623a7b7ab38b..02d0c210a1ce 100644
--- a/arch/powerpc/mm/book3s64/hash_native.c
+++ b/arch/powerpc/mm/book3s64/hash_native.c
@@ -43,6 +43,29 @@ 
 
 static DEFINE_RAW_SPINLOCK(native_tlbie_lock);
 
+#ifdef CONFIG_LOCKDEP
+static struct lockdep_map hpte_lock_map =
+        STATIC_LOCKDEP_MAP_INIT("hpte_lock", &hpte_lock_map);
+
+static void acquire_hpte_lock(void)
+{
+	lock_map_acquire(&hpte_lock_map);
+}
+
+static void release_hpte_lock(void)
+{
+	lock_map_release(&hpte_lock_map);
+}
+#else
+static void acquire_hpte_lock(void)
+{
+}
+
+static void release_hpte_lock(void)
+{
+}
+#endif
+
 static inline unsigned long  ___tlbie(unsigned long vpn, int psize,
 						int apsize, int ssize)
 {
@@ -220,6 +243,7 @@  static inline void native_lock_hpte(struct hash_pte *hptep)
 {
 	unsigned long *word = (unsigned long *)&hptep->v;
 
+	acquire_hpte_lock();
 	while (1) {
 		if (!test_and_set_bit_lock(HPTE_LOCK_BIT, word))
 			break;
@@ -234,6 +258,7 @@  static inline void native_unlock_hpte(struct hash_pte *hptep)
 {
 	unsigned long *word = (unsigned long *)&hptep->v;
 
+	release_hpte_lock();
 	clear_bit_unlock(HPTE_LOCK_BIT, word);
 }
 
@@ -279,6 +304,7 @@  static long native_hpte_insert(unsigned long hpte_group, unsigned long vpn,
 		hpte_v = hpte_old_to_new_v(hpte_v);
 	}
 
+	release_hpte_lock();
 	hptep->r = cpu_to_be64(hpte_r);
 	/* Guarantee the second dword is visible before the valid bit */
 	eieio();
@@ -327,6 +353,7 @@  static long native_hpte_remove(unsigned long hpte_group)
 		return -1;
 
 	/* Invalidate the hpte. NOTE: this also unlocks it */
+	release_hpte_lock();
 	hptep->v = 0;
 
 	return i;
@@ -517,10 +544,11 @@  static void native_hpte_invalidate(unsigned long slot, unsigned long vpn,
 		/* recheck with locks held */
 		hpte_v = hpte_get_old_v(hptep);
 
-		if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID))
+		if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID)) {
 			/* Invalidate the hpte. NOTE: this also unlocks it */
+			release_hpte_lock();
 			hptep->v = 0;
-		else
+		} else
 			native_unlock_hpte(hptep);
 	}
 	/*
@@ -580,10 +608,8 @@  static void native_hugepage_invalidate(unsigned long vsid,
 			hpte_v = hpte_get_old_v(hptep);
 
 			if (HPTE_V_COMPARE(hpte_v, want_v) && (hpte_v & HPTE_V_VALID)) {
-				/*
-				 * Invalidate the hpte. NOTE: this also unlocks it
-				 */
-
+				/* Invalidate the hpte. NOTE: this also unlocks it */
+				release_hpte_lock();
 				hptep->v = 0;
 			} else
 				native_unlock_hpte(hptep);
@@ -765,8 +791,10 @@  static void native_flush_hash_range(unsigned long number, int local)
 
 			if (!HPTE_V_COMPARE(hpte_v, want_v) || !(hpte_v & HPTE_V_VALID))
 				native_unlock_hpte(hptep);
-			else
+			else {
+				release_hpte_lock();
 				hptep->v = 0;
+			}
 
 		} pte_iterate_hashed_end();
 	}