On (09/15/15 09:18), Vitaly Wool 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? Oh, yep, missed that. Please read Dan Streetman's reply. I agree with what he said/suggested. And can you please switch your email client to 'plain text' mode only? haven't checked, but I suspect that some of vger lists block !plain_text emails. ... apart from the fact that it's hard to read such emails in mutt, etc. -ss > > > > > 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 <[2]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 <[3]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 [4]majordomo@xxxxxxxxx. For more info on Linux MM, > > > see: [5]http://www.linux-mm.org/ . > > > Don't email: <a href=mailto:"[6]dont@xxxxxxxxx"> [7]email@xxxxxxxxx > </a> > > > > > References > > Visible links > 1. mailto:sergey.senozhatsky.work@xxxxxxxxx > 2. mailto:vitalywool@xxxxxxxxx > 3. mailto:ngupta@xxxxxxxxxx > 4. mailto:majordomo@xxxxxxxxx > 5. http://www.linux-mm.org/ > 6. mailto:dont@xxxxxxxxx > 7. mailto:email@xxxxxxxxx -- 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=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>