Re: [PATCH 2/2] zram: gather statistics in a unique file
On 02/11/2013 10:16 AM, Greg Kroah-Hartman wrote: On Mon, Feb 11, 2013 at 10:07:45AM -0800, Davidlohr Bueso wrote: On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: Instead of having one sysfs file per zram statistic, group them all in a single, reader-friendly, 'statistics' file. This not only reduces code but is also makes it easier to visualize. The new file looks like: Number of reads:24 Number of writes: 1055 Invalid IO: 0 Notify free:0 Zero pages: 1042 Orig data size: 49152 bytes Compressed data:838 bytes Total memory used: 53248 bytes Signed-off-by: Davidlohr Bueso No, please, the rule for sysfs is "one value per file", not files with lots of data that you need to parse. Ok. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. So, you would you be open to having the statistics file in debugfs and removing the individual files sysfs? If these are merely debugging information, they should go to debugfs. If they are something that all users need/want, they should stay in sysfs as-is. These zram statistics should stay in sysfs. These stats could be used by userspace programs to trigger different actions. For instance, defrag%, calculated using compressed size and actual memory used, could be used to trigger defrag (not yet implemented). Or low hit rates [number of reads/number of writes] is useful in determining is zram is really helpful or not, in case it's being used as swap. If not, zram should be unloaded or reset to save memory space. Other than this, plain compression ratio stats along with zero_pages are also useful and not really debug info. Though stats are spread across many files, it's easy to script them into a single view: http://compcache.googlecode.com/git/sub-projects/scripts/zram_stats I use this script to gather and display stats for each active zram device. Thanks, Nitin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Mon, Feb 11, 2013 at 10:07:45AM -0800, Davidlohr Bueso wrote: > On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: > > On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: > > > Instead of having one sysfs file per zram statistic, group them all > > > in a single, reader-friendly, 'statistics' file. This not only reduces > > > code but is also makes it easier to visualize. The new file looks like: > > > > > > Number of reads:24 > > > Number of writes: 1055 > > > Invalid IO: 0 > > > Notify free:0 > > > Zero pages: 1042 > > > Orig data size: 49152 bytes > > > Compressed data:838 bytes > > > Total memory used: 53248 bytes > > > > > > Signed-off-by: Davidlohr Bueso > > > > No, please, the rule for sysfs is "one value per file", not files with > > lots of data that you need to parse. > > Ok. > > > > > If you want to do something like this, then do it in debugfs, but NEVER > > in sysfs. > > So, you would you be open to having the statistics file in debugfs and > removing the individual files sysfs? If these are merely debugging information, they should go to debugfs. If they are something that all users need/want, they should stay in sysfs as-is. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: > On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: > > Instead of having one sysfs file per zram statistic, group them all > > in a single, reader-friendly, 'statistics' file. This not only reduces > > code but is also makes it easier to visualize. The new file looks like: > > > > Number of reads:24 > > Number of writes: 1055 > > Invalid IO: 0 > > Notify free:0 > > Zero pages: 1042 > > Orig data size: 49152 bytes > > Compressed data:838 bytes > > Total memory used: 53248 bytes > > > > Signed-off-by: Davidlohr Bueso > > No, please, the rule for sysfs is "one value per file", not files with > lots of data that you need to parse. Ok. > > If you want to do something like this, then do it in debugfs, but NEVER > in sysfs. So, you would you be open to having the statistics file in debugfs and removing the individual files sysfs? Thanks, Davidlohr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: Instead of having one sysfs file per zram statistic, group them all in a single, reader-friendly, 'statistics' file. This not only reduces code but is also makes it easier to visualize. The new file looks like: Number of reads:24 Number of writes: 1055 Invalid IO: 0 Notify free:0 Zero pages: 1042 Orig data size: 49152 bytes Compressed data:838 bytes Total memory used: 53248 bytes Signed-off-by: Davidlohr Bueso davidlohr.bu...@hp.com No, please, the rule for sysfs is one value per file, not files with lots of data that you need to parse. Ok. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. So, you would you be open to having the statistics file in debugfs and removing the individual files sysfs? Thanks, Davidlohr -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Mon, Feb 11, 2013 at 10:07:45AM -0800, Davidlohr Bueso wrote: On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: Instead of having one sysfs file per zram statistic, group them all in a single, reader-friendly, 'statistics' file. This not only reduces code but is also makes it easier to visualize. The new file looks like: Number of reads:24 Number of writes: 1055 Invalid IO: 0 Notify free:0 Zero pages: 1042 Orig data size: 49152 bytes Compressed data:838 bytes Total memory used: 53248 bytes Signed-off-by: Davidlohr Bueso davidlohr.bu...@hp.com No, please, the rule for sysfs is one value per file, not files with lots of data that you need to parse. Ok. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. So, you would you be open to having the statistics file in debugfs and removing the individual files sysfs? If these are merely debugging information, they should go to debugfs. If they are something that all users need/want, they should stay in sysfs as-is. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On 02/11/2013 10:16 AM, Greg Kroah-Hartman wrote: On Mon, Feb 11, 2013 at 10:07:45AM -0800, Davidlohr Bueso wrote: On Sun, 2013-02-10 at 21:41 -0800, Greg Kroah-Hartman wrote: On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: Instead of having one sysfs file per zram statistic, group them all in a single, reader-friendly, 'statistics' file. This not only reduces code but is also makes it easier to visualize. The new file looks like: Number of reads:24 Number of writes: 1055 Invalid IO: 0 Notify free:0 Zero pages: 1042 Orig data size: 49152 bytes Compressed data:838 bytes Total memory used: 53248 bytes Signed-off-by: Davidlohr Bueso davidlohr.bu...@hp.com No, please, the rule for sysfs is one value per file, not files with lots of data that you need to parse. Ok. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. So, you would you be open to having the statistics file in debugfs and removing the individual files sysfs? If these are merely debugging information, they should go to debugfs. If they are something that all users need/want, they should stay in sysfs as-is. These zram statistics should stay in sysfs. These stats could be used by userspace programs to trigger different actions. For instance, defrag%, calculated using compressed size and actual memory used, could be used to trigger defrag (not yet implemented). Or low hit rates [number of reads/number of writes] is useful in determining is zram is really helpful or not, in case it's being used as swap. If not, zram should be unloaded or reset to save memory space. Other than this, plain compression ratio stats along with zero_pages are also useful and not really debug info. Though stats are spread across many files, it's easy to script them into a single view: http://compcache.googlecode.com/git/sub-projects/scripts/zram_stats I use this script to gather and display stats for each active zram device. Thanks, Nitin -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: > Instead of having one sysfs file per zram statistic, group them all > in a single, reader-friendly, 'statistics' file. This not only reduces > code but is also makes it easier to visualize. The new file looks like: > > Number of reads:24 > Number of writes: 1055 > Invalid IO: 0 > Notify free:0 > Zero pages: 1042 > Orig data size: 49152 bytes > Compressed data:838 bytes > Total memory used: 53248 bytes > > Signed-off-by: Davidlohr Bueso No, please, the rule for sysfs is "one value per file", not files with lots of data that you need to parse. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. sorry, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] zram: gather statistics in a unique file
Sorry, I forgot to include the updated ABI changes with this patch. Sending v2.
On Sun, 2013-02-10 at 20:29 -0800, Davidlohr Bueso wrote:
> Instead of having one sysfs file per zram statistic, group them all
> in a single, reader-friendly, 'statistics' file. This not only reduces
> code but is also makes it easier to visualize. The new file looks like:
>
> Number of reads:24
> Number of writes: 1055
> Invalid IO: 0
> Notify free:0
> Zero pages: 1042
> Orig data size: 49152 bytes
> Compressed data:838 bytes
> Total memory used: 53248 bytes
>
> Signed-off-by: Davidlohr Bueso
> ---
> drivers/staging/zram/zram.txt | 20 ++-
> drivers/staging/zram/zram_sysfs.c | 109
> --
> 2 files changed, 25 insertions(+), 104 deletions(-)
>
> diff --git a/drivers/staging/zram/zram.txt b/drivers/staging/zram/zram.txt
> index 765d790..b3111bc 100644
> --- a/drivers/staging/zram/zram.txt
> +++ b/drivers/staging/zram/zram.txt
> @@ -12,7 +12,7 @@ good amounts of memory savings. Some of the usecases
> include /tmp storage,
> use as swap disks, various caches under /var and maybe many more :)
>
> Statistics for individual zram devices are exported through sysfs nodes at
> -/sys/block/zram/
> +/sys/block/zram/statistics
>
> * Usage
>
> @@ -42,25 +42,11 @@ Following shows a typical sequence of steps for using
> zram.
> mkfs.ext4 /dev/zram1
> mount /dev/zram1 /tmp
>
> -4) Stats:
> - Per-device statistics are exported as various nodes under
> - /sys/block/zram/
> - disksize
> - num_reads
> - num_writes
> - invalid_io
> - notify_free
> - discard
> - zero_pages
> - orig_data_size
> - compr_data_size
> - mem_used_total
> -
> -5) Deactivate:
> +4) Deactivate:
> swapoff /dev/zram0
> umount /dev/zram1
>
> -6) Reset:
> +5) Reset:
> Write any positive value to 'reset' sysfs node
> echo 1 > /sys/block/zram0/reset
> echo 1 > /sys/block/zram1/reset
> diff --git a/drivers/staging/zram/zram_sysfs.c
> b/drivers/staging/zram/zram_sysfs.c
> index e6a929d..2aac370 100644
> --- a/drivers/staging/zram/zram_sysfs.c
> +++ b/drivers/staging/zram/zram_sysfs.c
> @@ -119,106 +119,41 @@ static ssize_t reset_store(struct device *dev,
> return len;
> }
>
> -static ssize_t num_reads_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - zram_stat64_read(zram, >stats.num_reads));
> -}
> -
> -static ssize_t num_writes_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - zram_stat64_read(zram, >stats.num_writes));
> -}
> -
> -static ssize_t invalid_io_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - zram_stat64_read(zram, >stats.invalid_io));
> -}
> -
> -static ssize_t notify_free_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - zram_stat64_read(zram, >stats.notify_free));
> -}
> -
> -static ssize_t zero_pages_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%u\n", zram->stats.pages_zero);
> -}
> -
> -static ssize_t orig_data_size_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - (u64)(zram->stats.pages_stored) << PAGE_SHIFT);
> -}
> -
> -static ssize_t compr_data_size_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static ssize_t statistics_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> {
> struct zram *zram = dev_to_zram(dev);
> -
> - return sprintf(buf, "%llu\n",
> - zram_stat64_read(zram, >stats.compr_size));
> -}
> -
> -static ssize_t mem_used_total_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - u64 val = 0;
> - struct zram *zram = dev_to_zram(dev);
> struct zram_meta *meta = zram->meta;
>
> - if (zram->init_done)
> - val = zs_get_total_size_bytes(meta->mem_pool);
> -
> - return sprintf(buf, "%llu\n", val);
> + return sprintf(buf,
> +"Number of reads:\t%llu\n"
> +"Number of writes:\t%llu\n"
> +
Re: [PATCH 2/2] zram: gather statistics in a unique file
Sorry, I forgot to include the updated ABI changes with this patch. Sending v2.
On Sun, 2013-02-10 at 20:29 -0800, Davidlohr Bueso wrote:
Instead of having one sysfs file per zram statistic, group them all
in a single, reader-friendly, 'statistics' file. This not only reduces
code but is also makes it easier to visualize. The new file looks like:
Number of reads:24
Number of writes: 1055
Invalid IO: 0
Notify free:0
Zero pages: 1042
Orig data size: 49152 bytes
Compressed data:838 bytes
Total memory used: 53248 bytes
Signed-off-by: Davidlohr Bueso davidlohr.bu...@hp.com
---
drivers/staging/zram/zram.txt | 20 ++-
drivers/staging/zram/zram_sysfs.c | 109
--
2 files changed, 25 insertions(+), 104 deletions(-)
diff --git a/drivers/staging/zram/zram.txt b/drivers/staging/zram/zram.txt
index 765d790..b3111bc 100644
--- a/drivers/staging/zram/zram.txt
+++ b/drivers/staging/zram/zram.txt
@@ -12,7 +12,7 @@ good amounts of memory savings. Some of the usecases
include /tmp storage,
use as swap disks, various caches under /var and maybe many more :)
Statistics for individual zram devices are exported through sysfs nodes at
-/sys/block/zramid/
+/sys/block/zramid/statistics
* Usage
@@ -42,25 +42,11 @@ Following shows a typical sequence of steps for using
zram.
mkfs.ext4 /dev/zram1
mount /dev/zram1 /tmp
-4) Stats:
- Per-device statistics are exported as various nodes under
- /sys/block/zramid/
- disksize
- num_reads
- num_writes
- invalid_io
- notify_free
- discard
- zero_pages
- orig_data_size
- compr_data_size
- mem_used_total
-
-5) Deactivate:
+4) Deactivate:
swapoff /dev/zram0
umount /dev/zram1
-6) Reset:
+5) Reset:
Write any positive value to 'reset' sysfs node
echo 1 /sys/block/zram0/reset
echo 1 /sys/block/zram1/reset
diff --git a/drivers/staging/zram/zram_sysfs.c
b/drivers/staging/zram/zram_sysfs.c
index e6a929d..2aac370 100644
--- a/drivers/staging/zram/zram_sysfs.c
+++ b/drivers/staging/zram/zram_sysfs.c
@@ -119,106 +119,41 @@ static ssize_t reset_store(struct device *dev,
return len;
}
-static ssize_t num_reads_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- zram_stat64_read(zram, zram-stats.num_reads));
-}
-
-static ssize_t num_writes_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- zram_stat64_read(zram, zram-stats.num_writes));
-}
-
-static ssize_t invalid_io_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- zram_stat64_read(zram, zram-stats.invalid_io));
-}
-
-static ssize_t notify_free_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- zram_stat64_read(zram, zram-stats.notify_free));
-}
-
-static ssize_t zero_pages_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %u\n, zram-stats.pages_zero);
-}
-
-static ssize_t orig_data_size_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- (u64)(zram-stats.pages_stored) PAGE_SHIFT);
-}
-
-static ssize_t compr_data_size_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t statistics_show(struct device *dev,
+struct device_attribute *attr, char *buf)
{
struct zram *zram = dev_to_zram(dev);
-
- return sprintf(buf, %llu\n,
- zram_stat64_read(zram, zram-stats.compr_size));
-}
-
-static ssize_t mem_used_total_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- u64 val = 0;
- struct zram *zram = dev_to_zram(dev);
struct zram_meta *meta = zram-meta;
- if (zram-init_done)
- val = zs_get_total_size_bytes(meta-mem_pool);
-
- return sprintf(buf, %llu\n, val);
+ return sprintf(buf,
+Number of reads:\t%llu\n
+Number of writes:\t%llu\n
+Invalid IO:\t\t%llu\n
+Notify free:\t\t%llu\n
+Zero pages:\t\t%u\n
+
Re: [PATCH 2/2] zram: gather statistics in a unique file
On Sun, Feb 10, 2013 at 08:29:06PM -0800, Davidlohr Bueso wrote: Instead of having one sysfs file per zram statistic, group them all in a single, reader-friendly, 'statistics' file. This not only reduces code but is also makes it easier to visualize. The new file looks like: Number of reads:24 Number of writes: 1055 Invalid IO: 0 Notify free:0 Zero pages: 1042 Orig data size: 49152 bytes Compressed data:838 bytes Total memory used: 53248 bytes Signed-off-by: Davidlohr Bueso davidlohr.bu...@hp.com No, please, the rule for sysfs is one value per file, not files with lots of data that you need to parse. If you want to do something like this, then do it in debugfs, but NEVER in sysfs. sorry, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
