From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Dan Streetman <ddstreet@ieee.org>
Cc: Vitaly Wool <vitalywool@gmail.com>,
Minchan Kim <minchan@kernel.org>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH 1/3] zram: make max_zpage_size configurable
Date: Tue, 15 Sep 2015 15:08:46 +0900 [thread overview]
Message-ID: <20150915060846.GA454@swordfish> (raw)
In-Reply-To: <CALZtOND74zjQCoVc+X4PBdZE1vKHGpt_nauU0JnyMC0c-u1bsg@mail.gmail.com>
On (09/15/15 01:42), Dan Streetman wrote:
> Well, zram explicitly expects to be able to store PAGE_SIZE'd objects:
>
> if (unlikely(clen > max_zpage_size))
> clen = PAGE_SIZE;
> handle = zs_malloc(meta->mem_pool, clen);
>
> so the max_zpage_size doesn't prevent zram from trying to store the
> page in zsmalloc/zbud/whatever; instead, if the compressed page is
> larger than max_zpage_size, it just stores it uncompressed (as a side
> note, I'm not quite sure what the benefit of not storing in compressed
> form any pages that compress to between 3/4 and 1 page is...I suppose
> the decompression time is skipped, but it also wastes space...i would
> just make max_zpage_size == PAGE_SIZE).
correct, to avoid decompression of something that doesn't really
save any memory. compressed buffer may be very close to PAGE_SIZE,
so it sort of makes sense. I agree, that (3/4, 1 page] looks like
a magically picked range, though.
>
> but zbud can't store a PAGE_SIZE'd object. so the behavior would
> change. The current behavior is:
>
> compressed page <= max_zpage_size : stored compressed
> compressed page > max_zpage_size : stored uncompressed
>
> new behavior:
>
> compressed page <= max_zpage_size : stored compressed
> compressed page > max_zpage_size : zram write fails
yes. and per my observation, `compressed page > max_zpage_size'
happens "quite often".
-ss
> to do this right, I think you have to change zbud to be able to store
> PAGE_SIZE'd objects. That should be doable, I think you can just the
> page->lru to store it in the zbud lru, and use a page flag to indicate
> it's uncompressed, full PAGE_SIZE page, or something like that. But
> without the ability to store full pages, zbud won't work well with
> zram.
>
> >
> > So, let's have max_zpage_size configurable as a module parameter.
> >
> > Signed-off-by: Vitaly Wool <vitalywool@gmail.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);
> > + 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");
> >
> > MODULE_LICENSE("Dual BSD/GPL");
> > MODULE_AUTHOR("Nitin Gupta <ngupta@vflare.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@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
next prev parent reply other threads:[~2015-09-15 6:08 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-14 13:49 [PATCH 0/3] allow zram to use zbud as underlying allocator Vitaly Wool
2015-09-14 13:50 ` [PATCH 1/3] zram: make max_zpage_size configurable Vitaly Wool
2015-09-15 1:00 ` Sergey Senozhatsky
[not found] ` <CAMJBoFPynFJpu57Gftm6bbyGfdSK67y-HbvoQPwyuHS6+0tOyQ@mail.gmail.com>
2015-09-15 7:38 ` Sergey Senozhatsky
2015-09-15 5:42 ` Dan Streetman
2015-09-15 6:08 ` Sergey Senozhatsky [this message]
2015-09-14 13:51 ` [PATCH 2/3] zpool/zsmalloc/zbud: align on interfaces Vitaly Wool
2015-09-15 1:06 ` Sergey Senozhatsky
2015-09-15 5:09 ` Dan Streetman
2015-09-14 13:55 ` [PATCH 3/3] zram: use common zpool interface Vitaly Wool
2015-09-15 1:12 ` Sergey Senozhatsky
2015-09-15 6:03 ` Dan Streetman
2015-09-14 14:01 ` [PATCH 0/3] allow zram to use zbud as underlying allocator Vlastimil Babka
2015-09-14 14:12 ` Vitaly Wool
2015-09-14 14:14 ` Vlastimil Babka
2015-09-15 4:08 ` Dan Streetman
2015-09-15 4:22 ` Sergey Senozhatsky
2015-09-17 6:21 ` Vlastimil Babka
2015-09-17 9:19 ` Sergey Senozhatsky
2015-09-15 0:49 ` Sergey Senozhatsky
2015-09-15 6:13 ` Minchan Kim
2015-09-25 9:54 ` Vitaly Wool
2015-09-30 7:52 ` Minchan Kim
2015-09-30 8:01 ` Vitaly Wool
2015-09-30 8:13 ` Minchan Kim
2015-09-30 8:18 ` Vitaly Wool
2015-09-30 15:37 ` Vlastimil Babka
2015-09-30 15:46 ` Vitaly Wool
2015-10-01 7:52 ` Vlastimil Babka
2015-10-10 9:33 ` Vitaly Wool
2015-10-14 13:28 ` Vlastimil Babka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150915060846.GA454@swordfish \
--to=sergey.senozhatsky.work@gmail.com \
--cc=ddstreet@ieee.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=sergey.senozhatsky@gmail.com \
--cc=vitalywool@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).