When configured with pre-trained compression/decompression
dictionary support, zstd requires custom memory allocator,
which it calls internally from compression()/decompression()
routines.  That means allocation from atomic context (either
under entry spin-lock, or per-CPU local-lock or both).  Now,
with non-atomic zram read()/write(), those limitations are
relaxed and we can allow direct and indirect reclaim.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/backend_zstd.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/backend_zstd.c b/drivers/block/zram/backend_zstd.c
index 1184c0036f44..53431251ea62 100644
--- a/drivers/block/zram/backend_zstd.c
+++ b/drivers/block/zram/backend_zstd.c
@@ -24,19 +24,14 @@ struct zstd_params {
 /*
  * For C/D dictionaries we need to provide zstd with zstd_custom_mem,
  * which zstd uses internally to allocate/free memory when needed.
- *
- * This means that allocator.customAlloc() can be called from zcomp_compress()
- * under local-lock (per-CPU compression stream), in which case we must use
- * GFP_ATOMIC.
- *
- * Another complication here is that we can be configured as a swap device.
  */
 static void *zstd_custom_alloc(void *opaque, size_t size)
 {
-	if (!preemptible())
+	/* Technically this should not happen */
+	if (WARN_ON_ONCE(!preemptible()))
 		return kvzalloc(size, GFP_ATOMIC);
 
-	return kvzalloc(size, __GFP_KSWAPD_RECLAIM | __GFP_NOWARN);
+	return kvzalloc(size, GFP_NOIO | __GFP_NOWARN);
 }
 
 static void zstd_custom_free(void *opaque, void *address)
-- 
2.48.1.601.g30ceb7b040-goog

On 2025-02-22 07:25:45 [+0900], Sergey Senozhatsky wrote:
>  static void *zstd_custom_alloc(void *opaque, size_t size)
>  {
> -	if (!preemptible())
> +	/* Technically this should not happen */
> +	if (WARN_ON_ONCE(!preemptible()))
>  		return kvzalloc(size, GFP_ATOMIC);

This check works only on preemptible kernels.
If you run this on !PREEMPTIBLE kernels, preemptible() reports always 0
so that WARNING will always trigger there.

> -	return kvzalloc(size, __GFP_KSWAPD_RECLAIM | __GFP_NOWARN);
> +	return kvzalloc(size, GFP_NOIO | __GFP_NOWARN);
>  }
>  
>  static void zstd_custom_free(void *opaque, void *address)
> -- 
> 2.48.1.601.g30ceb7b040-goog
> 

Sebastian

On (25/02/24 10:10), Sebastian Andrzej Siewior wrote:
> On 2025-02-22 07:25:45 [+0900], Sergey Senozhatsky wrote:
> >  static void *zstd_custom_alloc(void *opaque, size_t size)
> >  {
> > -	if (!preemptible())
> > +	/* Technically this should not happen */
> > +	if (WARN_ON_ONCE(!preemptible()))
> >  		return kvzalloc(size, GFP_ATOMIC);
> 
> This check works only on preemptible kernels.

I'm not sure this is true.

> If you run this on !PREEMPTIBLE kernels, preemptible() reports always 0
> so that WARNING will always trigger there.

I thought that preemptible() depends on PREEMPT_COUNT, not on
PREEMPTIBLE, because even on !PREEMPTIBLE preempt-count still
holds hard/soft irq counts, etc.

I ran CONFIG_PREEMPT_NONE=y zram-zstd tests and didn't see any
warnings.

On 2025-02-25 13:42:55 [+0900], Sergey Senozhatsky wrote:
> On (25/02/24 10:10), Sebastian Andrzej Siewior wrote:
> > On 2025-02-22 07:25:45 [+0900], Sergey Senozhatsky wrote:
> > >  static void *zstd_custom_alloc(void *opaque, size_t size)
> > >  {
> > > -	if (!preemptible())
> > > +	/* Technically this should not happen */
> > > +	if (WARN_ON_ONCE(!preemptible()))
> > >  		return kvzalloc(size, GFP_ATOMIC);
> > 
> > This check works only on preemptible kernels.
> 
> I'm not sure this is true.
> 
> > If you run this on !PREEMPTIBLE kernels, preemptible() reports always 0
> > so that WARNING will always trigger there.
> 
> I thought that preemptible() depends on PREEMPT_COUNT, not on
> PREEMPTIBLE, because even on !PREEMPTIBLE preempt-count still
> holds hard/soft irq counts, etc.

Yes. The preempt count is always there to hold NMI/ HARDIRQ/ SOFTIRQ.
However only on a preemptible (that is with PREEMPT_COUNT) kernel
preempt_disable() does something. So on !PREEMPTIBLE kernel you don't
see spin_lock() or preempt_disable() reflect in preempt_count. Unless
you enable debugging which force this option into a non-preemptible
kernel.

> I ran CONFIG_PREEMPT_NONE=y zram-zstd tests and didn't see any
> warnings.

Sebastian

On (25/02/25 13:42), Sergey Senozhatsky wrote:
> On (25/02/24 10:10), Sebastian Andrzej Siewior wrote:
> > On 2025-02-22 07:25:45 [+0900], Sergey Senozhatsky wrote:
> > >  static void *zstd_custom_alloc(void *opaque, size_t size)
> > >  {
> > > +	if (WARN_ON_ONCE(!preemptible()))

Gone.