zram memory control enhance

zs_get_total_size_bytes returns a amount of memory zsmalloc consumed with *byte unit* but zsmalloc operates *page unit* rather than byte unit so let's change the API so benefit we could get is that reduce unnecessary overhead (ie, change page unit with byte unit) in zsmalloc.

Now, zswap can rollback to zswap_pool_pages.
Over to zswap guys ;-)

Signed-off-by Minchan Kim <[email protected]>

---
 drivers/block/zram/zram_drv.c |  4 ++--
 include/linux/zsmalloc.h      |  2 +-
 mm/zsmalloc.c                 | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

drivers/block/zram/zram_drv.c +2 -2

index d00831c3d731..302dd37bcea3 100644
@@ -103,10 +103,10 @@ static ssize_t mem_used_total_show(struct device *dev,
 
        down_read(&zram->init_lock);
        if (init_done(zram))
-               val = zs_get_total_size_bytes(meta->mem_pool);
+               val = zs_get_total_size(meta->mem_pool);
        up_read(&zram->init_lock);
 
-       return scnprintf(buf, PAGE_SIZE, "%llu\n", val);
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
 }
 
 static ssize_t max_comp_streams_show(struct device *dev,

include/linux/zsmalloc.h +0 -0

index e44d634e7fb7..105b56e45d23 100644

include/linux/zsmalloc.h +0 -0

 #endif

mm/zsmalloc.c +5 -6

index a65924255763..80408a1da03a 100644
@@ -299,7 +299,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle)
 
 static u64 zs_zpool_total_size(void *pool)
 {
-       return zs_get_total_size_bytes(pool);
+       return zs_get_total_size(pool) << PAGE_SHIFT;
 }
 
 static struct zpool_driver zs_zpool_driver = {

@@ -1186,16 +1186,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
-u64 zs_get_total_size_bytes(struct zs_pool *pool)
+unsigned long zs_get_total_size(struct zs_pool *pool)
 {
-       u64 npages;
+       unsigned long npages;
 
        spin_lock(&pool->stat_lock);
        npages = pool->pages_allocated;
        spin_unlock(&pool->stat_lock);
-       return npages << PAGE_SHIFT;
+       return npages;
 }
-EXPORT_SYMBOL_GPL(zs_get_total_size_bytes);
+EXPORT_SYMBOL_GPL(zs_get_total_size);
 
 module_init(zs_init);
 module_exit(zs_exit);
--

2.0.0

Quoted 8 lines · Minchan Kim

zs_get_total_size_bytes returns a amount of memory zsmalloc consumed with *byte unit* but zsmalloc operates *page unit* rather than byte unit so let's change the API so benefit we could get is that reduce unnecessary overhead (ie, change page unit with byte unit) in zsmalloc.

Now, zswap can rollback to zswap_pool_pages.
Over to zswap guys ;-)

We could change zpool/zswap over to total pages instead of total bytes, since both zbud and zsmalloc now report size in pages. The only downside would be if either changed later to not use only whole pages (or if they start using huge pages for storage...), but for what they do that seems unlikely. After this patch is finalized I can write up a quick patch unless Seth disagrees (or already has a patch :)

Quoted 31 lines

Signed-off-by Minchan Kim <[email protected]>

---
 drivers/block/zram/zram_drv.c |  4 ++--
 include/linux/zsmalloc.h      |  2 +-
 mm/zsmalloc.c                 | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

drivers/block/zram/zram_drv.c +2 -2

index d00831c3d731..302dd37bcea3 100644
@@ -103,10 +103,10 @@ static ssize_t mem_used_total_show(struct device *dev,
 
        down_read(&zram->init_lock);
        if (init_done(zram))
-               val = zs_get_total_size_bytes(meta->mem_pool);
+               val = zs_get_total_size(meta->mem_pool);
        up_read(&zram->init_lock);
 
-       return scnprintf(buf, PAGE_SIZE, "%llu\n", val);
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
 }
 
 static ssize_t max_comp_streams_show(struct device *dev,

include/linux/zsmalloc.h +0 -0

index e44d634e7fb7..105b56e45d23 100644

include/linux/zsmalloc.h void *zs_map_object(struct zs_pool *pool, unsigned long handle, · line 49

Quoted 5 lines

include/linux/zsmalloc.h +1 -1

@@ -46,6 +46,6 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
                        enum zs_mapmode mm);
 void zs_unmap_object(struct zs_pool *pool, unsigned long handle);
 
-u64 zs_get_total_size_bytes(struct zs_pool *pool);
+unsigned long zs_get_total_size(struct zs_pool *pool);

minor naming suggestion, but since the name is changing anyway, "zs_get_total_size" implies to me the units are bytes, would "zs_get_total_pages" be clearer that it's returning size in # of pages, not bytes?

Quoted 34 lines

include/linux/zsmalloc.h +0 -0

 #endif

mm/zsmalloc.c +5 -5

index a65924255763..80408a1da03a 100644
@@ -299,7 +299,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle)
 
 static u64 zs_zpool_total_size(void *pool)
 {
-       return zs_get_total_size_bytes(pool);
+       return zs_get_total_size(pool) << PAGE_SHIFT;
 }
 
 static struct zpool_driver zs_zpool_driver = {

@@ -1186,16 +1186,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
-u64 zs_get_total_size_bytes(struct zs_pool *pool)
+unsigned long zs_get_total_size(struct zs_pool *pool)
 {
-       u64 npages;
+       unsigned long npages;
 
        spin_lock(&pool->stat_lock);
        npages = pool->pages_allocated;
        spin_unlock(&pool->stat_lock);
-       return npages << PAGE_SHIFT;
+       return npages;
 }
-EXPORT_SYMBOL_GPL(zs_get_total_size_bytes);
+EXPORT_SYMBOL_GPL(zs_get_total_size);
 

Quoted 5 lines

mm/zsmalloc.c +0 -1

 module_init(zs_init);
 module_exit(zs_exit);
--

2.0.0

Quoted 8 lines · Minchan Kim

zs_get_total_size_bytes returns a amount of memory zsmalloc consumed with *byte unit* but zsmalloc operates *page unit* rather than byte unit so let's change the API so benefit we could get is that reduce unnecessary overhead (ie, change page unit with byte unit) in zsmalloc.

Now, zswap can rollback to zswap_pool_pages.
Over to zswap guys ;-)

