[PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[Richard Chang]

In zs_page_migrate(), locks are acquired in the following order:
  1. write_lock(&pool->lock)
  2. spin_lock(&class->lock)
  3. zspage_write_trylock(zspage)

However, upon successful page migration, they were being released in
forward acquisition (FIFO) order:
  1. write_unlock(&pool->lock)
  2. spin_unlock(&class->lock)
  3. zspage_write_unlock(zspage)

Fix the unlocking order to release locks in strict reverse (LIFO)
order of acquisition:
  3. zspage_write_unlock(zspage)
  2. spin_unlock(&class->lock)
  1. write_unlock(&pool->lock)

Releasing locks in reverse order of acquisition adheres to standard
kernel locking hygiene, prevents potential lock ordering and lockdep
inconsistencies.

Signed-off-by: Richard Chang <richardycc@google.com>
---
 mm/zsmalloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 83f5820c45f9..b09299115886 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1772,9 +1772,9 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
 	 * Since we complete the data copy and set up new zspage structure,
 	 * it's okay to release migration_lock.
 	 */
-	write_unlock(&pool->lock);
-	spin_unlock(&class->lock);
 	zspage_write_unlock(zspage);
+	spin_unlock(&class->lock);
+	write_unlock(&pool->lock);
 
 	zpdesc_get(newzpdesc);
 	if (zpdesc_zone(newzpdesc) != zpdesc_zone(zpdesc)) {
-- 
2.55.0.229.g6434b31f56-goog

Re: [PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[Sergey Senozhatsky]

On (26/07/28 05:53), Richard Chang wrote:
> In zs_page_migrate(), locks are acquired in the following order:
>   1. write_lock(&pool->lock)
>   2. spin_lock(&class->lock)
>   3. zspage_write_trylock(zspage)
> 
> However, upon successful page migration, they were being released in
> forward acquisition (FIFO) order:
>   1. write_unlock(&pool->lock)
>   2. spin_unlock(&class->lock)
>   3. zspage_write_unlock(zspage)
> 
> Fix the unlocking order to release locks in strict reverse (LIFO)
> order of acquisition:
>   3. zspage_write_unlock(zspage)
>   2. spin_unlock(&class->lock)
>   1. write_unlock(&pool->lock)
> 
> Releasing locks in reverse order of acquisition adheres to standard
> kernel locking hygiene, prevents potential lock ordering and lockdep
> inconsistencies.
> 
> Signed-off-by: Richard Chang <richardycc@google.com>

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Re: [PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[Andrew Morton]

On Tue, 28 Jul 2026 05:53:33 +0000 Richard Chang <richardycc@google.com> wrote:

> In zs_page_migrate(), locks are acquired in the following order:
>   1. write_lock(&pool->lock)
>   2. spin_lock(&class->lock)
>   3. zspage_write_trylock(zspage)
> 
> However, upon successful page migration, they were being released in
> forward acquisition (FIFO) order:
>   1. write_unlock(&pool->lock)
>   2. spin_unlock(&class->lock)
>   3. zspage_write_unlock(zspage)
> 
> Fix the unlocking order to release locks in strict reverse (LIFO)
> order of acquisition:
>   3. zspage_write_unlock(zspage)
>   2. spin_unlock(&class->lock)
>   1. write_unlock(&pool->lock)
> 
> Releasing locks in reverse order of acquisition adheres to standard
> kernel locking hygiene, prevents potential lock ordering and lockdep
> inconsistencies.
> 

Thanks.

Does this actually fix a lockdep splat?  If so, is a link available?

Re: [PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[Richard Chang]

Hi Andrew,

On Wed, Jul 29, 2026 at 2:15 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> Thanks.
>
> Does this actually fix a lockdep splat?  If so, is a link available?
>

No, this does not fix an active lockdep splat or a reported runtime deadlock.
It is just a code hygiene to release locks in LIFO order.

Re: [PATCH] mm/zsmalloc: fix release order of locks in zs_page_migrate()

[Sergey Senozhatsky]

On (26/07/29 12:46), Richard Chang wrote:
> On Wed, Jul 29, 2026 at 2:15 AM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > Thanks.
> >
> > Does this actually fix a lockdep splat?  If so, is a link available?
> >
> 
> No, this does not fix an active lockdep splat or a reported runtime deadlock.
> It is just a code hygiene to release locks in LIFO order.

Right, surprisingly enough lockdep was never unhappy with the original
code (I run tests with CONFIG_PROVE_LOCKING=y.)