> -----Original Message-----
> From: Mike Galbraith [mailto:efa...@gmx.de]
> Sent: Sunday, December 20, 2020 8:48 PM
> To: Vitaly Wool <vitaly.w...@konsulko.com>; LKML
> <linux-kernel@vger.kernel.org>; linux-mm <linux...@kvack.org>
> Cc: Song Bao Hua (Barry Song) <song.bao....@hisilicon.com>; Sebastian Andrzej
> Siewior <bige...@linutronix.de>; Minchan Kim <minc...@kernel.org>; NitinGupta
> <ngu...@vflare.org>
> Subject: Re: [PATCH] zsmalloc: do not use bit_spin_lock
>
> On Sun, 2020-12-20 at 02:23 +0100, Mike Galbraith wrote:
> > On Sun, 2020-12-20 at 02:22 +0200, Vitaly Wool wrote:
> > > zsmalloc takes bit spinlock in its _map() callback and releases it
> > > only in unmap() which is unsafe and leads to zswap complaining
> > > about scheduling in atomic context.
> > >
> > > To fix that and to improve RT properties of zsmalloc, remove that
> > > bit spinlock completely and use a bit flag instead.
> >
> > It also does get_cpu_var() in map(), put_cpu_var() in unmap().
>
> That aside, the bit spinlock removal seems to hold up to beating in RT.
> I stripped out the RT changes to replace the bit spinlocks, applied the
> still needed atm might_sleep() fix, and ltp zram and zswap test are
> running in a loop with no signs that it's a bad idea, so I hope that
> makes it in (minus the preempt disabled spin which I whacked), as it
> makes zsmalloc markedly more RT friendly.
>
> RT changes go from:
> 1 file changed, 79 insertions(+), 6 deletions(-)
> to:
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
Sorry, would you like to show the change for
"8 insertions(+), 3 deletions(-)"?
BTW, your original patch looks not right as
crypto_wait_req(crypto_acomp_decompress()...)
can sleep too.
[copy from your original patch with comment]
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1258,20 +1258,20 @@ static int zswap_frontswap_load(unsigned
/* decompress */
dlen = PAGE_SIZE;
+ acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
+ mutex_lock(acomp_ctx->mutex);
src = zpool_map_handle(entry->pool->zpool, entry->handle, ZPOOL_MM_RO);
if (zpool_evictable(entry->pool->zpool))
src += sizeof(struct zswap_header);
- acomp_ctx = raw_cpu_ptr(entry->pool->acomp_ctx);
- mutex_lock(acomp_ctx->mutex);
sg_init_one(&input, src, entry->length);
sg_init_table(&output, 1);
sg_set_page(&output, page, PAGE_SIZE, 0);
acomp_request_set_params(acomp_ctx->req, &input, &output,
entry->length, dlen);
/*!!!!!!!!!!!!!!!!
* here crypto could sleep
!!!!!!!!!!!!!!*/
ret = crypto_wait_req(crypto_acomp_decompress(acomp_ctx->req),
&acomp_ctx->wait);
- mutex_unlock(acomp_ctx->mutex);
zpool_unmap_handle(entry->pool->zpool, entry->handle);
+ mutex_unlock(acomp_ctx->mutex);
BUG_ON(ret);
freeentry:
[end]
so I guess we have to fix zsmalloc.
> -Mike
Thanks
Barry