We could change zpool/zswap over to total pages instead of total bytes, since both zbud and zsmalloc now report size in pages. The only downside would be if either changed later to not use only whole pages (or if they start using huge pages for storage...), but for what they do that seems unlikely. After this patch is finalized I can write up a quick patch unless Seth disagrees (or already has a patch :)

Quoted 33 lines

Signed-off-by Minchan Kim <[email protected]>

---
 drivers/block/zram/zram_drv.c |  4 ++--
 include/linux/zsmalloc.h      |  2 +-
 mm/zsmalloc.c                 | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

drivers/block/zram/zram_drv.c +2 -2

index d00831c3d731..302dd37bcea3 100644
@@ -103,10 +103,10 @@ static ssize_t mem_used_total_show(struct device *dev,
 
        down_read(&zram->init_lock);
        if (init_done(zram))
-               val = zs_get_total_size_bytes(meta->mem_pool);
+               val = zs_get_total_size(meta->mem_pool);
        up_read(&zram->init_lock);
 
-       return scnprintf(buf, PAGE_SIZE, "%llu\n", val);
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
 }
 
 static ssize_t max_comp_streams_show(struct device *dev,

include/linux/zsmalloc.h +0 -0

index e44d634e7fb7..105b56e45d23 100644
@@ -46,6 +46,6 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
                        enum zs_mapmode mm);
 void zs_unmap_object(struct zs_pool *pool, unsigned long handle);
 

include/linux/zsmalloc.h void *zs_map_object(struct zs_pool *pool, unsigned long handle, · line 49

Quoted 2 lines

include/linux/zsmalloc.h +1 -1

-u64 zs_get_total_size_bytes(struct zs_pool *pool);
+unsigned long zs_get_total_size(struct zs_pool *pool);

minor naming suggestion, but since the name is changing anyway, "zs_get_total_size" implies to me the units are bytes, would "zs_get_total_pages" be clearer that it's returning size in # of pages, not bytes?

Quoted 8 lines · Minchan Kim

zs_get_total_size_bytes returns a amount of memory zsmalloc consumed with *byte unit* but zsmalloc operates *page unit* rather than byte unit so let's change the API so benefit we could get is that reduce unnecessary overhead (ie, change page unit with byte unit) in zsmalloc.

Now, zswap can rollback to zswap_pool_pages.
Over to zswap guys ;-)

We could change zpool/zswap over to total pages instead of total bytes, since both zbud and zsmalloc now report size in pages. The only downside would be if either changed later to not use only whole pages (or if they start using huge pages for storage...), but for what they do that seems unlikely. After this patch is finalized I can write up a quick patch unless Seth disagrees (or already has a patch :)

Quoted 31 lines

Signed-off-by Minchan Kim <[email protected]>

---
 drivers/block/zram/zram_drv.c |  4 ++--
 include/linux/zsmalloc.h      |  2 +-
 mm/zsmalloc.c                 | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

drivers/block/zram/zram_drv.c +2 -2

index d00831c3d731..302dd37bcea3 100644
@@ -103,10 +103,10 @@ static ssize_t mem_used_total_show(struct device *dev,
 
        down_read(&zram->init_lock);
        if (init_done(zram))
-               val = zs_get_total_size_bytes(meta->mem_pool);
+               val = zs_get_total_size(meta->mem_pool);
        up_read(&zram->init_lock);
 
-       return scnprintf(buf, PAGE_SIZE, "%llu\n", val);
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
 }
 
 static ssize_t max_comp_streams_show(struct device *dev,

include/linux/zsmalloc.h +0 -0

index e44d634e7fb7..105b56e45d23 100644

include/linux/zsmalloc.h void *zs_map_object(struct zs_pool *pool, unsigned long handle, · line 49

Quoted 5 lines

include/linux/zsmalloc.h +1 -1

@@ -46,6 +46,6 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
                        enum zs_mapmode mm);
 void zs_unmap_object(struct zs_pool *pool, unsigned long handle);
 
-u64 zs_get_total_size_bytes(struct zs_pool *pool);
+unsigned long zs_get_total_size(struct zs_pool *pool);

minor naming suggestion, but since the name is changing anyway, "zs_get_total_size" implies to me the units are bytes, would "zs_get_total_pages" be clearer that it's returning size in # of pages, not bytes?

Quoted 34 lines

include/linux/zsmalloc.h +0 -0

 #endif

mm/zsmalloc.c +5 -5

index a65924255763..80408a1da03a 100644
@@ -299,7 +299,7 @@ static void zs_zpool_unmap(void *pool, unsigned long handle)
 
 static u64 zs_zpool_total_size(void *pool)
 {
-       return zs_get_total_size_bytes(pool);
+       return zs_get_total_size(pool) << PAGE_SHIFT;
 }
 
 static struct zpool_driver zs_zpool_driver = {

@@ -1186,16 +1186,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 }
 EXPORT_SYMBOL_GPL(zs_unmap_object);
 
-u64 zs_get_total_size_bytes(struct zs_pool *pool)
+unsigned long zs_get_total_size(struct zs_pool *pool)
 {
-       u64 npages;
+       unsigned long npages;
 
        spin_lock(&pool->stat_lock);
        npages = pool->pages_allocated;
        spin_unlock(&pool->stat_lock);
-       return npages << PAGE_SHIFT;
+       return npages;
 }
-EXPORT_SYMBOL_GPL(zs_get_total_size_bytes);
+EXPORT_SYMBOL_GPL(zs_get_total_size);
 

Quoted 5 lines

mm/zsmalloc.c +0 -1

 module_init(zs_init);
 module_exit(zs_exit);
--

2.0.0

Since zram has no control feature to limit memory usage, it makes hard to manage system memrory.

This patch adds new knob "mem_limit" via sysfs to set up the a limit so that zram could fail allocation once it reaches the limit.

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram |  9 +++++++
 Documentation/blockdev/zram.txt            | 20 ++++++++++++---
 drivers/block/zram/zram_drv.c              | 41 ++++++++++++++++++++++++++++++
 drivers/block/zram/zram_drv.h              |  5 ++++
 4 files changed, 71 insertions(+), 4 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +0 -0

index 70ec992514d0..025331c19045 100644
@@ -119,3 +119,12 @@ Description:
                efficiency can be calculated using compr_data_size and this

Documentation/ABI/testing/sysfs-block-zram +1 -0

+               Unit: bytes

Documentation/blockdev/zram.txt +1 -1

index 0595c3f56ccf..9f239ff8c444 100644
@@ -74,14 +74,26 @@ There is little point creating a zram of greater than twice the size of memory
 since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
 size of the disk when not in use so a huge zram is wasteful.
 
-5) Activate:
+5) Set memory limit: Optional

