[patch] mm, zsmalloc: Convert zsmalloc_handle.lock to spinlock_t

[patch] mm, zsmalloc: Convert zsmalloc_handle.lock to spinlock_t

[Mike Galbraith]

local_lock_t becoming a synonym of spinlock_t had consequences for the RT
mods to zsmalloc, which were taking a mutex while holding a local_lock,
inspiring a lockdep "BUG: Invalid wait context" gripe.

Converting zsmalloc_handle.lock to a spinlock_t restored lockdep silence.

Signed-off-by: Mike Galbraith <efault@gmx.de>
---
 mm/zsmalloc.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -82,7 +82,7 @@

 struct zsmalloc_handle {
 	unsigned long addr;
-	struct mutex lock;
+	spinlock_t lock;
 };

 #define ZS_HANDLE_ALLOC_SIZE (sizeof(struct zsmalloc_handle))
@@ -370,7 +370,7 @@ static unsigned long cache_alloc_handle(
 	if (p) {
 		struct zsmalloc_handle *zh = p;

-		mutex_init(&zh->lock);
+		spin_lock_init(&zh->lock);
 	}
 #endif
 	return (unsigned long)p;
@@ -927,7 +927,7 @@ static inline int testpin_tag(unsigned l
 #ifdef CONFIG_PREEMPT_RT
 	struct zsmalloc_handle *zh = zs_get_pure_handle(handle);

-	return mutex_is_locked(&zh->lock);
+	return spin_is_locked(&zh->lock);
 #else
 	return bit_spin_is_locked(HANDLE_PIN_BIT, (unsigned long *)handle);
 #endif
@@ -938,7 +938,7 @@ static inline int trypin_tag(unsigned lo
 #ifdef CONFIG_PREEMPT_RT
 	struct zsmalloc_handle *zh = zs_get_pure_handle(handle);

-	return mutex_trylock(&zh->lock);
+	return spin_trylock(&zh->lock);
 #else
 	return bit_spin_trylock(HANDLE_PIN_BIT, (unsigned long *)handle);
 #endif
@@ -949,7 +949,7 @@ static void pin_tag(unsigned long handle
 #ifdef CONFIG_PREEMPT_RT
 	struct zsmalloc_handle *zh = zs_get_pure_handle(handle);

-	return mutex_lock(&zh->lock);
+	return spin_lock(&zh->lock);
 #else
 	bit_spin_lock(HANDLE_PIN_BIT, (unsigned long *)handle);
 #endif
@@ -960,7 +960,7 @@ static void unpin_tag(unsigned long hand
 #ifdef CONFIG_PREEMPT_RT
 	struct zsmalloc_handle *zh = zs_get_pure_handle(handle);

-	return mutex_unlock(&zh->lock);
+	return spin_unlock(&zh->lock);
 #else
 	bit_spin_unlock(HANDLE_PIN_BIT, (unsigned long *)handle);
 #endif

Re: [patch] mm, zsmalloc: Convert zsmalloc_handle.lock to spinlock_t

[Vlastimil Babka]

On 8/24/21 13:08, Mike Galbraith wrote:
> 
> local_lock_t becoming a synonym of spinlock_t had consequences for the RT
> mods to zsmalloc, which were taking a mutex while holding a local_lock,
> inspiring a lockdep "BUG: Invalid wait context" gripe.
> 
> Converting zsmalloc_handle.lock to a spinlock_t restored lockdep silence.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>

BTW, does anyone really run zswap on a RT system?

Re: [patch] mm, zsmalloc: Convert zsmalloc_handle.lock to spinlock_t

[Mike Galbraith]

On Tue, 2021-08-24 at 13:14 +0200, Vlastimil Babka wrote:
> On 8/24/21 13:08, Mike Galbraith wrote:
> >
> > local_lock_t becoming a synonym of spinlock_t had consequences for
> > the RT
> > mods to zsmalloc, which were taking a mutex while holding a
> > local_lock,
> > inspiring a lockdep "BUG: Invalid wait context" gripe.
> >
> > Converting zsmalloc_handle.lock to a spinlock_t restored lockdep
> > silence.
> >
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
>
> BTW, does anyone really run zswap on a RT system?

(eek, oh hell no;) I only run ltp's zram tests to give box something
non-routine to do.

	-Mike

Re: [patch] mm, zsmalloc: Convert zsmalloc_handle.lock to spinlock_t

[Sebastian Andrzej Siewior]

On 2021-08-24 13:14:52 [+0200], Vlastimil Babka wrote:
> On 8/24/21 13:08, Mike Galbraith wrote:
> > 
> > local_lock_t becoming a synonym of spinlock_t had consequences for the RT
> > mods to zsmalloc, which were taking a mutex while holding a local_lock,
> > inspiring a lockdep "BUG: Invalid wait context" gripe.
> > 
> > Converting zsmalloc_handle.lock to a spinlock_t restored lockdep silence.
> > 
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
> 
> BTW, does anyone really run zswap on a RT system?

The bit spinlock is problematic for RT and needs a replacement. Having a
page not available in RT is a no no. But then it might be a good
testcase if mlock() has been setup properly :) But other than that, I
can't imagine a real use case.

I'm going to apply that patch for the time being.

Sebastian