Date:	Fri, 13 Mar 2015 08:55:47 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [PATCHv2 7/7] zram: deprecate zram attrs sysfs nodes

Hi Sergey,

On Thu, Mar 12, 2015 at 11:47:18PM +0900, Sergey Senozhatsky wrote:
> Add Documentation/ABI/obsolete/sysfs-block-zram file and list
> obsolete and deprecated attributes there. The patch also adds
> additional information to zram documentation and describes the
> basic strategy:
> - the existing RW nodes will be downgraded to WO nodes (in 4.11)
> - deprecated RO sysfs nodes will eventually be removed (in 4.11)
> 
> Users will be additionally notified about deprecated attr usage
> by pr_warn_once() (added to every deprecated attr _show()), as
> suggested by Minchan Kim.
> 
> Example:
>  zram: Attribute mem_used_max will be removed. See zram documentation.
> 
> User space is advised to use zram<id>/stat, zram<id>/io_stat and
> zram<id>/mm_stat files.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> ---
>  Documentation/ABI/obsolete/sysfs-block-zram | 119 ++++++++++++++++++++++++++++
>  Documentation/blockdev/zram.txt             |  16 ++++
>  drivers/block/zram/zram_drv.c               |  12 +++
>  3 files changed, 147 insertions(+)
>  create mode 100644 Documentation/ABI/obsolete/sysfs-block-zram
> 
> diff --git a/Documentation/ABI/obsolete/sysfs-block-zram b/Documentation/ABI/obsolete/sysfs-block-zram
> new file mode 100644
> index 0000000..720ea92
> --- /dev/null
> +++ b/Documentation/ABI/obsolete/sysfs-block-zram
> @@ -0,0 +1,119 @@
> +What:		/sys/block/zram<id>/num_reads
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The num_reads file is read-only and specifies the number of
> +		reads (failed or successful) done on this device.
> +		Now accessible via zram<id>/stat node.
> +
> +What:		/sys/block/zram<id>/num_writes
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The num_writes file is read-only and specifies the number of
> +		writes (failed or successful) done on this device.
> +		Now accessible via zram<id>/stat node.
> +
> +What:		/sys/block/zram<id>/invalid_io
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The invalid_io file is read-only and specifies the number of
> +		non-page-size-aligned I/O requests issued to this device.
> +		Now accessible via zram<id>/io_stat node.
> +
> +What:		/sys/block/zram<id>/failed_reads
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The failed_reads file is read-only and specifies the number of
> +		failed reads happened on this device.
> +		Now accessible via zram<id>/io_stat node.
> +
> +What:		/sys/block/zram<id>/failed_writes
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The failed_writes file is read-only and specifies the number of
> +		failed writes happened on this device.
> +		Now accessible via zram<id>/io_stat node.
> +
> +What:		/sys/block/zram<id>/notify_free
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The notify_free file is read-only. Depending on device usage
> +		scenario it may account a) the number of pages freed because
> +		of swap slot free notifications or b) the number of pages freed
> +		because of REQ_DISCARD requests sent by bio. The former ones
> +		are sent to a swap block device when a swap slot is freed, which
> +		implies that this disk is being used as a swap disk. The latter
> +		ones are sent by filesystem mounted with discard option,
> +		whenever some data blocks are getting discarded.
> +		Now accessible via zram<id>/io_stat node.
> +
> +What:		/sys/block/zram<id>/zero_pages
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The zero_pages file is read-only and specifies number of zero
> +		filled pages written to this disk. No memory is allocated for
> +		such pages.
> +		Now accessible via zram<id>/mm_stat node.
> +
> +What:		/sys/block/zram<id>/orig_data_size
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The orig_data_size file is read-only and specifies uncompressed
> +		size of data stored in this disk. This excludes zero-filled
> +		pages (zero_pages) since no memory is allocated for them.
> +		Unit: bytes
> +		Now accessible via zram<id>/mm_stat node.
> +
> +What:		/sys/block/zram<id>/compr_data_size
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The compr_data_size file is read-only and specifies compressed
> +		size of data stored in this disk. So, compression ratio can be
> +		calculated using orig_data_size and this statistic.
> +		Unit: bytes
> +		Now accessible via zram<id>/mm_stat node.
> +
> +What:		/sys/block/zram<id>/mem_used_total
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The mem_used_total file is read-only and specifies the amount
> +		of memory, including allocator fragmentation and metadata
> +		overhead, allocated for this disk. So, allocator space
> +		efficiency can be calculated using compr_data_size and this
> +		statistic.
> +		Unit: bytes
> +		Now accessible via zram<id>/mm_stat node.
> +
> +What:		/sys/block/zram<id>/mem_used_max
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +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 write "0". Otherwise,
> +		you could see -EINVAL.
> +		Unit: bytes
> +		Downgraded to write-only node: so it's possible to set new
> +		value only; its current value is stored in zram<id>/mm_stat
> +		node.
> +
> +What:		/sys/block/zram<id>/mem_limit
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> +Description:
> +		The mem_limit file is read/write and specifies the maximum
> +		amount of memory ZRAM can use to store the compressed data.
> +		The limit could be changed in run time and "0" means disable
> +		the limit.  No limit is the initial state.  Unit: bytes
> +		Downgraded to write-only node: so it's possible to set new
> +		value only; its current value is stored in zram<id>/mm_stat
> +		node.
> diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
> index b133138..44b1a77 100644
> --- a/Documentation/blockdev/zram.txt
> +++ b/Documentation/blockdev/zram.txt
> @@ -148,6 +148,22 @@ num_migrated      RO    the number of objects migrated migrated by compaction
>  compact           WO    trigger memory compaction
>  
>  
> +WARNING
> +=======
> +per-stat sysfs attributes are considered to be deprecated.
> +The basic strategy is:
> +-- the existing RW nodes will be downgraded to WO nodes (in linux 4.11)
> +-- deprecated RO sysfs nodes will eventually be removed (in linux 4.11)
> +
> +The list of deprecated attributes can be found here:
> +Documentation/ABI/obsolete/sysfs-block-zram
> +
> +Basically, every attribute that has its own read accessible sysfs node
> +(e.g. num_reads) *AND* is accessible via one of the stat files (zram<id>/stat
> +or zram<id>/io_stat or zram<id>/mm_stat) is considered to be deprecated.
> +
> +User space is advised to use the following files to read the device statistics.
> +
>  File /sys/block/zram<id>/stat
>  
>  Represents block layer statistics. Read Documentation/block/stat.txt for
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 7493096..2aced91 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -47,11 +47,19 @@ static const char *default_compressor = "lzo";
>  /* Module params (documentation at end) */
>  static unsigned int num_devices = 1;
>  
> +static inline void deprecated_attr_warn(const char *name)
> +{
> +	pr_warn_once("Attribute %s will be removed. See zram documentation.\n",
> +			name);

How about adding pid, comm? Admin can know which process touches.

> +}
> +
>  #define ZRAM_ATTR_RO(name)						\
>  static ssize_t name##_show(struct device *d,				\
>  				struct device_attribute *attr, char *b)	\
>  {									\
>  	struct zram *zram = dev_to_zram(d);				\
> +									\
> +	deprecated_attr_warn(__stringify(name));			\

It just reports once for first-touched stat and miss other stats by ZRAM_ATTR_RO.
Of course, you warned "See zram documentation" so he might know about other stats
plan, too. If it's your intention, let's make warn more clear.

"Atrribute $s will be removed. Also, there are other stats we will remove
in future. See Documentation/blockdev/zram.txt"


>  	return scnprintf(b, PAGE_SIZE, "%llu\n",			\
>  		(u64)atomic64_read(&zram->stats.name));			\
>  }									\
> @@ -206,6 +214,7 @@ static ssize_t orig_data_size_show(struct device *dev,
>  {
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("orig_data_size");
>  	return scnprintf(buf, PAGE_SIZE, "%llu\n",
>  		(u64)(atomic64_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
>  }
> @@ -216,6 +225,7 @@ static ssize_t mem_used_total_show(struct device *dev,
>  	u64 val = 0;
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("mem_used_total");
>  	down_read(&zram->init_lock);
>  	if (init_done(zram)) {
>  		struct zram_meta *meta = zram->meta;
> @@ -245,6 +255,7 @@ static ssize_t mem_limit_show(struct device *dev,
>  	u64 val;
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("mem_limit");
>  	down_read(&zram->init_lock);
>  	val = zram->limit_pages;
>  	up_read(&zram->init_lock);
> @@ -276,6 +287,7 @@ static ssize_t mem_used_max_show(struct device *dev,
>  	u64 val = 0;
>  	struct zram *zram = dev_to_zram(dev);
>  
> +	deprecated_attr_warn("mem_used_max");
>  	down_read(&zram->init_lock);
>  	if (init_done(zram))
>  		val = atomic_long_read(&zram->stats.max_used_pages);
> -- 
> 2.3.2.209.gd67f9d5
> 

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists