From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Barry Song <song.bao.hua@hisilicon.com>
Cc: linux-mm@kvack.org, linux-crypto@vger.kernel.org,
akpm@linux-foundation.org, linuxarm@huawei.com,
fanghao11@huawei.com, linux-kernel@vger.kernel.org,
Vitaly Wool <vitalywool@gmail.com>,
"Luis Claudio R . Goncalves" <lgoncalv@redhat.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S . Miller" <davem@davemloft.net>,
Mahipal Challa <mahipalreddy2006@gmail.com>,
Seth Jennings <sjenning@redhat.com>,
Dan Streetman <ddstreet@ieee.org>,
Zhou Wang <wangzhou1@hisilicon.com>,
Colin Ian King <colin.king@canonical.com>
Subject: Re: [PATCH v7] mm/zswap: move to use crypto_acomp API for hardware acceleration
Date: Mon, 9 Nov 2020 11:29:09 +0100 [thread overview]
Message-ID: <20201109102909.u34zzudqqng6nhg6@linutronix.de> (raw)
In-Reply-To: <20201107065332.26992-1-song.bao.hua@hisilicon.com>
I've been looking at the patch and it looks like it should work. Having
numbers to backup the performance in the pure-software version and with
HW acceleration would _very_ nice to have.
On 2020-11-07 19:53:32 [+1300], Barry Song wrote:
> index fbb7829..73f04de 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -415,30 +445,54 @@ static int zswap_dstmem_dead(unsigned int cpu)
…
> + acomp_ctx->req = req;
> +
> + crypto_init_wait(&acomp_ctx->wait);
> + /*
> + * if the backend of acomp is async zip, crypto_req_done() will wakeup
> + * crypto_wait_req(); if the backend of acomp is scomp, the callback
> + * won't be called, crypto_wait_req() will return without blocking.
> + */
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> + crypto_req_done, &acomp_ctx->wait);
> +
> + acomp_ctx->mutex = per_cpu(zswap_mutex, cpu);
> + acomp_ctx->dstmem = per_cpu(zswap_dstmem, cpu);
You added a comment here and there you never mentioned that this single
per-CPU mutex protects the per-CPU context (which you can have more than
one on a single CPU) and the scratch/dstmem which is one per-CPU. Of
course if you read the code you figure it out.
I still think that you should have a pool of memory and crypto contexts
which you can use instead of having them strictly per-CPU. The code is
fully preemptible and you may have multiple requests on the same CPU.
Yes, locking works but at the same you block processing while waiting on
a lock and the "reserved memory" on other CPUs remains unused.
Sebastian
next prev parent reply other threads:[~2020-11-09 10:29 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-07 6:53 [PATCH v7] mm/zswap: move to use crypto_acomp API for hardware acceleration Barry Song
2020-11-09 10:29 ` Sebastian Andrzej Siewior [this message]
2020-11-09 11:46 ` Song Bao Hua (Barry Song)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201109102909.u34zzudqqng6nhg6@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=akpm@linux-foundation.org \
--cc=colin.king@canonical.com \
--cc=davem@davemloft.net \
--cc=ddstreet@ieee.org \
--cc=fanghao11@huawei.com \
--cc=herbert@gondor.apana.org.au \
--cc=lgoncalv@redhat.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxarm@huawei.com \
--cc=mahipalreddy2006@gmail.com \
--cc=sjenning@redhat.com \
--cc=song.bao.hua@hisilicon.com \
--cc=vitalywool@gmail.com \
--cc=wangzhou1@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).