linux-kernel - Re: [PATCH 1/3] zram: make max_zpage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150915010043.GB1860@swordfish>
Date:	Tue, 15 Sep 2015 10:00:43 +0900
From:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:	Vitaly Wool <vitalywool@...il.com>
Cc:	minchan@...nel.org, sergey.senozhatsky@...il.com,
	ddstreet@...e.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 1/3] zram: make max_zpage_size configurable

On (09/14/15 15:50), Vitaly Wool wrote:
> It makes sense to have control over what compression ratios are
> ok to store pages uncompressed and what not.

um... I don't want this to be exported. this is very 'zram-internal'.

besides, you remove the exsiting default value
- static const size_t max_zpage_size = PAGE_SIZE / 4 * 3;

so now people must provide this module param in order to make zram
work the way it used to work for years?


> Moreover, if we end up using zbud allocator for zram, any attempt to
> allocate a whole page will fail, so we may want to avoid this as much
> as possible.

so how does it help?

> So, let's have max_zpage_size configurable as a module parameter.
> 
> Signed-off-by: Vitaly Wool <vitalywool@...il.com>
> ---
>  drivers/block/zram/zram_drv.c | 13 +++++++++++++
>  drivers/block/zram/zram_drv.h | 16 ----------------
>  2 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 9fa15bb..6d9f1d1 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -42,6 +42,7 @@ static const char *default_compressor = "lzo";
>  
>  /* Module params (documentation at end) */
>  static unsigned int num_devices = 1;
> +static size_t max_zpage_size = PAGE_SIZE / 4 * 3;
>  
>  static inline void deprecated_attr_warn(const char *name)
>  {
> @@ -1411,6 +1412,16 @@ static int __init zram_init(void)
>  		return ret;
>  	}
>  
> +	/*
> +	 * max_zpage_size must be less than or equal to:
> +	 * ZS_MAX_ALLOC_SIZE. Otherwise, zs_malloc() would
> +	 * always return failure.
> +	 */
> +	if (max_zpage_size > PAGE_SIZE) {
> +		pr_err("Invalid max_zpage_size %ld\n", max_zpage_size);

and how do people find out ZS_MAX_ALLOC_SIZE? this error message does not
help.

> +		return -EINVAL;
> +	}
> +
>  	zram_major = register_blkdev(0, "zram");
>  	if (zram_major <= 0) {
>  		pr_err("Unable to get major number\n");
> @@ -1444,6 +1455,8 @@ module_exit(zram_exit);
>  
>  module_param(num_devices, uint, 0);
>  MODULE_PARM_DESC(num_devices, "Number of pre-created zram devices");
> +module_param(max_zpage_size, ulong, 0);
> +MODULE_PARM_DESC(max_zpage_size, "Threshold for storing compressed pages");

unclear description.


>  
>  MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_AUTHOR("Nitin Gupta <ngupta@...are.org>");
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index 8e92339..3a29c33 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -20,22 +20,6 @@
>  
>  #include "zcomp.h"
>  
> -/*-- Configurable parameters */
> -
> -/*
> - * Pages that compress to size greater than this are stored
> - * uncompressed in memory.
> - */
> -static const size_t max_zpage_size = PAGE_SIZE / 4 * 3;
> -
> -/*
> - * NOTE: max_zpage_size must be less than or equal to:
> - *   ZS_MAX_ALLOC_SIZE. Otherwise, zs_malloc() would
> - * always return failure.
> - */
> -
> -/*-- End of configurable params */
> -
>  #define SECTOR_SHIFT		9
>  #define SECTORS_PER_PAGE_SHIFT	(PAGE_SHIFT - SECTOR_SHIFT)
>  #define SECTORS_PER_PAGE	(1 << SECTORS_PER_PAGE_SHIFT)
> -- 
> 1.9.1
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/