Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1246361 > unrolled thread

[PATCH v4 8/8] zram: enable contextless compression alg in zram

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 8/8] zram: enable contextless compression alg in zram Joonsoo Kim <js1304@gmail.com> - 2015-10-14 09:40 +0200
    Re: [PATCH v4 8/8] zram: enable contextless compression alg in zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-10-15 02:30 +0200
      Re: [PATCH v4 8/8] zram: enable contextless compression alg in zram Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-10-15 02:50 +0200
    Re: [PATCH v4 8/8] zram: enable contextless compression alg in zram Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-10-15 04:10 +0200
      Re: [PATCH v4 8/8] zram: enable contextless compression alg in zram Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2015-10-15 04:50 +0200

#1246361 — [PATCH v4 8/8] zram: enable contextless compression alg in zram

Now, zram uses contextless compression API and there is no reason
to limit compression algorithm through hard-wired string. This patch
remove it so enable all contextless compression algorithm in the
system.

After this patch, available compression algorithm for zram can be
retrieved by searching contextless compression in /proc/crypto.

cat /proc/crypto | grep ccomp -B 7
name         : lz4
[snip...]
type         : ccomp

Previous interface comp_algorithm attr will remain to set new algorithm.
Read from previous interface also works for compatibility but it will
be wrong.

Note that after this patch is applied, there is no way to know current
compression algorithm so it's really bad thing. Please let me know proper
solution if someone have better idea.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 Documentation/blockdev/zram.txt | 29 +++++++++++++++++++++++------
 drivers/block/zram/Kconfig      |  9 ---------
 drivers/block/zram/zcomp.c      | 17 +++++++----------
 drivers/block/zram/zram_drv.c   |  1 +
 4 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 5bda503..7a165c2 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -81,15 +81,32 @@ dynamic max_comp_streams. Only multi stream backend supports dynamic
 max_comp_streams adjustment.
 
 3) Select compression algorithm
-	Using comp_algorithm device attribute one can see available and
-	currently selected (shown in square brackets) compression algorithms,
-	change selected compression algorithm (once the device is initialised
-	there is no way to change compression algorithm).
+	You can find available compression algorithms by searching contextless
+	compression algorithm (type: ccomp) in /proc/crypto.
+	Using comp_algorithm device attribute one can change selected
+	compression algorithm (once the device is initialised there is no way
+	to change compression algorithm).
 
 	Examples:
 	#show supported compression algorithms
-	cat /sys/block/zram0/comp_algorithm
-	lzo [lz4]
+	cat /proc/crypto | grep ccomp -B 7
+	name         : lz4
+	driver       : lz4-generic
+	module       : kernel
+	priority     : 0
+	refcnt       : 1
+	selftest     : passed
+	internal     : no
+	type         : ccomp
+	--
+	name         : lzo
+	driver       : lzo-generic
+	module       : kernel
+	priority     : 0
+	refcnt       : 1
+	selftest     : passed
+	internal     : no
+	type         : ccomp
 
 	#select lzo compression algorithm
 	echo lzo > /sys/block/zram0/comp_algorithm
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 7619bed..36ec96f 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -13,12 +13,3 @@ config ZRAM
 	  disks and maybe many more.
 
 	  See zram.txt for more information.
-
-config ZRAM_LZ4_COMPRESS
-	bool "Enable LZ4 algorithm support"
-	depends on ZRAM
-	select CRYPTO_LZ4
-	default n
-	help
-	  This option enables LZ4 compression algorithm support. Compression
-	  algorithm can be changed using `comp_algorithm' device attribute.
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 9be83db..f91c0659 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -41,22 +41,16 @@ struct zcomp_strm_multi {
 
 static const char * const backends[] = {
 	"lzo",
-#ifdef CONFIG_ZRAM_LZ4_COMPRESS
 	"lz4",
-#endif
 	NULL
 };
 
 static const char *find_backend(const char *compress)
 {
-	int i = 0;
-	while (backends[i]) {
-		if (sysfs_streq(compress, backends[i]) &&
-			crypto_has_comp(backends[i], 0, 0))
-			break;
-		i++;
-	}
-	return backends[i];
+	if (crypto_has_comp(compress, 0, 0))
+		return compress;
+
+	return NULL;
 }
 
 static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
@@ -277,6 +271,9 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
 	int i = 0;
 
 	while (backends[i]) {
+		if (!crypto_has_comp(backends[i], 0, 0))
+			continue;
+
 		if (!strcmp(comp, backends[i]))
 			sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
 					"[%s] ", backends[i]);
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 6f04fb2..6b4cf85 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -352,6 +352,7 @@ static ssize_t comp_algorithm_show(struct device *dev,
 	size_t sz;
 	struct zram *zram = dev_to_zram(dev);
 
+	deprecated_attr_warn("comp_algorithm");
 	down_read(&zram->init_lock);
 	sz = zcomp_available_show(zram->compressor, buf);
 	up_read(&zram->init_lock);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1247270

Hi,

On (10/14/15 16:38), Joonsoo Kim wrote:
[..]
>  static const char * const backends[] = {
>  	"lzo",
> -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
>  	"lz4",
> -#endif
>  	NULL
>  };
>  
>  static const char *find_backend(const char *compress)
>  {
> -	int i = 0;
> -	while (backends[i]) {
> -		if (sysfs_streq(compress, backends[i]) &&
> -			crypto_has_comp(backends[i], 0, 0))
> -			break;
> -		i++;
> -	}
> -	return backends[i];
> +	if (crypto_has_comp(compress, 0, 0))
> +		return compress;
> +
> +	return NULL;
>  }
>  
>  static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> @@ -277,6 +271,9 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
>  	int i = 0;
>  
>  	while (backends[i]) {
> +		if (!crypto_has_comp(backends[i], 0, 0))
> +			continue;
> +

hm... this sort of looks a bit `unnatural' to me. we have two _independent_
sets -- what zram supports and what crypto supports. that's why you have
to do extra work and consult crypto. can we return back the old scheme:
use ifdef CONFIG in backends, but replace CONFIG_ZRAM with CONFIG_CRYPTO?

e.g.

	static const char * const backends[] = {
		"lzo",
	#ifdef CONFIG_CRYPTO_LZ4
		"lz4",
	#endif
		NULL
	};


so you can remove `crypto_has_comp(backends[i], 0, 0)' from
zcomp_available_show(), because zram will support *only* what
crypto supports.

	-ss


>  		if (!strcmp(comp, backends[i]))
>  			sz += scnprintf(buf + sz, PAGE_SIZE - sz - 2,
>  					"[%s] ", backends[i]);
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 6f04fb2..6b4cf85 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -352,6 +352,7 @@ static ssize_t comp_algorithm_show(struct device *dev,
>  	size_t sz;
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("comp_algorithm");
>  	down_read(&zram->init_lock);
>  	sz = zcomp_available_show(zram->compressor, buf);
>  	up_read(&zram->init_lock);
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247276

On Thu, Oct 15, 2015 at 09:29:03AM +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> On (10/14/15 16:38), Joonsoo Kim wrote:
> [..]
> >  static const char * const backends[] = {
> >  	"lzo",
> > -#ifdef CONFIG_ZRAM_LZ4_COMPRESS
> >  	"lz4",
> > -#endif
> >  	NULL
> >  };
> >  
> >  static const char *find_backend(const char *compress)
> >  {
> > -	int i = 0;
> > -	while (backends[i]) {
> > -		if (sysfs_streq(compress, backends[i]) &&
> > -			crypto_has_comp(backends[i], 0, 0))
> > -			break;
> > -		i++;
> > -	}
> > -	return backends[i];
> > +	if (crypto_has_comp(compress, 0, 0))
> > +		return compress;
> > +
> > +	return NULL;
> >  }
> >  
> >  static void zcomp_strm_free(struct zcomp *comp, struct zcomp_strm *zstrm)
> > @@ -277,6 +271,9 @@ ssize_t zcomp_available_show(const char *comp, char *buf)
> >  	int i = 0;
> >  
> >  	while (backends[i]) {
> > +		if (!crypto_has_comp(backends[i], 0, 0))
> > +			continue;
> > +
> 
> hm... this sort of looks a bit `unnatural' to me. we have two _independent_
> sets -- what zram supports and what crypto supports. that's why you have
> to do extra work and consult crypto. can we return back the old scheme:
> use ifdef CONFIG in backends, but replace CONFIG_ZRAM with CONFIG_CRYPTO?
> 
> e.g.
> 
> 	static const char * const backends[] = {
> 		"lzo",
> 	#ifdef CONFIG_CRYPTO_LZ4
> 		"lz4",
> 	#endif
> 		NULL
> 	};
> 
> 
> so you can remove `crypto_has_comp(backends[i], 0, 0)' from
> zcomp_available_show(), because zram will support *only* what
> crypto supports.

Hello, Sergey.

Okay. I will change it in next spin.

Anyway, now I noticed that crypto_has_comp() is not proper API to
check contextless compression algorithm. I will change it, too.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247323

On (10/14/15 16:38), Joonsoo Kim wrote:
[..]
> @@ -352,6 +352,7 @@ static ssize_t comp_algorithm_show(struct device *dev,
>  	size_t sz;
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("comp_algorithm");
>  	down_read(&zram->init_lock);
>  	sz = zcomp_available_show(zram->compressor, buf);
>  	up_read(&zram->init_lock);

oh, one more thing.

deprecated_attr_warn() should come with `Documentation/ABI/obsolete/sysfs-block-zram' update.
something like:

diff --git a/Documentation/ABI/obsolete/sysfs-block-zram b/Documentation/ABI/obsolete/sysfs-block-zram
index 720ea92..ad5b59d 100644
--- a/Documentation/ABI/obsolete/sysfs-block-zram
+++ b/Documentation/ABI/obsolete/sysfs-block-zram
@@ -117,3 +117,14 @@ Description:
                Downgraded to write-only node: so it's possible to set new
                value only; its current value is stored in zram<id>/mm_stat
                node.
+
+What:          /sys/block/zram<id>/comp_algorithm
+Date:          XXX
+Contact:       XXX
+Description:
+               The comp_algorithm file is read/write and provides information
+               on available, currently selected compression algorithm (read
+               operation) and lets one to change the compression algorithm
+               (write operation).
+               Downgraded to write-only node: use `/proc/crypto' to get the
+               list of supported compression algorithms.


---


And I guess Cc `Jonathan Corbet <corbet@lwn.net>' (doc maintainer) and
'linux-api@vger.kernel.org' will be the right thing to do here.

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247352

On Thu, Oct 15, 2015 at 11:05:49AM +0900, Sergey Senozhatsky wrote:
> On (10/14/15 16:38), Joonsoo Kim wrote:
> [..]
> > @@ -352,6 +352,7 @@ static ssize_t comp_algorithm_show(struct device *dev,
> >  	size_t sz;
> >  	struct zram *zram = dev_to_zram(dev);
> >  
> > +	deprecated_attr_warn("comp_algorithm");
> >  	down_read(&zram->init_lock);
> >  	sz = zcomp_available_show(zram->compressor, buf);
> >  	up_read(&zram->init_lock);
> 
> oh, one more thing.
> 
> deprecated_attr_warn() should come with `Documentation/ABI/obsolete/sysfs-block-zram' update.
> something like:
> 
> diff --git a/Documentation/ABI/obsolete/sysfs-block-zram b/Documentation/ABI/obsolete/sysfs-block-zram
> index 720ea92..ad5b59d 100644
> --- a/Documentation/ABI/obsolete/sysfs-block-zram
> +++ b/Documentation/ABI/obsolete/sysfs-block-zram
> @@ -117,3 +117,14 @@ Description:
>                 Downgraded to write-only node: so it's possible to set new
>                 value only; its current value is stored in zram<id>/mm_stat
>                 node.
> +
> +What:          /sys/block/zram<id>/comp_algorithm
> +Date:          XXX
> +Contact:       XXX
> +Description:
> +               The comp_algorithm file is read/write and provides information
> +               on available, currently selected compression algorithm (read
> +               operation) and lets one to change the compression algorithm
> +               (write operation).
> +               Downgraded to write-only node: use `/proc/crypto' to get the
> +               list of supported compression algorithms.
> 
> 
> ---
> 
> 
> And I guess Cc `Jonathan Corbet <corbet@lwn.net>' (doc maintainer) and
> 'linux-api@vger.kernel.org' will be the right thing to do here.

Okay. I will do it in next spin.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web