Documentation/blockdev/zram.txt +5 -3

+
+6) Activate:
        mkswap /dev/zram0
        swapon /dev/zram0
 
        mkfs.ext4 /dev/zram1
        mount /dev/zram1 /tmp
 
-6) Stats:
+7) Stats:
        Per-device statistics are exported as various nodes under
        /sys/block/zram<id>/
                disksize

@@ -96,11 +108,11 @@ size of the disk when not in use so a huge zram is wasteful.
                compr_data_size
                mem_used_total
 
-7) Deactivate:
+8) Deactivate:
        swapoff /dev/zram0
        umount /dev/zram1
 
-8) Reset:
+9) Reset:
        Write any positive value to 'reset' sysfs node
        echo 1 > /sys/block/zram0/reset
        echo 1 > /sys/block/zram1/reset

drivers/block/zram/zram_drv.c +41 -0

index 302dd37bcea3..adc91c7ecaef 100644
@@ -122,6 +122,33 @@ 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_pages;
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+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);
+
+       limit = memparse(buf, NULL);
+       down_write(&zram->init_lock);
+       zram->limit_pages = PAGE_ALIGN(limit) >> PAGE_SHIFT;
+       up_write(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                ret = -ENOMEM;
                goto out;
        }
+
+       if (zram->limit_pages &&
+               zs_get_total_size(meta->mem_pool) > zram->limit_pages) {
+               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)) {

@@ -617,6 +652,9 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
        struct zram_meta *meta;
 
        down_write(&zram->init_lock);
+
+       zram->limit_pages = 0;
+
        if (!init_done(zram)) {
                up_write(&zram->init_lock);
                return;

@@ -857,6 +895,8 @@ static DEVICE_ATTR(initstate, S_IRUGO, initstate_show, NULL);
 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_limit, S_IRUGO | S_IWUSR, 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,

@@ -885,6 +925,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_orig_data_size.attr,
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
+       &dev_attr_mem_limit.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +5 -1

index e0f725c87cc6..b7aa9c21553f 100644
@@ -112,6 +112,11 @@ struct zram {
        u64 disksize;   /* bytes */
        int max_comp_streams;
        struct zram_stats stats;
+       /*
+        * the number of pages zram can consume for storing compressed data
+        */
+       unsigned long limit_pages;
+
        char compressor[10];
 };
 #endif
--

2.0.0

Quoted 26 lines · Minchan Kim

Since zram has no control feature to limit memory usage, it makes hard to manage system memrory.

This patch adds new knob "mem_limit" via sysfs to set up the a limit so that zram could fail allocation once it reaches the limit.

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram |  9 +++++++
 Documentation/blockdev/zram.txt            | 20 ++++++++++++---
 drivers/block/zram/zram_drv.c              | 41 ++++++++++++++++++++++++++++++
 drivers/block/zram/zram_drv.h              |  5 ++++
 4 files changed, 71 insertions(+), 4 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +3 -0

index 70ec992514d0..025331c19045 100644
@@ -119,3 +119,12 @@ Description:
                efficiency can be calculated using compr_data_size and this
                statistic.
                Unit: bytes
+
+What:          /sys/block/zram<id>/mem_limit
+Date:          August 2014

Documentation/ABI/testing/sysfs-block-zram Description: · line 129

Quoted 5 lines

Documentation/ABI/testing/sysfs-block-zram +5 -0

+Contact:       Minchan Kim <[email protected]>
+Description:
+               The mem_limit file is read/write and specifies the amount
+               of memory to be able to consume memory to store store
+               compressed data.

might want to clarify here that the value "0", which is the default, disables the limit.

Quoted 10 lines

Documentation/ABI/testing/sysfs-block-zram +1 -0

+               Unit: bytes

Documentation/blockdev/zram.txt +0 -0

index 0595c3f56ccf..9f239ff8c444 100644
@@ -74,14 +74,26 @@ There is little point creating a zram of greater than twice the size of memory
 since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
 size of the disk when not in use so a huge zram is wasteful.
 

Documentation/blockdev/zram.txt There is little point creating a zram of greater than twice the size of memory · line 87

Quoted 12 lines

Documentation/blockdev/zram.txt +11 -1

-5) Activate:
+5) Set memory limit: Optional
+       Set memory limit by writing the value to sysfs node 'mem_limit'.
+       The value can be either in bytes or you can use mem suffixes.
+       Examples:
+           # limit /dev/zram0 with 50MB memory
+           echo $((50*1024*1024)) > /sys/block/zram0/mem_limit
+
+           # Using mem suffixes
+           echo 256K > /sys/block/zram0/mem_limit
+           echo 512M > /sys/block/zram0/mem_limit
+           echo 1G > /sys/block/zram0/mem_limit

