Hi Seth, Herbert,
This series adds support for kernel's new crypto acomp/scomp compression &
decompression framework to zswap. We verified these changes using the
kernel's crypto deflate-scomp, lzo-scomp modules and Cavium's ThunderX
ZIP driver (We will post the Cavium's ThunderX ZIP driver v2 patches with
acomp/scomp support soon).
Patch is on top of 'crypto-2.6' branch.
please provide your comments.
Regards,
Mahipal
Mahipal Challa (1):
mm: zswap - Add crypto acomp/scomp framework support
mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 99 insertions(+), 30 deletions(-)
--
1.8.3.1
This adds the support for kernel's crypto new acomp/scomp framework
to zswap.
Signed-off-by: Mahipal Challa <[email protected]>
Signed-off-by: Vishnu Nair <[email protected]>
---
mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 99 insertions(+), 30 deletions(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 067a0d6..d08631b 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -33,6 +33,8 @@
#include <linux/rbtree.h>
#include <linux/swap.h>
#include <linux/crypto.h>
+#include <crypto/acompress.h>
+#include <linux/scatterlist.h>
#include <linux/mempool.h>
#include <linux/zpool.h>
@@ -114,7 +116,8 @@ static int zswap_compressor_param_set(const char *,
struct zswap_pool {
struct zpool *zpool;
- struct crypto_comp * __percpu *tfm;
+ struct crypto_acomp * __percpu *acomp;
+ struct acomp_req * __percpu *acomp_req;
struct kref kref;
struct list_head list;
struct work_struct work;
@@ -379,30 +382,49 @@ static int zswap_dstmem_dead(unsigned int cpu)
static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
{
struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
- struct crypto_comp *tfm;
+ struct crypto_acomp *acomp;
+ struct acomp_req *acomp_req;
- if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu)))
+ if (WARN_ON(*per_cpu_ptr(pool->acomp, cpu)))
return 0;
+ if (WARN_ON(*per_cpu_ptr(pool->acomp_req, cpu)))
+ return 0;
+
+ acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0);
+ if (IS_ERR_OR_NULL(acomp)) {
+ pr_err("could not alloc crypto acomp %s : %ld\n",
+ pool->tfm_name, PTR_ERR(acomp));
+ return -ENOMEM;
+ }
+ *per_cpu_ptr(pool->acomp, cpu) = acomp;
- tfm = crypto_alloc_comp(pool->tfm_name, 0, 0);
- if (IS_ERR_OR_NULL(tfm)) {
- pr_err("could not alloc crypto comp %s : %ld\n",
- pool->tfm_name, PTR_ERR(tfm));
+ acomp_req = acomp_request_alloc(acomp);
+ if (IS_ERR_OR_NULL(acomp_req)) {
+ pr_err("could not alloc crypto acomp %s : %ld\n",
+ pool->tfm_name, PTR_ERR(acomp));
return -ENOMEM;
}
- *per_cpu_ptr(pool->tfm, cpu) = tfm;
+ *per_cpu_ptr(pool->acomp_req, cpu) = acomp_req;
+
return 0;
}
static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
{
struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
- struct crypto_comp *tfm;
+ struct crypto_acomp *acomp;
+ struct acomp_req *acomp_req;
+
+ acomp_req = *per_cpu_ptr(pool->acomp_req, cpu);
+ if (!IS_ERR_OR_NULL(acomp_req))
+ acomp_request_free(acomp_req);
+ *per_cpu_ptr(pool->acomp_req, cpu) = NULL;
+
+ acomp = *per_cpu_ptr(pool->acomp, cpu);
+ if (!IS_ERR_OR_NULL(acomp))
+ crypto_free_acomp(acomp);
+ *per_cpu_ptr(pool->acomp, cpu) = NULL;
- tfm = *per_cpu_ptr(pool->tfm, cpu);
- if (!IS_ERR_OR_NULL(tfm))
- crypto_free_comp(tfm);
- *per_cpu_ptr(pool->tfm, cpu) = NULL;
return 0;
}
@@ -503,8 +525,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
strlcpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
- pool->tfm = alloc_percpu(struct crypto_comp *);
- if (!pool->tfm) {
+ pool->acomp = alloc_percpu(struct crypto_acomp *);
+ if (!pool->acomp) {
+ pr_err("percpu alloc failed\n");
+ goto error;
+ }
+
+ pool->acomp_req = alloc_percpu(struct acomp_req *);
+ if (!pool->acomp_req) {
pr_err("percpu alloc failed\n");
goto error;
}
@@ -526,7 +554,8 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
return pool;
error:
- free_percpu(pool->tfm);
+ free_percpu(pool->acomp_req);
+ free_percpu(pool->acomp);
if (pool->zpool)
zpool_destroy_pool(pool->zpool);
kfree(pool);
@@ -566,7 +595,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool)
zswap_pool_debug("destroying", pool);
cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
- free_percpu(pool->tfm);
+ free_percpu(pool->acomp_req);
+ free_percpu(pool->acomp);
zpool_destroy_pool(pool->zpool);
kfree(pool);
}
@@ -763,7 +793,8 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
pgoff_t offset;
struct zswap_entry *entry;
struct page *page;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
u8 *src, *dst;
unsigned int dlen;
int ret;
@@ -803,14 +834,23 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
case ZSWAP_SWAPCACHE_NEW: /* page is locked */
/* decompress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
dlen = PAGE_SIZE;
src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
ZPOOL_MM_RO) + sizeof(struct zswap_header);
dst = kmap_atomic(page);
- tfm = *get_cpu_ptr(entry->pool->tfm);
- ret = crypto_comp_decompress(tfm, src, entry->length,
- dst, &dlen);
- put_cpu_ptr(entry->pool->tfm);
+
+ sg_init_one(&input, src, entry->length);
+ sg_init_one(&output, dst, dlen);
+ acomp_request_set_params(req, &input, &output, entry->length,
+ dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+ NULL, NULL);
+
+ ret = crypto_acomp_decompress(req);
+
+ dlen = req->dlen;
+ put_cpu_ptr(entry->pool->acomp_req);
kunmap_atomic(dst);
zpool_unmap_handle(entry->pool->zpool, entry->handle);
BUG_ON(ret);
@@ -886,7 +926,8 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
{
struct zswap_tree *tree = zswap_trees[type];
struct zswap_entry *entry, *dupentry;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
int ret;
unsigned int dlen = PAGE_SIZE, len;
unsigned long handle;
@@ -925,12 +966,27 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
}
/* compress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
+ if (!req) {
+ put_cpu_ptr(entry->pool->acomp_req);
+ ret = -EINVAL;
+ goto freepage;
+ }
+
dst = get_cpu_var(zswap_dstmem);
- tfm = *get_cpu_ptr(entry->pool->tfm);
src = kmap_atomic(page);
- ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
+
+ sg_init_one(&input, src, PAGE_SIZE);
+ /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
+ sg_init_one(&output, dst, PAGE_SIZE * 2);
+ acomp_request_set_params(req, &input, &output, PAGE_SIZE, dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
+ NULL);
+
+ ret = crypto_acomp_compress(req);
kunmap_atomic(src);
- put_cpu_ptr(entry->pool->tfm);
+ put_cpu_ptr(entry->pool->acomp_req);
+ dlen = req->dlen;
if (ret) {
ret = -EINVAL;
goto put_dstmem;
@@ -998,7 +1054,8 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
{
struct zswap_tree *tree = zswap_trees[type];
struct zswap_entry *entry;
- struct crypto_comp *tfm;
+ struct scatterlist input, output;
+ struct acomp_req *req;
u8 *src, *dst;
unsigned int dlen;
int ret;
@@ -1014,13 +1071,25 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
spin_unlock(&tree->lock);
/* decompress */
+ req = *get_cpu_ptr(entry->pool->acomp_req);
+ if (!req) {
+ put_cpu_ptr(entry->pool->acomp_req);
+ return -1;
+ }
dlen = PAGE_SIZE;
src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
ZPOOL_MM_RO) + sizeof(struct zswap_header);
dst = kmap_atomic(page);
- tfm = *get_cpu_ptr(entry->pool->tfm);
- ret = crypto_comp_decompress(tfm, src, entry->length, dst, &dlen);
- put_cpu_ptr(entry->pool->tfm);
+
+ sg_init_one(&input, src, entry->length);
+ sg_init_one(&output, dst, dlen);
+ acomp_request_set_params(req, &input, &output, entry->length, dlen);
+ acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
+ NULL);
+
+ ret = crypto_acomp_decompress(req);
+
+ put_cpu_ptr(entry->pool->acomp_req);
kunmap_atomic(dst);
zpool_unmap_handle(entry->pool->zpool, entry->handle);
BUG_ON(ret);
--
1.8.3.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On Tue, Feb 14, 2017 at 9:40 AM, Mahipal Challa
<[email protected]> wrote:
> This adds the support for kernel's crypto new acomp/scomp framework
> to zswap.
>
> Signed-off-by: Mahipal Challa <[email protected]>
> Signed-off-by: Vishnu Nair <[email protected]>
> ---
> mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 99 insertions(+), 30 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 067a0d6..d08631b 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -33,6 +33,8 @@
> #include <linux/rbtree.h>
> #include <linux/swap.h>
> #include <linux/crypto.h>
> +#include <crypto/acompress.h>
> +#include <linux/scatterlist.h>
> #include <linux/mempool.h>
> #include <linux/zpool.h>
>
> @@ -114,7 +116,8 @@ static int zswap_compressor_param_set(const char *,
>
> struct zswap_pool {
> struct zpool *zpool;
> - struct crypto_comp * __percpu *tfm;
> + struct crypto_acomp * __percpu *acomp;
> + struct acomp_req * __percpu *acomp_req;
> struct kref kref;
> struct list_head list;
> struct work_struct work;
> @@ -379,30 +382,49 @@ static int zswap_dstmem_dead(unsigned int cpu)
> static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
> {
> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
> - struct crypto_comp *tfm;
> + struct crypto_acomp *acomp;
> + struct acomp_req *acomp_req;
>
> - if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu)))
> + if (WARN_ON(*per_cpu_ptr(pool->acomp, cpu)))
> return 0;
> + if (WARN_ON(*per_cpu_ptr(pool->acomp_req, cpu)))
> + return 0;
> +
> + acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0);
> + if (IS_ERR_OR_NULL(acomp)) {
> + pr_err("could not alloc crypto acomp %s : %ld\n",
> + pool->tfm_name, PTR_ERR(acomp));
> + return -ENOMEM;
> + }
> + *per_cpu_ptr(pool->acomp, cpu) = acomp;
>
> - tfm = crypto_alloc_comp(pool->tfm_name, 0, 0);
> - if (IS_ERR_OR_NULL(tfm)) {
> - pr_err("could not alloc crypto comp %s : %ld\n",
> - pool->tfm_name, PTR_ERR(tfm));
> + acomp_req = acomp_request_alloc(acomp);
> + if (IS_ERR_OR_NULL(acomp_req)) {
> + pr_err("could not alloc crypto acomp %s : %ld\n",
> + pool->tfm_name, PTR_ERR(acomp));
> return -ENOMEM;
> }
> - *per_cpu_ptr(pool->tfm, cpu) = tfm;
> + *per_cpu_ptr(pool->acomp_req, cpu) = acomp_req;
> +
> return 0;
> }
>
> static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
> {
> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
> - struct crypto_comp *tfm;
> + struct crypto_acomp *acomp;
> + struct acomp_req *acomp_req;
> +
> + acomp_req = *per_cpu_ptr(pool->acomp_req, cpu);
> + if (!IS_ERR_OR_NULL(acomp_req))
> + acomp_request_free(acomp_req);
> + *per_cpu_ptr(pool->acomp_req, cpu) = NULL;
> +
> + acomp = *per_cpu_ptr(pool->acomp, cpu);
> + if (!IS_ERR_OR_NULL(acomp))
> + crypto_free_acomp(acomp);
> + *per_cpu_ptr(pool->acomp, cpu) = NULL;
>
> - tfm = *per_cpu_ptr(pool->tfm, cpu);
> - if (!IS_ERR_OR_NULL(tfm))
> - crypto_free_comp(tfm);
> - *per_cpu_ptr(pool->tfm, cpu) = NULL;
> return 0;
> }
>
> @@ -503,8 +525,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
>
> strlcpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
> - pool->tfm = alloc_percpu(struct crypto_comp *);
> - if (!pool->tfm) {
> + pool->acomp = alloc_percpu(struct crypto_acomp *);
> + if (!pool->acomp) {
> + pr_err("percpu alloc failed\n");
> + goto error;
> + }
> +
> + pool->acomp_req = alloc_percpu(struct acomp_req *);
> + if (!pool->acomp_req) {
> pr_err("percpu alloc failed\n");
> goto error;
> }
> @@ -526,7 +554,8 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
> return pool;
>
> error:
> - free_percpu(pool->tfm);
> + free_percpu(pool->acomp_req);
> + free_percpu(pool->acomp);
> if (pool->zpool)
> zpool_destroy_pool(pool->zpool);
> kfree(pool);
> @@ -566,7 +595,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool)
> zswap_pool_debug("destroying", pool);
>
> cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
> - free_percpu(pool->tfm);
> + free_percpu(pool->acomp_req);
> + free_percpu(pool->acomp);
> zpool_destroy_pool(pool->zpool);
> kfree(pool);
> }
> @@ -763,7 +793,8 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
> pgoff_t offset;
> struct zswap_entry *entry;
> struct page *page;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> u8 *src, *dst;
> unsigned int dlen;
> int ret;
> @@ -803,14 +834,23 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>
> case ZSWAP_SWAPCACHE_NEW: /* page is locked */
> /* decompress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> dlen = PAGE_SIZE;
> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
> ZPOOL_MM_RO) + sizeof(struct zswap_header);
> dst = kmap_atomic(page);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> - ret = crypto_comp_decompress(tfm, src, entry->length,
> - dst, &dlen);
> - put_cpu_ptr(entry->pool->tfm);
> +
> + sg_init_one(&input, src, entry->length);
> + sg_init_one(&output, dst, dlen);
> + acomp_request_set_params(req, &input, &output, entry->length,
> + dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> + NULL, NULL);
> +
> + ret = crypto_acomp_decompress(req);
I assume all of these crypto_acomp_[compress|decompress] calls are
actually synchronous,
not asynchronous as the name suggests. Otherwise, this would blow up
quite spectacularly
since all the resources we use in the call get derefed/unmapped below.
Could an async algorithm be implement/used that would break this assumption?
Seth
> +
> + dlen = req->dlen;
> + put_cpu_ptr(entry->pool->acomp_req);
> kunmap_atomic(dst);
> zpool_unmap_handle(entry->pool->zpool, entry->handle);
> BUG_ON(ret);
> @@ -886,7 +926,8 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> {
> struct zswap_tree *tree = zswap_trees[type];
> struct zswap_entry *entry, *dupentry;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> int ret;
> unsigned int dlen = PAGE_SIZE, len;
> unsigned long handle;
> @@ -925,12 +966,27 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
> }
>
> /* compress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> + if (!req) {
> + put_cpu_ptr(entry->pool->acomp_req);
> + ret = -EINVAL;
> + goto freepage;
> + }
> +
> dst = get_cpu_var(zswap_dstmem);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> src = kmap_atomic(page);
> - ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
> +
> + sg_init_one(&input, src, PAGE_SIZE);
> + /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
> + sg_init_one(&output, dst, PAGE_SIZE * 2);
> + acomp_request_set_params(req, &input, &output, PAGE_SIZE, dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
> + NULL);
> +
> + ret = crypto_acomp_compress(req);
> kunmap_atomic(src);
> - put_cpu_ptr(entry->pool->tfm);
> + put_cpu_ptr(entry->pool->acomp_req);
> + dlen = req->dlen;
> if (ret) {
> ret = -EINVAL;
> goto put_dstmem;
> @@ -998,7 +1054,8 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> {
> struct zswap_tree *tree = zswap_trees[type];
> struct zswap_entry *entry;
> - struct crypto_comp *tfm;
> + struct scatterlist input, output;
> + struct acomp_req *req;
> u8 *src, *dst;
> unsigned int dlen;
> int ret;
> @@ -1014,13 +1071,25 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
> spin_unlock(&tree->lock);
>
> /* decompress */
> + req = *get_cpu_ptr(entry->pool->acomp_req);
> + if (!req) {
> + put_cpu_ptr(entry->pool->acomp_req);
> + return -1;
> + }
> dlen = PAGE_SIZE;
> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
> ZPOOL_MM_RO) + sizeof(struct zswap_header);
> dst = kmap_atomic(page);
> - tfm = *get_cpu_ptr(entry->pool->tfm);
> - ret = crypto_comp_decompress(tfm, src, entry->length, dst, &dlen);
> - put_cpu_ptr(entry->pool->tfm);
> +
> + sg_init_one(&input, src, entry->length);
> + sg_init_one(&output, dst, dlen);
> + acomp_request_set_params(req, &input, &output, entry->length, dlen);
> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
> + NULL);
> +
> + ret = crypto_acomp_decompress(req);
> +
> + put_cpu_ptr(entry->pool->acomp_req);
> kunmap_atomic(dst);
> zpool_unmap_handle(entry->pool->zpool, entry->handle);
> BUG_ON(ret);
> --
> 1.8.3.1
>
> I assume all of these crypto_acomp_[compress|decompress] calls are
> actually synchronous,
> not asynchronous as the name suggests. Otherwise, this would blow up
> quite spectacularly
> since all the resources we use in the call get derefed/unmapped below.
>
> Could an async algorithm be implement/used that would break this assumption?
The callback is set to NULL using acomp_request_set_callback(). This
implies synchronous mode of operation. So the underlying implementation
must complete the operation synchronously.
Prasad
On Tuesday 14 February 2017 09:50 PM, Seth Jennings wrote:
> On Tue, Feb 14, 2017 at 9:40 AM, Mahipal Challa
> <[email protected]> wrote:
>> This adds the support for kernel's crypto new acomp/scomp framework
>> to zswap.
>>
>> Signed-off-by: Mahipal Challa <[email protected]>
>> Signed-off-by: Vishnu Nair <[email protected]>
>> ---
>> mm/zswap.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 99 insertions(+), 30 deletions(-)
>>
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index 067a0d6..d08631b 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -33,6 +33,8 @@
>> #include <linux/rbtree.h>
>> #include <linux/swap.h>
>> #include <linux/crypto.h>
>> +#include <crypto/acompress.h>
>> +#include <linux/scatterlist.h>
>> #include <linux/mempool.h>
>> #include <linux/zpool.h>
>>
>> @@ -114,7 +116,8 @@ static int zswap_compressor_param_set(const char *,
>>
>> struct zswap_pool {
>> struct zpool *zpool;
>> - struct crypto_comp * __percpu *tfm;
>> + struct crypto_acomp * __percpu *acomp;
>> + struct acomp_req * __percpu *acomp_req;
>> struct kref kref;
>> struct list_head list;
>> struct work_struct work;
>> @@ -379,30 +382,49 @@ static int zswap_dstmem_dead(unsigned int cpu)
>> static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node)
>> {
>> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
>> - struct crypto_comp *tfm;
>> + struct crypto_acomp *acomp;
>> + struct acomp_req *acomp_req;
>>
>> - if (WARN_ON(*per_cpu_ptr(pool->tfm, cpu)))
>> + if (WARN_ON(*per_cpu_ptr(pool->acomp, cpu)))
>> return 0;
>> + if (WARN_ON(*per_cpu_ptr(pool->acomp_req, cpu)))
>> + return 0;
>> +
>> + acomp = crypto_alloc_acomp(pool->tfm_name, 0, 0);
>> + if (IS_ERR_OR_NULL(acomp)) {
>> + pr_err("could not alloc crypto acomp %s : %ld\n",
>> + pool->tfm_name, PTR_ERR(acomp));
>> + return -ENOMEM;
>> + }
>> + *per_cpu_ptr(pool->acomp, cpu) = acomp;
>>
>> - tfm = crypto_alloc_comp(pool->tfm_name, 0, 0);
>> - if (IS_ERR_OR_NULL(tfm)) {
>> - pr_err("could not alloc crypto comp %s : %ld\n",
>> - pool->tfm_name, PTR_ERR(tfm));
>> + acomp_req = acomp_request_alloc(acomp);
>> + if (IS_ERR_OR_NULL(acomp_req)) {
>> + pr_err("could not alloc crypto acomp %s : %ld\n",
>> + pool->tfm_name, PTR_ERR(acomp));
>> return -ENOMEM;
>> }
>> - *per_cpu_ptr(pool->tfm, cpu) = tfm;
>> + *per_cpu_ptr(pool->acomp_req, cpu) = acomp_req;
>> +
>> return 0;
>> }
>>
>> static int zswap_cpu_comp_dead(unsigned int cpu, struct hlist_node *node)
>> {
>> struct zswap_pool *pool = hlist_entry(node, struct zswap_pool, node);
>> - struct crypto_comp *tfm;
>> + struct crypto_acomp *acomp;
>> + struct acomp_req *acomp_req;
>> +
>> + acomp_req = *per_cpu_ptr(pool->acomp_req, cpu);
>> + if (!IS_ERR_OR_NULL(acomp_req))
>> + acomp_request_free(acomp_req);
>> + *per_cpu_ptr(pool->acomp_req, cpu) = NULL;
>> +
>> + acomp = *per_cpu_ptr(pool->acomp, cpu);
>> + if (!IS_ERR_OR_NULL(acomp))
>> + crypto_free_acomp(acomp);
>> + *per_cpu_ptr(pool->acomp, cpu) = NULL;
>>
>> - tfm = *per_cpu_ptr(pool->tfm, cpu);
>> - if (!IS_ERR_OR_NULL(tfm))
>> - crypto_free_comp(tfm);
>> - *per_cpu_ptr(pool->tfm, cpu) = NULL;
>> return 0;
>> }
>>
>> @@ -503,8 +525,14 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
>> pr_debug("using %s zpool\n", zpool_get_type(pool->zpool));
>>
>> strlcpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
>> - pool->tfm = alloc_percpu(struct crypto_comp *);
>> - if (!pool->tfm) {
>> + pool->acomp = alloc_percpu(struct crypto_acomp *);
>> + if (!pool->acomp) {
>> + pr_err("percpu alloc failed\n");
>> + goto error;
>> + }
>> +
>> + pool->acomp_req = alloc_percpu(struct acomp_req *);
>> + if (!pool->acomp_req) {
>> pr_err("percpu alloc failed\n");
>> goto error;
>> }
>> @@ -526,7 +554,8 @@ static struct zswap_pool *zswap_pool_create(char *type, char *compressor)
>> return pool;
>>
>> error:
>> - free_percpu(pool->tfm);
>> + free_percpu(pool->acomp_req);
>> + free_percpu(pool->acomp);
>> if (pool->zpool)
>> zpool_destroy_pool(pool->zpool);
>> kfree(pool);
>> @@ -566,7 +595,8 @@ static void zswap_pool_destroy(struct zswap_pool *pool)
>> zswap_pool_debug("destroying", pool);
>>
>> cpuhp_state_remove_instance(CPUHP_MM_ZSWP_POOL_PREPARE, &pool->node);
>> - free_percpu(pool->tfm);
>> + free_percpu(pool->acomp_req);
>> + free_percpu(pool->acomp);
>> zpool_destroy_pool(pool->zpool);
>> kfree(pool);
>> }
>> @@ -763,7 +793,8 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>> pgoff_t offset;
>> struct zswap_entry *entry;
>> struct page *page;
>> - struct crypto_comp *tfm;
>> + struct scatterlist input, output;
>> + struct acomp_req *req;
>> u8 *src, *dst;
>> unsigned int dlen;
>> int ret;
>> @@ -803,14 +834,23 @@ static int zswap_writeback_entry(struct zpool *pool, unsigned long handle)
>>
>> case ZSWAP_SWAPCACHE_NEW: /* page is locked */
>> /* decompress */
>> + req = *get_cpu_ptr(entry->pool->acomp_req);
>> dlen = PAGE_SIZE;
>> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
>> ZPOOL_MM_RO) + sizeof(struct zswap_header);
>> dst = kmap_atomic(page);
>> - tfm = *get_cpu_ptr(entry->pool->tfm);
>> - ret = crypto_comp_decompress(tfm, src, entry->length,
>> - dst, &dlen);
>> - put_cpu_ptr(entry->pool->tfm);
>> +
>> + sg_init_one(&input, src, entry->length);
>> + sg_init_one(&output, dst, dlen);
>> + acomp_request_set_params(req, &input, &output, entry->length,
>> + dlen);
>> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
>> + NULL, NULL);
>> +
>> + ret = crypto_acomp_decompress(req);
> I assume all of these crypto_acomp_[compress|decompress] calls are
> actually synchronous,
> not asynchronous as the name suggests. Otherwise, this would blow up
> quite spectacularly
> since all the resources we use in the call get derefed/unmapped below.
>
> Could an async algorithm be implement/used that would break this assumption?
>
> Seth
>
>> +
>> + dlen = req->dlen;
>> + put_cpu_ptr(entry->pool->acomp_req);
>> kunmap_atomic(dst);
>> zpool_unmap_handle(entry->pool->zpool, entry->handle);
>> BUG_ON(ret);
>> @@ -886,7 +926,8 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
>> {
>> struct zswap_tree *tree = zswap_trees[type];
>> struct zswap_entry *entry, *dupentry;
>> - struct crypto_comp *tfm;
>> + struct scatterlist input, output;
>> + struct acomp_req *req;
>> int ret;
>> unsigned int dlen = PAGE_SIZE, len;
>> unsigned long handle;
>> @@ -925,12 +966,27 @@ static int zswap_frontswap_store(unsigned type, pgoff_t offset,
>> }
>>
>> /* compress */
>> + req = *get_cpu_ptr(entry->pool->acomp_req);
>> + if (!req) {
>> + put_cpu_ptr(entry->pool->acomp_req);
>> + ret = -EINVAL;
>> + goto freepage;
>> + }
>> +
>> dst = get_cpu_var(zswap_dstmem);
>> - tfm = *get_cpu_ptr(entry->pool->tfm);
>> src = kmap_atomic(page);
>> - ret = crypto_comp_compress(tfm, src, PAGE_SIZE, dst, &dlen);
>> +
>> + sg_init_one(&input, src, PAGE_SIZE);
>> + /* zswap_dstmem is of size (PAGE_SIZE * 2). Reflect same in sg_list */
>> + sg_init_one(&output, dst, PAGE_SIZE * 2);
>> + acomp_request_set_params(req, &input, &output, PAGE_SIZE, dlen);
>> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
>> + NULL);
>> +
>> + ret = crypto_acomp_compress(req);
>> kunmap_atomic(src);
>> - put_cpu_ptr(entry->pool->tfm);
>> + put_cpu_ptr(entry->pool->acomp_req);
>> + dlen = req->dlen;
>> if (ret) {
>> ret = -EINVAL;
>> goto put_dstmem;
>> @@ -998,7 +1054,8 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
>> {
>> struct zswap_tree *tree = zswap_trees[type];
>> struct zswap_entry *entry;
>> - struct crypto_comp *tfm;
>> + struct scatterlist input, output;
>> + struct acomp_req *req;
>> u8 *src, *dst;
>> unsigned int dlen;
>> int ret;
>> @@ -1014,13 +1071,25 @@ static int zswap_frontswap_load(unsigned type, pgoff_t offset,
>> spin_unlock(&tree->lock);
>>
>> /* decompress */
>> + req = *get_cpu_ptr(entry->pool->acomp_req);
>> + if (!req) {
>> + put_cpu_ptr(entry->pool->acomp_req);
>> + return -1;
>> + }
>> dlen = PAGE_SIZE;
>> src = (u8 *)zpool_map_handle(entry->pool->zpool, entry->handle,
>> ZPOOL_MM_RO) + sizeof(struct zswap_header);
>> dst = kmap_atomic(page);
>> - tfm = *get_cpu_ptr(entry->pool->tfm);
>> - ret = crypto_comp_decompress(tfm, src, entry->length, dst, &dlen);
>> - put_cpu_ptr(entry->pool->tfm);
>> +
>> + sg_init_one(&input, src, entry->length);
>> + sg_init_one(&output, dst, dlen);
>> + acomp_request_set_params(req, &input, &output, entry->length, dlen);
>> + acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG, NULL,
>> + NULL);
>> +
>> + ret = crypto_acomp_decompress(req);
>> +
>> + put_cpu_ptr(entry->pool->acomp_req);
>> kunmap_atomic(dst);
>> zpool_unmap_handle(entry->pool->zpool, entry->handle);
>> BUG_ON(ret);
>> --
>> 1.8.3.1
>>
> .
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
On Wed, Feb 15, 2017 at 07:27:30PM +0530, Narayana Prasad Athreya wrote:
> > I assume all of these crypto_acomp_[compress|decompress] calls are
> > actually synchronous,
> > not asynchronous as the name suggests. Otherwise, this would blow up
> > quite spectacularly
> > since all the resources we use in the call get derefed/unmapped below.
> >
> > Could an async algorithm be implement/used that would break this assumption?
>
> The callback is set to NULL using acomp_request_set_callback(). This implies
> synchronous mode of operation. So the underlying implementation must
> complete the operation synchronously.
This assumption is not correct. An asynchronous implementation, when
it finishes processing a request, will call acomp_request_complete() which
in turn calls the callback.
If the callback is set to NULL, this function will dereference a NULL
pointer.
Regards,
--
Giovanni
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>This assumption is not correct. An asynchronous implementation, when
>it finishes processing a request, will call acomp_request_complete() which
>in turn calls the callback.
>If the callback is set to NULL, this function will dereference a NULL
>pointer.
This would leave us with the option of waiting in zswap until completion. Here we had a doubt.
If we go ahead with an implementation similar to the one found in crypto/testmgr.c, the private data(result) which is registered via 'acomp_request_set_callback()' is coming from stack. Do you see this as a potential problem for an acutal asynchronus algorithm due to the context from which callback is called? Do we have to use per-cpu dynamic allocation?
Thanks,
Vishnu
________________________________
From: Giovanni Cabiddu <[email protected]>
Sent: Thursday, February 16, 2017 3:42 AM
To: Narayana, Prasad Athreya
Cc: Seth Jennings; Mahipal Challa; [email protected]; [email protected]; [email protected]; LKML; Linux-MM; Narayana, Prasad Athreya; Nair, Vishnu; Challa, Mahipal; Nair, Vishnu
Subject: Re: [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp framework support
On Wed, Feb 15, 2017 at 07:27:30PM +0530, Narayana Prasad Athreya wrote:
> > I assume all of these crypto_acomp_[compress|decompress] calls are
> > actually synchronous,
> > not asynchronous as the name suggests. Otherwise, this would blow up
> > quite spectacularly
> > since all the resources we use in the call get derefed/unmapped below.
> >
> > Could an async algorithm be implement/used that would break this assumption?
>
> The callback is set to NULL using acomp_request_set_callback(). This implies
> synchronous mode of operation. So the underlying implementation must
> complete the operation synchronously.
This assumption is not correct. An asynchronous implementation, when
it finishes processing a request, will call acomp_request_complete() which
in turn calls the callback.
If the callback is set to NULL, this function will dereference a NULL
pointer.
Regards,
--
Giovanni