Re: [RFC PATCH] zram: avoid preemption with CPU-based compression backends

On Wed, Aug 05, 2026 at 08:55:45AM +0800, Barry Song (Xiaomi) wrote:
> Since commit 2efa9e9eb4db ("zram: permit preemption with active
> compression stream"), a major Android regression has been reported.
>
> We add an async flag (currently false for almost all backends) to
> indicate whether a backend is asynchronous. For synchronous
> backends, we use preempt_disable() in the !PREEMPT_RT case. A
> zram_zs_malloc() wrapper is provided to support a two-stage
> zs_malloc() path, allowing allocation to transition from a
> non-sleepable context to a sleepable context.

Hi Barry,

Thanks for working on this. The priority inversion issue on Android is
real and we've hit it too.

However, I think there's a bug in the zcomp_stream_put() change.

The new zcomp_stream_put() uses raw_cpu_ptr() to find the stream:

  void zcomp_stream_put(struct zcomp *comp)
  {
      struct zcomp_strm *zstrm = raw_cpu_ptr(comp->stream);
      if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_enable();
      mutex_unlock(&zstrm->lock);
  }

It relies on the thread still being on the same CPU where
zcomp_stream_get() locked the stream. In the normal path it works
fine, because preempt is disabled the entire time.

But zram_zs_malloc() breaks this assumption in the fallback path:

  static unsigned long zram_zs_malloc(...)
  {
      if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT)) {
          handle = zs_malloc(..., __GFP_KSWAPD_RECLAIM | ...);
          if (!IS_ERR_VALUE(handle))
              return handle;
          preempt_enable();        // preempt is now enabled
      }

      handle = zs_malloc(..., GFP_NOIO | ...);  // may sleep, may migrate
      if (!async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_disable(); // Preempt disabled, but it may work on another CPU
      return handle;
  }

If the first zs_malloc() fails, preempt is enabled and the second
zs_malloc(GFP_NOIO) can sleep. During this window the task can migrate
to another CPU. After preempt_disable(), we continue on the new CPU.

When zcomp_stream_put() is later called, raw_cpu_ptr() returns the new
CPU's stream, which is not the one that was originally locked. Which may
cause a mutex_unlock() on the wrong CPU.

The original code didn't have this problem because zcomp_stream_put()
took the zstrm pointer directly:

  void zcomp_stream_put(struct zcomp_strm *zstrm)
  {
      mutex_unlock(&zstrm->lock);
  }

The caller always passed the saved pointer from zcomp_stream_get(),
so regardless of CPU migration, the correct mutex was always unlocked.

I think the fix is to keep passing the zstrm pointer:

  void zcomp_stream_put(struct zcomp *comp, struct zcomp_strm *zstrm)
  {
      if (!comp->ops->async && !IS_ENABLED(CONFIG_PREEMPT_RT))
          preempt_enable();
      mutex_unlock(&zstrm->lock);
  }

All callers already have zstrm available. Then It works as expected.

Bo