On Sep 15, 2015 2:59 AM, "Sergey Senozhatsky" <sergey.senozhatsky.work@xxxxxxxxx> wrote:
>
> 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?
>
I do not remove it, I move it :) May I ask to review the patch again please?
>
> > 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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>");
> > 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@xxxxxxxxx. For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href="" href="mailto:dont@xxxxxxxxx">dont@xxxxxxxxx"> email@xxxxxxxxx </a>
> >