# To disable memory limit
echo 0 > /sys/block/zram0/mem_limit

Quoted 119 lines

Documentation/blockdev/zram.txt +5 -3

+
+6) Activate:
        mkswap /dev/zram0
        swapon /dev/zram0
 
        mkfs.ext4 /dev/zram1
        mount /dev/zram1 /tmp
 
-6) Stats:
+7) Stats:
        Per-device statistics are exported as various nodes under
        /sys/block/zram<id>/
                disksize

@@ -96,11 +108,11 @@ size of the disk when not in use so a huge zram is wasteful.
                compr_data_size
                mem_used_total
 
-7) Deactivate:
+8) Deactivate:
        swapoff /dev/zram0
        umount /dev/zram1
 
-8) Reset:
+9) Reset:
        Write any positive value to 'reset' sysfs node
        echo 1 > /sys/block/zram0/reset
        echo 1 > /sys/block/zram1/reset

drivers/block/zram/zram_drv.c +41 -0

index 302dd37bcea3..adc91c7ecaef 100644
@@ -122,6 +122,33 @@ 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_pages;
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+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);
+
+       limit = memparse(buf, NULL);
+       down_write(&zram->init_lock);
+       zram->limit_pages = PAGE_ALIGN(limit) >> PAGE_SHIFT;
+       up_write(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                ret = -ENOMEM;
                goto out;
        }
