The patch contains too much noise and unrelated changes. Its goal is to switch zcomp to use Crypto api. I really would love to see zram_drv aligned with https://lkml.org/lkml/2015/8/13/343 IOW, only zcomp_decompress_begin()/zcomp_decompress_end() changes in zram; the rest is purely zcomp related. [..] > -static struct zcomp_backend *backends[] = { > - &zcomp_lzo, > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS > - &zcomp_lz4, > +static const char * const backends[] = { > + "lzo", > +#ifdef CONFIG_CRYPTO_LZ4 > + "lz4", > +#endif > +#ifdef CONFIG_CRYPTO_LZ4HC > + "lz4hc", > +#endif > +#ifdef CONFIG_CRYPTO_DEFLATE > + "deflate", > +#endif > +#ifdef CONFIG_CRYPTO_842 > + "842", > #endif > NULL > }; why this change is in this patch? > -static struct zcomp_backend *find_backend(const char *compress) > -{ > - int i = 0; > - while (backends[i]) { > - if (sysfs_streq(compress, backends[i]->name)) > - break; > - i++; > - } > - return backends[i]; > -} No, find_backend() and zcomp_available_algorithm() must stay. Don't call crypto API functions from zram_drv directly. This functionality belongs to zcomp. > static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm) > { > - if (zstrm->private) > - comp->backend->destroy(zstrm->private); > + if (zstrm->tfm) > + crypto_free_comp(zstrm->tfm); > free_pages((unsigned long)zstrm->buffer, 1); > kfree(zstrm); > } > > /* > - * allocate new zcomp_strm structure with ->private initialized by > - * backend, return NULL on error > + * allocate new zcomp_strm structure with initializing crypto data structure, > + * return NULL on error > */ > static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp) > { > @@ -80,13 +75,18 @@ static struct zcomp_strm *zcomp_strm_alloc(struct zcomp *comp) > if (!zstrm) > return NULL; > > - zstrm->private = comp->backend->create(); > + zstrm->tfm = crypto_alloc_comp(comp->name, 0, 0); > + if (IS_ERR(zstrm->tfm)) { > + kfree(zstrm); > + return NULL; > + } > + > /* > * allocate 2 pages. 1 for compressed data, plus 1 extra for the > * case when compressed size is larger than the original one > */ > zstrm->buffer = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1); > - if (!zstrm->private || !zstrm->buffer) { > + if (!zstrm->buffer) { > zcomp_strm_free(comp, zstrm); > zstrm = NULL; > } > @@ -274,23 +274,18 @@ ssize_t zcomp_available_show(const char *comp, char *buf) > int i = 0; > > while (backends[i]) { > - if (!strcmp(comp, backends[i]->name)) > + if (!strcmp(comp, backends[i])) > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > - "[%s] ", backends[i]->name); > + "[%s] ", backends[i]); > else > sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2, > - "%s ", backends[i]->name); > + "%s ", backends[i]); > i++; > } > sz += scnprintf(buf + sz, PAGE_SIZE - sz, "\n"); > return sz; > } > > -bool zcomp_available_algorithm(const char *comp) > -{ > - return find_backend(comp) != NULL; > -} > - No. > bool zcomp_set_max_streams(struct zcomp *comp, int num_strm) > { > return comp->set_max_streams(comp, num_strm); > @@ -307,15 +302,21 @@ void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm) > } > > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > - const unsigned char *src, unsigned char *dst, size_t *dst_len) > + const unsigned char *src, unsigned char *dst, > + unsigned int *dst_len) > { > - return comp->backend->compress(src, dst, dst_len, zstrm->private); > + *dst_len = PAGE_SIZE << 1; > + > + return crypto_comp_compress(zstrm->tfm, src, PAGE_SIZE, dst, dst_len); > } No, see later. > -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, > - size_t src_len, unsigned char *dst) > +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, > + const unsigned char *src, unsigned int src_len, > + unsigned char *dst) > { > - return comp->backend->decompress(src, src_len, dst); > + unsigned int size = PAGE_SIZE; > + > + return crypto_comp_decompress(zstrm->tfm, src, src_len, dst, &size); > } No, no direct Crypto API calls from zram_drv. compression/decompression should be handled in zcomp. period. What's the point of having zcomp in the first place then? > > void zcomp_destroy(struct zcomp *comp) > @@ -334,17 +335,16 @@ void zcomp_destroy(struct zcomp *comp) > struct zcomp *zcomp_create(const char *compress, int max_strm) > { > struct zcomp *comp; > - struct zcomp_backend *backend; > > - backend = find_backend(compress); > - if (!backend) > + if (!crypto_has_comp(compress, 0, 0)) > return ERR_PTR(-EINVAL); No. Crypto API calls stay in zcomp. zram_drv should know nothing about it. > comp = kzalloc(sizeof(struct zcomp), GFP_KERNEL); > if (!comp) > return ERR_PTR(-ENOMEM); > > - comp->backend = backend; > + comp->name = compress; No. I don't like that zram now use `struct zcomp' internals. Besides, ->tfm keeps alg name, zram keeps alg name. > if (max_strm > 1) > zcomp_strm_multi_create(comp, max_strm); > else > diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h > index b2388e0..c47db4d 100644 > --- a/drivers/block/zram/zcomp.h > +++ b/drivers/block/zram/zcomp.h > @@ -10,39 +10,23 @@ > #ifndef _ZCOMP_H_ > #define _ZCOMP_H_ > > +#include <linux/crypto.h> > #include <linux/mutex.h> > > struct zcomp_strm { > + struct crypto_comp *tfm; > + > /* compression/decompression buffer */ > void *buffer; > - /* > - * The private data of the compression stream, only compression > - * stream backend can touch this (e.g. compression algorithm > - * working memory) > - */ > - void *private; > + > /* used in multi stream backend, protected by backend strm_lock */ > struct list_head list; > }; > > -/* static compression backend */ > -struct zcomp_backend { > - int (*compress)(const unsigned char *src, unsigned char *dst, > - size_t *dst_len, void *private); > - > - int (*decompress)(const unsigned char *src, size_t src_len, > - unsigned char *dst); > - > - void *(*create)(void); > - void (*destroy)(void *private); > - > - const char *name; > -}; > - > /* dynamic per-device compression frontend */ > struct zcomp { > void *stream; > - struct zcomp_backend *backend; > + const char *name; > > struct zcomp_strm *(*strm_find)(struct zcomp *comp); > void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm); > @@ -51,7 +35,6 @@ struct zcomp { > }; > > ssize_t zcomp_available_show(const char *comp, char *buf); > -bool zcomp_available_algorithm(const char *comp); > > struct zcomp *zcomp_create(const char *comp, int max_strm); > void zcomp_destroy(struct zcomp *comp); > @@ -60,10 +43,12 @@ struct zcomp_strm *zcomp_strm_find(struct zcomp *comp); > void zcomp_strm_release(struct zcomp *comp, struct zcomp_strm *zstrm); > > int zcomp_compress(struct zcomp *comp, struct zcomp_strm *zstrm, > - const unsigned char *src, unsigned char *dst, size_t *dst_len); > + const unsigned char *src, unsigned char *dst, > + unsigned int *dst_len); > > -int zcomp_decompress(struct zcomp *comp, const unsigned char *src, > - size_t src_len, unsigned char *dst); > +int zcomp_decompress(struct zcomp *comp, struct zcomp_strm *zstrm, > + const unsigned char *src, unsigned int src_len, > + unsigned char *dst); > > bool zcomp_set_max_streams(struct zcomp *comp, int num_strm); > #endif /* _ZCOMP_H_ */ > diff --git a/drivers/block/zram/zcomp_lz4.c b/drivers/block/zram/zcomp_lz4.c > deleted file mode 100644 > index f2afb7e..0000000 > --- a/drivers/block/zram/zcomp_lz4.c > +++ /dev/null > @@ -1,47 +0,0 @@ > -/* > - * Copyright (C) 2014 Sergey Senozhatsky. > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License > - * as published by the Free Software Foundation; either version > - * 2 of the License, or (at your option) any later version. > - */ > - > -#include <linux/kernel.h> > -#include <linux/slab.h> > -#include <linux/lz4.h> > - > -#include "zcomp_lz4.h" > - > -static void *zcomp_lz4_create(void) > -{ > - return kzalloc(LZ4_MEM_COMPRESS, GFP_KERNEL); > -} > - > -static void zcomp_lz4_destroy(void *private) > -{ > - kfree(private); > -} > - > -static int zcomp_lz4_compress(const unsigned char *src, unsigned char *dst, > - size_t *dst_len, void *private) > -{ > - /* return : Success if return 0 */ > - return lz4_compress(src, PAGE_SIZE, dst, dst_len, private); > -} > - > -static int zcomp_lz4_decompress(const unsigned char *src, size_t src_len, > - unsigned char *dst) > -{ > - size_t dst_len = PAGE_SIZE; > - /* return : Success if return 0 */ > - return lz4_decompress_unknownoutputsize(src, src_len, dst, &dst_len); > -} > - > -struct zcomp_backend zcomp_lz4 = { > - .compress = zcomp_lz4_compress, > - .decompress = zcomp_lz4_decompress, > - .create = zcomp_lz4_create, > - .destroy = zcomp_lz4_destroy, > - .name = "lz4", > -}; > diff --git a/drivers/block/zram/zcomp_lz4.h b/drivers/block/zram/zcomp_lz4.h > deleted file mode 100644 > index 60613fb..0000000 > --- a/drivers/block/zram/zcomp_lz4.h > +++ /dev/null > @@ -1,17 +0,0 @@ > -/* > - * Copyright (C) 2014 Sergey Senozhatsky. > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License > - * as published by the Free Software Foundation; either version > - * 2 of the License, or (at your option) any later version. > - */ > - > -#ifndef _ZCOMP_LZ4_H_ > -#define _ZCOMP_LZ4_H_ > - > -#include "zcomp.h" > - > -extern struct zcomp_backend zcomp_lz4; > - > -#endif /* _ZCOMP_LZ4_H_ */ > diff --git a/drivers/block/zram/zcomp_lzo.c b/drivers/block/zram/zcomp_lzo.c > deleted file mode 100644 > index da1bc47..0000000 > --- a/drivers/block/zram/zcomp_lzo.c > +++ /dev/null > @@ -1,47 +0,0 @@ > -/* > - * Copyright (C) 2014 Sergey Senozhatsky. > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License > - * as published by the Free Software Foundation; either version > - * 2 of the License, or (at your option) any later version. > - */ > - > -#include <linux/kernel.h> > -#include <linux/slab.h> > -#include <linux/lzo.h> > - > -#include "zcomp_lzo.h" > - > -static void *lzo_create(void) > -{ > - return kzalloc(LZO1X_MEM_COMPRESS, GFP_KERNEL); > -} > - > -static void lzo_destroy(void *private) > -{ > - kfree(private); > -} > - > -static int lzo_compress(const unsigned char *src, unsigned char *dst, > - size_t *dst_len, void *private) > -{ > - int ret = lzo1x_1_compress(src, PAGE_SIZE, dst, dst_len, private); > - return ret == LZO_E_OK ? 0 : ret; > -} > - > -static int lzo_decompress(const unsigned char *src, size_t src_len, > - unsigned char *dst) > -{ > - size_t dst_len = PAGE_SIZE; > - int ret = lzo1x_decompress_safe(src, src_len, dst, &dst_len); > - return ret == LZO_E_OK ? 0 : ret; > -} > - > -struct zcomp_backend zcomp_lzo = { > - .compress = lzo_compress, > - .decompress = lzo_decompress, > - .create = lzo_create, > - .destroy = lzo_destroy, > - .name = "lzo", > -}; > diff --git a/drivers/block/zram/zcomp_lzo.h b/drivers/block/zram/zcomp_lzo.h > deleted file mode 100644 > index 128c580..0000000 > --- a/drivers/block/zram/zcomp_lzo.h > +++ /dev/null > @@ -1,17 +0,0 @@ > -/* > - * Copyright (C) 2014 Sergey Senozhatsky. > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License > - * as published by the Free Software Foundation; either version > - * 2 of the License, or (at your option) any later version. > - */ > - > -#ifndef _ZCOMP_LZO_H_ > -#define _ZCOMP_LZO_H_ > - > -#include "zcomp.h" > - > -extern struct zcomp_backend zcomp_lzo; > - > -#endif /* _ZCOMP_LZO_H_ */ > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c > index 4801e4d..b3044d3 100644 > --- a/drivers/block/zram/zram_drv.c > +++ b/drivers/block/zram/zram_drv.c > @@ -30,6 +30,7 @@ > #include <linux/err.h> > #include <linux/idr.h> > #include <linux/sysfs.h> > +#include <linux/crypto.h> > > #include "zram_drv.h" > > @@ -378,7 +379,7 @@ static ssize_t comp_algorithm_store(struct device *dev, > if (sz > 0 && zram->compressor[sz - 1] == '\n') > zram->compressor[sz - 1] = 0x00; > > - if (!zcomp_available_algorithm(zram->compressor)) > + if (!crypto_has_comp(zram->compressor, 0, 0)) > len = -EINVAL; No. > up_write(&zram->init_lock); > @@ -562,13 +563,14 @@ static void zram_free_page(struct zram *zram, size_t index) > zram_set_obj_size(meta, index, 0); > } > > -static int zram_decompress_page(struct zram *zram, char *mem, u32 index) > +static int zram_decompress_page(struct zram *zram, struct zcomp_strm *zstrm, > + char *mem, u32 index) > { > int ret = 0; > unsigned char *cmem; > struct zram_meta *meta = zram->meta; > unsigned long handle; > - size_t size; > + unsigned int size; > > bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value); > handle = meta->table[index].handle; > @@ -584,7 +586,7 @@ static int zram_decompress_page(struct zram *zram, char *mem, u32 index) > if (size == PAGE_SIZE) > copy_page(mem, cmem); > else > - ret = zcomp_decompress(zram->comp, cmem, size, mem); > + ret = zcomp_decompress(zram->comp, zstrm, cmem, size, mem); > zs_unmap_object(meta->mem_pool, handle); > bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value); > > @@ -604,6 +606,8 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec, > struct page *page; > unsigned char *user_mem, *uncmem = NULL; > struct zram_meta *meta = zram->meta; > + struct zcomp_strm *zstrm; > + > page = bvec->bv_page; > > bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value); > @@ -619,6 +623,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec, > /* Use a temporary buffer to decompress the page */ > uncmem = kmalloc(PAGE_SIZE, GFP_NOIO); > > + zstrm = zcomp_strm_find(zram->comp); No. zcomp_decompress_begin()/zcomp_decompress_end() please, and then just change them to return NULL or zstrm when needed. don't change zcomp_strm_find() to return NULL. that completely brakes zcomp design. it always return zstream -- immediately (if there is an idle zstrm) or after sleep. > user_mem = kmap_atomic(page); > if (!is_partial_io(bvec)) > uncmem = user_mem; > @@ -629,7 +634,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec, > goto out_cleanup; > } > > - ret = zram_decompress_page(zram, uncmem, index); > + ret = zram_decompress_page(zram, zstrm, uncmem, index); > /* Should NEVER happen. Return bio error if it does. */ > if (unlikely(ret)) > goto out_cleanup; > @@ -642,6 +647,7 @@ static int zram_bvec_read(struct zram *zram, struct bio_vec *bvec, > ret = 0; > out_cleanup: > kunmap_atomic(user_mem); > + zcomp_strm_release(zram->comp, zstrm); > if (is_partial_io(bvec)) > kfree(uncmem); > return ret; > @@ -651,7 +657,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, > int offset) > { > int ret = 0; > - size_t clen; > + unsigned int clen; > unsigned long handle; > struct page *page; > unsigned char *user_mem, *cmem, *src, *uncmem = NULL; > @@ -670,12 +676,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, > ret = -ENOMEM; > goto out; > } > - ret = zram_decompress_page(zram, uncmem, index); > + zstrm = zcomp_strm_find(zram->comp); > + ret = zram_decompress_page(zram, zstrm, uncmem, index); > if (ret) > goto out; > } > > - zstrm = zcomp_strm_find(zram->comp); > + if (!zstrm) > + zstrm = zcomp_strm_find(zram->comp); > user_mem = kmap_atomic(page); > > if (is_partial_io(bvec)) { > @@ -721,7 +729,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, > > handle = zs_malloc(meta->mem_pool, clen); > if (!handle) { > - pr_info("Error allocating memory for compressed page: %u, size=%zu\n", > + pr_info("Error allocating memory for compressed page: %u, size=%u\n", > index, clen); Please rebase against the linux-next. I do believe that I have changed that line a couple of weeks ago. -ss -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
