Re: [RFC 3/3] zram: limit memory size for zram
From: Minchan Kim
Date: Wed Aug 13 2014 - 19:27:16 EST
Hey Sergey,
On Tue, Aug 05, 2014 at 10:16:15PM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> On (08/05/14 18:48), Minchan Kim wrote:
> > Another idea: we could define void zs_limit_mem(unsinged long nr_pages)
> > in zsmalloc and put the limit in zs_pool via new API from zram so that
> > zs_malloc could be failed as soon as it exceeds the limit.
> >
> > In the end, zram doesn't need to call zs_get_total_size_bytes on every
> > write. It's more clean and right layer, IMHO.
>
> yes, I think this one is better.
Although I suggested this new one, a few days ago I changed the decision
and was testing the new patchset.
If we add new API for zsmalloc, it adds unnecessary overhead for users who
doesn't care of limit. Although it's cheap, I'd like to avoid that.
The zsmalloc is just allocator so anybody can use it if they want.
But limitation is just requirement of zram who is a one of client
being able to use zsmalloc potentially so accouting should be on zram,
not zsmalloc.
If we might have more users of zsmalloc in future and they all want this
feature that limit of zsmalloc memory usage, we might move the feature
from client to zsmalloc core so everybody would be happy for performance
and readability but opposite would be painful.
In summary, let's keep the accounting logic in client side of zsmalloc(ie,
zram) at the moment but we could move it into zsmalloc core possibly
in future.
Any thoughts?
>
> -ss
>
> > On Tue, Aug 05, 2014 at 05:02:03PM +0900, Minchan Kim wrote:
> > > I have received a request several time from zram users.
> > > They want to limit memory size for zram because zram can consume
> > > lot of memory on system without limit so it makes memory management
> > > control hard.
> > >
> > > This patch adds new knob to limit memory of zram.
> > >
> > > Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> > > ---
> > > Documentation/blockdev/zram.txt | 1 +
> > > drivers/block/zram/zram_drv.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > > drivers/block/zram/zram_drv.h | 1 +
> > > 3 files changed, 43 insertions(+)
> > >
> > > diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
> > > index d24534bee763..fcb0561dfe2e 100644
> > > --- a/Documentation/blockdev/zram.txt
> > > +++ b/Documentation/blockdev/zram.txt
> > > @@ -96,6 +96,7 @@ size of the disk when not in use so a huge zram is wasteful.
> > > compr_data_size
> > > mem_used_total
> > > mem_used_max
> > > + mem_limit
> > >
> > > 7) Deactivate:
> > > swapoff /dev/zram0
> > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > index a4d637b4db7d..47f68bbb2c44 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -137,6 +137,37 @@ static ssize_t max_comp_streams_show(struct device *dev,
> > > return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> > > }
> > >
> > > +static ssize_t mem_limit_show(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + u64 val;
> > > + struct zram *zram = dev_to_zram(dev);
> > > +
> > > + down_read(&zram->init_lock);
> > > + val = zram->limit_bytes;
> > > + up_read(&zram->init_lock);
> > > +
> > > + return scnprintf(buf, PAGE_SIZE, "%llu\n", val);
> > > +}
> > > +
> > > +static ssize_t mem_limit_store(struct device *dev,
> > > + struct device_attribute *attr, const char *buf, size_t len)
> > > +{
> > > + u64 limit;
> > > + struct zram *zram = dev_to_zram(dev);
> > > + int ret;
> > > +
> > > + ret = kstrtoull(buf, 0, &limit);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + down_write(&zram->init_lock);
> > > + zram->limit_bytes = limit;
> > > + ret = len;
> > > + up_write(&zram->init_lock);
> > > + return ret;
> > > +}
> > > +
> > > static ssize_t max_comp_streams_store(struct device *dev,
> > > struct device_attribute *attr, const char *buf, size_t len)
> > > {
> > > @@ -511,6 +542,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> > > ret = -ENOMEM;
> > > goto out;
> > > }
> > > +
> > > + if (zram->limit_bytes &&
> > > + zs_get_total_size_bytes(meta->mem_pool) >= zram->limit_bytes) {
> > > + zs_free(meta->mem_pool, handle);
> > > + ret = -ENOMEM;
> > > + goto out;
> > > + }
> > > +
> > > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > >
> > > if ((clen == PAGE_SIZE) && !is_partial_io(bvec)) {
> > > @@ -854,6 +893,7 @@ static DEVICE_ATTR(reset, S_IWUSR, NULL, reset_store);
> > > static DEVICE_ATTR(orig_data_size, S_IRUGO, orig_data_size_show, NULL);
> > > static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
> > > static DEVICE_ATTR(mem_used_max, S_IRUGO, mem_used_max_show, NULL);
> > > +static DEVICE_ATTR(mem_limit, S_IRUGO, mem_limit_show, mem_limit_store);
> > > static DEVICE_ATTR(max_comp_streams, S_IRUGO | S_IWUSR,
> > > max_comp_streams_show, max_comp_streams_store);
> > > static DEVICE_ATTR(comp_algorithm, S_IRUGO | S_IWUSR,
> > > @@ -883,6 +923,7 @@ static struct attribute *zram_disk_attrs[] = {
> > > &dev_attr_compr_data_size.attr,
> > > &dev_attr_mem_used_total.attr,
> > > &dev_attr_mem_used_max.attr,
> > > + &dev_attr_mem_limit.attr,
> > > &dev_attr_max_comp_streams.attr,
> > > &dev_attr_comp_algorithm.attr,
> > > NULL,
> > > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > > index 7f21c145e317..c0d497ff6efc 100644
> > > --- a/drivers/block/zram/zram_drv.h
> > > +++ b/drivers/block/zram/zram_drv.h
> > > @@ -99,6 +99,7 @@ struct zram {
> > > * we can store in a disk.
> > > */
> > > u64 disksize; /* bytes */
> > > + u64 limit_bytes;
> > > int max_comp_streams;
> > > struct zram_stats stats;
> > > char compressor[10];
> > > --
> > > 2.0.0
> >
> > --
> > Kind regards,
> > Minchan Kim
> >
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/