+
+       if (zram->limit_pages &&
+               zs_get_total_size(meta->mem_pool) > zram->limit_pages) {
+               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)) {

@@ -617,6 +652,9 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
        struct zram_meta *meta;
 
        down_write(&zram->init_lock);
+
+       zram->limit_pages = 0;
+
        if (!init_done(zram)) {
                up_write(&zram->init_lock);
                return;

@@ -857,6 +895,8 @@ static DEVICE_ATTR(initstate, S_IRUGO, initstate_show, NULL);
 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_limit, S_IRUGO | S_IWUSR, 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,

@@ -885,6 +925,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_orig_data_size.attr,
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
+       &dev_attr_mem_limit.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +3 -0

index e0f725c87cc6..b7aa9c21553f 100644
@@ -112,6 +112,11 @@ struct zram {
        u64 disksize;   /* bytes */
        int max_comp_streams;
        struct zram_stats stats;
+       /*
+        * the number of pages zram can consume for storing compressed data
+        */

Quoted 8 lines

drivers/block/zram/zram_drv.h +2 -1

+       unsigned long limit_pages;
+
        char compressor[10];
 };
 #endif
--

2.0.0

Normally, zram user could get maximum memory usage zram consumed via polling mem_used_total with sysfs in userspace.

But it has a critical problem because user can miss peak memory usage during update inverval of polling. For avoiding that, user should poll it with shorter interval(ie, 0.0000000001s) with mlocking to avoid page fault delay when memory pressure is heavy. It would be troublesome.

This patch adds new knob "mem_used_max" so user could see the maximum memory usage easily via reading the knob and reset it via "echo 0 > /sys/block/zram0/mem_used_max".

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++
 Documentation/blockdev/zram.txt            |  1 +
 drivers/block/zram/zram_drv.c              | 57 ++++++++++++++++++++++++++++--
 drivers/block/zram/zram_drv.h              |  1 +
 4 files changed, 67 insertions(+), 2 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +10 -0

index 025331c19045..ffd1ea7443dd 100644
@@ -120,6 +120,16 @@ Description:
                statistic.
                Unit: bytes
 
+What:          /sys/block/zram<id>/mem_used_max
+Date:          August 2014
+Contact:       Minchan Kim <[email protected]>
+Description:
+               The mem_used_max file is read/write and specifies the amount
+               of maximum memory zram have consumed to store compressed data.
+               For resetting the value, you should do "echo 0". Otherwise,
+               you could see -EINVAL.
+               Unit: bytes
+
 What:          /sys/block/zram<id>/mem_limit
 Date:          August 2014
 Contact:       Minchan Kim <[email protected]>

Documentation/blockdev/zram.txt +1 -0

index 9f239ff8c444..3b2247c2d4cf 100644
@@ -107,6 +107,7 @@ size of the disk when not in use so a huge zram is wasteful.
                orig_data_size
                compr_data_size
                mem_used_total
+               mem_used_max
 
 8) Deactivate:
        swapoff /dev/zram0

drivers/block/zram/zram_drv.c +16 -0

index adc91c7ecaef..138787579478 100644
@@ -149,6 +149,41 @@ static ssize_t mem_limit_store(struct device *dev,
        return len;
 }
 
+static ssize_t mem_used_max_show(struct device *dev,
+               struct device_attribute *attr, char *buf)
+{
+       u64 val = 0;
+       struct zram *zram = dev_to_zram(dev);
+
+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               val = atomic64_read(&zram->stats.max_used_pages);
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+static ssize_t mem_used_max_store(struct device *dev,
+               struct device_attribute *attr, const char *buf, size_t len)

drivers/block/zram/zram_drv.c +11 -0

+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               atomic64_set(&zram->stats.max_used_pages,
+                               zs_get_total_size(meta->mem_pool));
+       up_read(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -461,6 +496,18 @@ out_cleanup:
        return ret;
 }
 
+static inline void update_used_max(struct zram *zram, const unsigned long pages)
+{

drivers/block/zram/zram_drv.c +1 -1

 static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                           int offset)
 {

@@ -472,6 +519,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
        struct zram_meta *meta = zram->meta;
        struct zcomp_strm *zstrm;
        bool locked = false;
+       unsigned long alloced_pages;
 
        page = bvec->bv_page;
        if (is_partial_io(bvec)) {

@@ -541,13 +589,15 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                goto out;
        }
 
-       if (zram->limit_pages &&

drivers/block/zram/zram_drv.c +3 -0

        cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
 
        if ((clen == PAGE_SIZE) && !is_partial_io(bvec)) {

@@ -897,6 +947,8 @@ 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_limit, S_IRUGO | S_IWUSR, mem_limit_show,
                mem_limit_store);
+static DEVICE_ATTR(mem_used_max, S_IRUGO | S_IWUSR, mem_used_max_show,
+               mem_used_max_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,

@@ -926,6 +978,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
        &dev_attr_mem_limit.attr,
+       &dev_attr_mem_used_max.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +1 -1

index b7aa9c21553f..29383312d543 100644
@@ -90,6 +90,7 @@ struct zram_stats {
        atomic64_t notify_free; /* no. of swap slot free notifications */
        atomic64_t zero_pages;          /* no. of zero filled pages */
        atomic64_t pages_stored;        /* no. of pages currently stored */
+       atomic64_t max_used_pages;      /* no. of maximum pages stored */
 };
 
 struct zram_meta {
--

2.0.0

Quoted 78 lines · Minchan Kim

Normally, zram user could get maximum memory usage zram consumed via polling mem_used_total with sysfs in userspace.

But it has a critical problem because user can miss peak memory usage during update inverval of polling. For avoiding that, user should poll it with shorter interval(ie, 0.0000000001s) with mlocking to avoid page fault delay when memory pressure is heavy. It would be troublesome.

This patch adds new knob "mem_used_max" so user could see the maximum memory usage easily via reading the knob and reset it via "echo 0 > /sys/block/zram0/mem_used_max".

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++
 Documentation/blockdev/zram.txt            |  1 +
 drivers/block/zram/zram_drv.c              | 57 ++++++++++++++++++++++++++++--
 drivers/block/zram/zram_drv.h              |  1 +
 4 files changed, 67 insertions(+), 2 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +10 -0

index 025331c19045..ffd1ea7443dd 100644
@@ -120,6 +120,16 @@ Description:
                statistic.
                Unit: bytes
 
+What:          /sys/block/zram<id>/mem_used_max
+Date:          August 2014
+Contact:       Minchan Kim <[email protected]>
+Description:
+               The mem_used_max file is read/write and specifies the amount
+               of maximum memory zram have consumed to store compressed data.
+               For resetting the value, you should do "echo 0". Otherwise,
+               you could see -EINVAL.
+               Unit: bytes
+
 What:          /sys/block/zram<id>/mem_limit
 Date:          August 2014
 Contact:       Minchan Kim <[email protected]>

Documentation/blockdev/zram.txt +1 -0

index 9f239ff8c444..3b2247c2d4cf 100644
@@ -107,6 +107,7 @@ size of the disk when not in use so a huge zram is wasteful.
                orig_data_size
                compr_data_size
                mem_used_total
+               mem_used_max
 
 8) Deactivate:
        swapoff /dev/zram0

drivers/block/zram/zram_drv.c +16 -0

index adc91c7ecaef..138787579478 100644
@@ -149,6 +149,41 @@ static ssize_t mem_limit_store(struct device *dev,
        return len;
 }
 
+static ssize_t mem_used_max_show(struct device *dev,
+               struct device_attribute *attr, char *buf)
+{
+       u64 val = 0;
+       struct zram *zram = dev_to_zram(dev);
+
+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               val = atomic64_read(&zram->stats.max_used_pages);
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+static ssize_t mem_used_max_store(struct device *dev,
+               struct device_attribute *attr, const char *buf, size_t len)

drivers/block/zram/zram_drv.c static ssize_t mem_limit_store(struct device *dev, · line 177

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+{
+       int err;
+       unsigned long val;
+       struct zram *zram = dev_to_zram(dev);
+       struct zram_meta *meta = zram->meta;
+
+       err = kstrtoul(buf, 10, &val);
+       if (err || val != 0)
+               return -EINVAL;
+

Yes - this works better for the user than explicit single "0" check Thanks for testing.

Quoted 16 lines

drivers/block/zram/zram_drv.c +9 -0

+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               atomic64_set(&zram->stats.max_used_pages,
+                               zs_get_total_size(meta->mem_pool));
+       up_read(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -461,6 +496,18 @@ out_cleanup:
        return ret;
 }
 

drivers/block/zram/zram_drv.c out_cleanup: · line 510

Quoted 12 lines

drivers/block/zram/zram_drv.c +12 -0

+static inline void update_used_max(struct zram *zram, const unsigned long pages)
+{
+       u64 old_max, cur_max;
+
+       do {
+               old_max = cur_max = atomic64_read(&zram->stats.max_used_pages);
+               if (pages > cur_max)
+                       old_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       cur_max, pages);
+       } while (old_max != cur_max);
+}
+

This can be tightened up some:

+static inline void update_used_max(struct zram *zram, const unsigned long pages)

(quoted diff) +11 -0

+{
+       u64 prev_max, old_max = 0;
+
+       prev_max = atomic64_read(&zram->stats.max_used_pages);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       old_max, pages);
+       };
+}
+

And then can be generalized to:

(quoted diff) +12 -0

+static inline void update_max(uint64_t *addr, const uint64_t newvalue)
+{
+       uint64_t prev_max, old_max = 0;
+
+       prev_max = atomic64_read(addr);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(addr,
+                                       old_max, newvalue);
+       };
+}
+

Called with : update_max(&zram->stats.max_used_pages, alloced_pages).

(I hope I got all the datatypes (I understand there are some implicit casts) and modes correct).

The idea should be clear.

Quoted 16 lines

drivers/block/zram/zram_drv.c +1 -1

 static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                           int offset)
 {

@@ -472,6 +519,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
        struct zram_meta *meta = zram->meta;
        struct zcomp_strm *zstrm;
        bool locked = false;
+       unsigned long alloced_pages;
 
        page = bvec->bv_page;
        if (is_partial_io(bvec)) {

@@ -541,13 +589,15 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                goto out;
        }
 
-       if (zram->limit_pages &&

drivers/block/zram/zram_drv.c static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, · line 600

Quoted 10 lines

drivers/block/zram/zram_drv.c +4 -1

-               zs_get_total_size(meta->mem_pool) > zram->limit_pages) {
+       alloced_pages = zs_get_total_size(meta->mem_pool);
+       if (zram->limit_pages && alloced_pages > zram->limit_pages) {
                zs_free(meta->mem_pool, handle);
                ret = -ENOMEM;
                goto out;
        }
 
+       update_used_max(zram, alloced_pages);
+

or generalized with update_max

Quoted 33 lines

drivers/block/zram/zram_drv.c +3 -0

        cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
 
        if ((clen == PAGE_SIZE) && !is_partial_io(bvec)) {

@@ -897,6 +947,8 @@ 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_limit, S_IRUGO | S_IWUSR, mem_limit_show,
                mem_limit_store);
+static DEVICE_ATTR(mem_used_max, S_IRUGO | S_IWUSR, mem_used_max_show,
+               mem_used_max_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,

@@ -926,6 +978,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
        &dev_attr_mem_limit.attr,
+       &dev_attr_mem_used_max.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +1 -1

index b7aa9c21553f..29383312d543 100644
@@ -90,6 +90,7 @@ struct zram_stats {
        atomic64_t notify_free; /* no. of swap slot free notifications */
        atomic64_t zero_pages;          /* no. of zero filled pages */
        atomic64_t pages_stored;        /* no. of pages currently stored */
+       atomic64_t max_used_pages;      /* no. of maximum pages stored */
 };
 
 struct zram_meta {
--

Quoted 2 lines

2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Quoted 109 lines · David Horner

Quoted 78 lines · Minchan Kim

Normally, zram user could get maximum memory usage zram consumed via polling mem_used_total with sysfs in userspace.

But it has a critical problem because user can miss peak memory usage during update inverval of polling. For avoiding that, user should poll it with shorter interval(ie, 0.0000000001s) with mlocking to avoid page fault delay when memory pressure is heavy. It would be troublesome.

This patch adds new knob "mem_used_max" so user could see the maximum memory usage easily via reading the knob and reset it via "echo 0 > /sys/block/zram0/mem_used_max".

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++
 Documentation/blockdev/zram.txt            |  1 +
 drivers/block/zram/zram_drv.c              | 57 ++++++++++++++++++++++++++++--
 drivers/block/zram/zram_drv.h              |  1 +
 4 files changed, 67 insertions(+), 2 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +10 -0

index 025331c19045..ffd1ea7443dd 100644
@@ -120,6 +120,16 @@ Description:
                statistic.
                Unit: bytes
 
+What:          /sys/block/zram<id>/mem_used_max
+Date:          August 2014
+Contact:       Minchan Kim <[email protected]>
+Description:
+               The mem_used_max file is read/write and specifies the amount
+               of maximum memory zram have consumed to store compressed data.
+               For resetting the value, you should do "echo 0". Otherwise,
+               you could see -EINVAL.
+               Unit: bytes
+
 What:          /sys/block/zram<id>/mem_limit
 Date:          August 2014
 Contact:       Minchan Kim <[email protected]>

Documentation/blockdev/zram.txt +1 -0

index 9f239ff8c444..3b2247c2d4cf 100644
@@ -107,6 +107,7 @@ size of the disk when not in use so a huge zram is wasteful.
                orig_data_size
                compr_data_size
                mem_used_total
+               mem_used_max
 
 8) Deactivate:
        swapoff /dev/zram0

drivers/block/zram/zram_drv.c +16 -0

index adc91c7ecaef..138787579478 100644
@@ -149,6 +149,41 @@ static ssize_t mem_limit_store(struct device *dev,
        return len;
 }
 
+static ssize_t mem_used_max_show(struct device *dev,
+               struct device_attribute *attr, char *buf)
+{
+       u64 val = 0;
+       struct zram *zram = dev_to_zram(dev);
+
+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               val = atomic64_read(&zram->stats.max_used_pages);
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+static ssize_t mem_used_max_store(struct device *dev,
+               struct device_attribute *attr, const char *buf, size_t len)

drivers/block/zram/zram_drv.c static ssize_t mem_limit_store(struct device *dev, · line 177

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+{
+       int err;
+       unsigned long val;
+       struct zram *zram = dev_to_zram(dev);
+       struct zram_meta *meta = zram->meta;
+
+       err = kstrtoul(buf, 10, &val);
+       if (err || val != 0)
+               return -EINVAL;
+

Yes - this works better for the user than explicit single "0" check Thanks for testing.

Quoted 16 lines

drivers/block/zram/zram_drv.c +9 -0

+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               atomic64_set(&zram->stats.max_used_pages,
+                               zs_get_total_size(meta->mem_pool));
+       up_read(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -461,6 +496,18 @@ out_cleanup:
        return ret;
 }
 

Quoted 14 lines

drivers/block/zram/zram_drv.c out_cleanup: · line 510

Quoted 12 lines

drivers/block/zram/zram_drv.c +12 -0

+static inline void update_used_max(struct zram *zram, const unsigned long pages)
+{
+       u64 old_max, cur_max;
+
+       do {
+               old_max = cur_max = atomic64_read(&zram->stats.max_used_pages);
+               if (pages > cur_max)
+                       old_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       cur_max, pages);
+       } while (old_max != cur_max);
+}
+

This can be tightened up some:

How many does it make tight?
If it's not a big, I'd like to stick my version.

Quoted 16 lines

+static inline void update_used_max(struct zram *zram, const unsigned long pages)

(quoted diff) +11 -0

+{
+       u64 prev_max, old_max = 0;
+
+       prev_max = atomic64_read(&zram->stats.max_used_pages);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       old_max, pages);
+       };
+}
+

Quoted 1 line

And then can be generalized to:

At the moment, we don't need it but it would be good if we need another max in future so I will keep in mind. Thanks for the suggestion!

Quoted 83 lines

(quoted diff) +12 -0

+static inline void update_max(uint64_t *addr, const uint64_t newvalue)
+{
+       uint64_t prev_max, old_max = 0;
+
+       prev_max = atomic64_read(addr);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(addr,
+                                       old_max, newvalue);
+       };
+}
+

Called with : update_max(&zram->stats.max_used_pages, alloced_pages).

(I hope I got all the datatypes (I understand there are some implicit casts) and modes correct).

The idea should be clear.

Quoted 16 lines

drivers/block/zram/zram_drv.c +1 -1

 static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                           int offset)
 {

@@ -472,6 +519,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
        struct zram_meta *meta = zram->meta;
        struct zcomp_strm *zstrm;
        bool locked = false;
+       unsigned long alloced_pages;
 
        page = bvec->bv_page;
        if (is_partial_io(bvec)) {

@@ -541,13 +589,15 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                goto out;
        }
 
-       if (zram->limit_pages &&

drivers/block/zram/zram_drv.c static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, · line 600

Quoted 10 lines

drivers/block/zram/zram_drv.c +4 -1

-               zs_get_total_size(meta->mem_pool) > zram->limit_pages) {
+       alloced_pages = zs_get_total_size(meta->mem_pool);
+       if (zram->limit_pages && alloced_pages > zram->limit_pages) {
                zs_free(meta->mem_pool, handle);
                ret = -ENOMEM;
                goto out;
        }
 
+       update_used_max(zram, alloced_pages);
+

or generalized with update_max

Quoted 31 lines

drivers/block/zram/zram_drv.c +3 -0

        cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
 
        if ((clen == PAGE_SIZE) && !is_partial_io(bvec)) {

@@ -897,6 +947,8 @@ 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_limit, S_IRUGO | S_IWUSR, mem_limit_show,
                mem_limit_store);
+static DEVICE_ATTR(mem_used_max, S_IRUGO | S_IWUSR, mem_used_max_show,
+               mem_used_max_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,

@@ -926,6 +978,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
        &dev_attr_mem_limit.attr,
+       &dev_attr_mem_used_max.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +1 -0

index b7aa9c21553f..29383312d543 100644
@@ -90,6 +90,7 @@ struct zram_stats {
        atomic64_t notify_free; /* no. of swap slot free notifications */
        atomic64_t zero_pages;          /* no. of zero filled pages */
        atomic64_t pages_stored;        /* no. of pages currently stored */
+       atomic64_t max_used_pages;      /* no. of maximum pages stored */
 };
 

Quoted 10 lines

Quoted 4 lines

drivers/block/zram/zram_drv.h +0 -1

 struct zram_meta {
--

2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
Kind regards,
Minchan Kim

Quoted 125 lines · Minchan Kim

Quoted 109 lines · David Horner

Quoted 78 lines · Minchan Kim

Normally, zram user could get maximum memory usage zram consumed via polling mem_used_total with sysfs in userspace.

But it has a critical problem because user can miss peak memory usage during update inverval of polling. For avoiding that, user should poll it with shorter interval(ie, 0.0000000001s) with mlocking to avoid page fault delay when memory pressure is heavy. It would be troublesome.

This patch adds new knob "mem_used_max" so user could see the maximum memory usage easily via reading the knob and reset it via "echo 0 > /sys/block/zram0/mem_used_max".

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/ABI/testing/sysfs-block-zram | 10 ++++++
 Documentation/blockdev/zram.txt            |  1 +
 drivers/block/zram/zram_drv.c              | 57 ++++++++++++++++++++++++++++--
 drivers/block/zram/zram_drv.h              |  1 +
 4 files changed, 67 insertions(+), 2 deletions(-)

Documentation/ABI/testing/sysfs-block-zram +10 -0

index 025331c19045..ffd1ea7443dd 100644
@@ -120,6 +120,16 @@ Description:
                statistic.
                Unit: bytes
 
+What:          /sys/block/zram<id>/mem_used_max
+Date:          August 2014
+Contact:       Minchan Kim <[email protected]>
+Description:
+               The mem_used_max file is read/write and specifies the amount
+               of maximum memory zram have consumed to store compressed data.
+               For resetting the value, you should do "echo 0". Otherwise,
+               you could see -EINVAL.
+               Unit: bytes
+
 What:          /sys/block/zram<id>/mem_limit
 Date:          August 2014
 Contact:       Minchan Kim <[email protected]>

Documentation/blockdev/zram.txt +1 -0

index 9f239ff8c444..3b2247c2d4cf 100644
@@ -107,6 +107,7 @@ size of the disk when not in use so a huge zram is wasteful.
                orig_data_size
                compr_data_size
                mem_used_total
+               mem_used_max
 
 8) Deactivate:
        swapoff /dev/zram0

drivers/block/zram/zram_drv.c +16 -0

index adc91c7ecaef..138787579478 100644
@@ -149,6 +149,41 @@ static ssize_t mem_limit_store(struct device *dev,
        return len;
 }
 
+static ssize_t mem_used_max_show(struct device *dev,
+               struct device_attribute *attr, char *buf)
+{
+       u64 val = 0;
+       struct zram *zram = dev_to_zram(dev);
+
+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               val = atomic64_read(&zram->stats.max_used_pages);
+       up_read(&zram->init_lock);
+
+       return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+static ssize_t mem_used_max_store(struct device *dev,
+               struct device_attribute *attr, const char *buf, size_t len)

drivers/block/zram/zram_drv.c static ssize_t mem_limit_store(struct device *dev, · line 177

Quoted 10 lines

drivers/block/zram/zram_drv.c +10 -0

+{
+       int err;
+       unsigned long val;
+       struct zram *zram = dev_to_zram(dev);
+       struct zram_meta *meta = zram->meta;
+
+       err = kstrtoul(buf, 10, &val);
+       if (err || val != 0)
+               return -EINVAL;
+

Yes - this works better for the user than explicit single "0" check Thanks for testing.

Quoted 16 lines

drivers/block/zram/zram_drv.c +9 -0

+       down_read(&zram->init_lock);
+       if (init_done(zram))
+               atomic64_set(&zram->stats.max_used_pages,
+                               zs_get_total_size(meta->mem_pool));
+       up_read(&zram->init_lock);
+
+       return len;
+}
+
 static ssize_t max_comp_streams_store(struct device *dev,
                struct device_attribute *attr, const char *buf, size_t len)
 {

@@ -461,6 +496,18 @@ out_cleanup:
        return ret;
 }
 

Quoted 14 lines

drivers/block/zram/zram_drv.c out_cleanup: · line 510

Quoted 12 lines

drivers/block/zram/zram_drv.c +12 -0

+static inline void update_used_max(struct zram *zram, const unsigned long pages)
+{
+       u64 old_max, cur_max;
+
+       do {
+               old_max = cur_max = atomic64_read(&zram->stats.max_used_pages);
+               if (pages > cur_max)
+                       old_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       cur_max, pages);
+       } while (old_max != cur_max);
+}
+

This can be tightened up some:

Quoted 2 lines

How many does it make tight?
If it's not a big, I'd like to stick my version.

It is another atomic (memory barrier) read each time through the loop.

But the expect usual case will be the uncontested case - a drop through.

It would only show up in heavily contested situations -

this is another variation that removes the unnecessary read
but retains the programming structure:and naming:

+static inline void update_used_max(struct zram *zram, const unsigned long pages)
+{
+ u64 old_max, cur_max;
+
+ old_max = atomic64_read(&zram->stats.max_used_pages);
+ do {
+ cur_max = old_max;
+ if (pages > cur_max)
+ old_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+ cur_max, pages);
+ } while (old_max != cur_max);
+}
+

Quoted 115 lines

Quoted 16 lines

+static inline void update_used_max(struct zram *zram, const unsigned long pages)

(quoted diff) +11 -0

+{
+       u64 prev_max, old_max = 0;
+
+       prev_max = atomic64_read(&zram->stats.max_used_pages);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(&zram->stats.max_used_pages,
+                                       old_max, pages);
+       };
+}
+

Quoted 1 line

And then can be generalized to:

At the moment, we don't need it but it would be good if we need another max in future so I will keep in mind. Thanks for the suggestion!

Quoted 83 lines

(quoted diff) +12 -0

+static inline void update_max(uint64_t *addr, const uint64_t newvalue)
+{
+       uint64_t prev_max, old_max = 0;
+
+       prev_max = atomic64_read(addr);
+       do while (pages > prev_max && prev_max != old_max) {
+               old_max = prev_max;
+               prev_max = atomic64_cmpxchg(addr,
+                                       old_max, newvalue);
+       };
+}
+

Called with : update_max(&zram->stats.max_used_pages, alloced_pages).

(I hope I got all the datatypes (I understand there are some implicit casts) and modes correct).

The idea should be clear.

Quoted 16 lines

drivers/block/zram/zram_drv.c +1 -1

 static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                           int offset)
 {

@@ -472,6 +519,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
        struct zram_meta *meta = zram->meta;
        struct zcomp_strm *zstrm;
        bool locked = false;
+       unsigned long alloced_pages;
 
        page = bvec->bv_page;
        if (is_partial_io(bvec)) {

@@ -541,13 +589,15 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
                goto out;
        }
 
-       if (zram->limit_pages &&

drivers/block/zram/zram_drv.c static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index, · line 600

Quoted 10 lines

drivers/block/zram/zram_drv.c +4 -1

-               zs_get_total_size(meta->mem_pool) > zram->limit_pages) {
+       alloced_pages = zs_get_total_size(meta->mem_pool);
+       if (zram->limit_pages && alloced_pages > zram->limit_pages) {
                zs_free(meta->mem_pool, handle);
                ret = -ENOMEM;
                goto out;
        }
 
+       update_used_max(zram, alloced_pages);
+

or generalized with update_max

Quoted 31 lines

drivers/block/zram/zram_drv.c +3 -0

        cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
 
        if ((clen == PAGE_SIZE) && !is_partial_io(bvec)) {

@@ -897,6 +947,8 @@ 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_limit, S_IRUGO | S_IWUSR, mem_limit_show,
                mem_limit_store);
+static DEVICE_ATTR(mem_used_max, S_IRUGO | S_IWUSR, mem_used_max_show,
+               mem_used_max_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,

@@ -926,6 +978,7 @@ static struct attribute *zram_disk_attrs[] = {
        &dev_attr_compr_data_size.attr,
        &dev_attr_mem_used_total.attr,
        &dev_attr_mem_limit.attr,
+       &dev_attr_mem_used_max.attr,
        &dev_attr_max_comp_streams.attr,
        &dev_attr_comp_algorithm.attr,
        NULL,

drivers/block/zram/zram_drv.h +1 -0

index b7aa9c21553f..29383312d543 100644
@@ -90,6 +90,7 @@ struct zram_stats {
        atomic64_t notify_free; /* no. of swap slot free notifications */
        atomic64_t zero_pages;          /* no. of zero filled pages */
        atomic64_t pages_stored;        /* no. of pages currently stored */
+       atomic64_t max_used_pages;      /* no. of maximum pages stored */
 };
 

Quoted 10 lines

Quoted 4 lines

drivers/block/zram/zram_drv.h +0 -1

 struct zram_meta {
--

2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Quoted 3 lines

--
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>