LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Nitin Gupta <ngupta@vflare.org>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH 5/8] zram: add dynamic device add/remove functionality
Date: Wed, 4 Mar 2015 16:10:30 +0900	[thread overview]
Message-ID: <20150304071030.GD5418@blaptop> (raw)
In-Reply-To: <1425386990-6339-6-git-send-email-sergey.senozhatsky@gmail.com>

On Tue, Mar 03, 2015 at 09:49:47PM +0900, Sergey Senozhatsky wrote:
> Introduce zram-control sysfs class, which has two sysfs attrs:
> - zram_add      -- add a new specific (device_id) zram device
> - zram_remove   -- remove a specific (device_id) zram device
> 
> Usage example:
> 	# add a new specific zram device
> 	echo 4 > /sys/class/zram-control/zram_add
> 
> 	# remove a specific zram device
> 	echo 4 > /sys/class/zram-control/zram_remove
> 
> There is no automatic device_id generation, so user is expected to
> provide one.

Hmm, so every user want to create zram dynamically should lock
the file to synchronize other user who want create same number
zram device? Otherwise, looping until he is successful?

Why should user specifiy zram-device id to create?

> 
> NOTE, there might be users who already depend on the fact that at
> least zram0 device gets always created by zram_init(). Thus, due to
> compatibility reasons, along with requested device_id (f.e. 5) zram0
> will also be created.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-class-zram |  23 ++++++
>  Documentation/blockdev/zram.txt            |  23 +++++-
>  drivers/block/zram/zram_drv.c              | 120 +++++++++++++++++++++++++++++
>  3 files changed, 162 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-zram
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-zram b/Documentation/ABI/testing/sysfs-class-zram
> new file mode 100644
> index 0000000..99b2a1e
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-zram
> @@ -0,0 +1,23 @@
> +What:		/sys/class/zram-control/
> +Date:		March 2015
> +KernelVersion:	4.1
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> +Description:
> +		The zram-control/ class sub-directory belongs to zram
> +		device class
> +
> +What:		/sys/class/zram-control/zram_add
> +Date:		March 2015
> +KernelVersion:	4.1
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> +Description:
> +		Add a specific /dev/zramX device, where X is a device_id
> +		provided by user
> +
> +		What:		/sys/class/zram-control/zram_add
> +Date:		March 2015
> +KernelVersion:	4.1
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> +Description:
> +		Remove a specific /dev/zramX device, where X is a device_id
> +		provided by user
> diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
> index 7fcf9c6..4b140fa 100644
> --- a/Documentation/blockdev/zram.txt
> +++ b/Documentation/blockdev/zram.txt
> @@ -19,7 +19,9 @@ Following shows a typical sequence of steps for using zram.
>  1) Load Module:
>  	modprobe zram num_devices=4
>  	This creates 4 devices: /dev/zram{0,1,2,3}
> -	(num_devices parameter is optional. Default: 1)
> +
> +num_devices parameter is optional and tells zram how many devices should be
> +pre-created. Default: 1.
>  
>  2) Set max number of compression streams
>  	Compression backend may use up to max_comp_streams compression streams,
> @@ -97,7 +99,20 @@ size of the disk when not in use so a huge zram is wasteful.
>  	mkfs.ext4 /dev/zram1
>  	mount /dev/zram1 /tmp
>  
> -7) Stats:
> +7) Add/remove zram devices
> +
> +zram provides a control interface, which enables dynamic (on-demand) device
> +addition and removal.
> +
> +In order to add a new /dev/zramX device (where X is a unique device id)
> +execute
> +	echo X > /sys/class/zram-control/zram_add
> +
> +To remove the existing /dev/zramX device (where X is a device id)
> +execute
> +	echo X > /sys/class/zram-control/zram_remove
> +
> +8) Stats:
>  	Per-device statistics are exported as various nodes under
>  	/sys/block/zram<id>/
>  		disksize
> @@ -113,11 +128,11 @@ size of the disk when not in use so a huge zram is wasteful.
>  		mem_used_total
>  		mem_used_max
>  
> -8) Deactivate:
> +9) Deactivate:
>  	swapoff /dev/zram0
>  	umount /dev/zram1
>  
> -9) Reset:
> +10) 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/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index bab8b20..a457e16 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -33,16 +33,23 @@
>  #include <linux/vmalloc.h>
>  #include <linux/err.h>
>  #include <linux/idr.h>
> +#include <linux/sysfs.h>
>  
>  #include "zram_drv.h"
>  
>  static DEFINE_IDR(zram_index_idr);
> +/* idr index must be protected */
> +static DEFINE_MUTEX(zram_index_mutex);
> +
>  static int zram_major;
>  static const char *default_compressor = "lzo";
>  
>  /* Module params (documentation at end) */
>  static unsigned int num_devices = 1;
>  
> +#define ZRAM_CTL_ADD		1
> +#define ZRAM_CTL_REMOVE		2
> +
>  #define ZRAM_ATTR_RO(name)						\
>  static ssize_t name##_show(struct device *d,				\
>  				struct device_attribute *attr, char *b)	\
> @@ -1173,6 +1180,109 @@ static void zram_remove(struct zram *zram)
>  	kfree(zram);
>  }
>  
> +/* lookup if there is any device pointer that match the given device_id.
> + * return device pointer if so, or ERR_PTR() otherwise. */
> +static struct zram *zram_lookup(int dev_id)
> +{
> +	struct zram *zram;
> +
> +	zram = idr_find(&zram_index_idr, dev_id);
> +	if (zram)
> +		return zram;
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +/* common zram-control add/remove handler */
> +static int zram_control(int cmd, const char *buf)
> +{
> +	struct zram *zram;
> +	int ret = -ENOSYS, err, dev_id;
> +
> +	/* dev_id is gendisk->first_minor, which is `int' */
> +	ret = kstrtoint(buf, 10, &dev_id);
> +	if (ret || dev_id < 0)
> +		return ret;
> +
> +	mutex_lock(&zram_index_mutex);
> +	zram = zram_lookup(dev_id);
> +
> +	switch (cmd) {
> +	case ZRAM_CTL_ADD:
> +		if (!IS_ERR(zram)) {
> +			ret = -EEXIST;
> +			break;
> +		}
> +		ret = zram_add(dev_id);
> +		break;
> +	case ZRAM_CTL_REMOVE:
> +		if (IS_ERR(zram)) {
> +			ret = PTR_ERR(zram);
> +			break;
> +		}
> +
> +		/* First, make ->disksize device attr RO, closing
> +		 * ZRAM_CTL_REMOVE vs disksize_store() race window */
> +		ret = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
> +				&dev_attr_disksize.attr, S_IRUGO);
> +		if (ret)
> +			break;
> +
> +		ret = zram_reset_device(zram);
> +		if (ret == 0) {
> +			/* ->disksize is RO and there are no ->bd_openers */
> +			zram_remove(zram);
> +			break;
> +		}
> +
> +		/* If there are still device bd_openers, try to make ->disksize
> +		 * RW again and return. even if we fail to make ->disksize RW,
> +		 * user still has RW ->reset attr. so it's possible to destroy
> +		 * that device */
> +		err = sysfs_chmod_file(&disk_to_dev(zram->disk)->kobj,
> +				&dev_attr_disksize.attr,
> +				S_IWUSR | S_IRUGO);
> +		if (err)
> +			ret = err;
> +		break;
> +	}
> +	mutex_unlock(&zram_index_mutex);
> +
> +	return ret;
> +}
> +
> +/* zram module control sysfs attributes */
> +static ssize_t zram_add_store(struct class *class,
> +			struct class_attribute *attr,
> +			const char *buf,
> +			size_t count)
> +{
> +	int ret = zram_control(ZRAM_CTL_ADD, buf);
> +
> +	return ret ? ret : count;
> +}
> +
> +static ssize_t zram_remove_store(struct class *class,
> +			struct class_attribute *attr,
> +			const char *buf,
> +			size_t count)
> +{
> +	int ret = zram_control(ZRAM_CTL_REMOVE, buf);
> +
> +	return ret ? ret : count;
> +}
> +
> +static struct class_attribute zram_control_class_attrs[] = {
> +	__ATTR_WO(zram_add),
> +	__ATTR_WO(zram_remove),
> +	__ATTR_NULL,
> +};
> +
> +static struct class zram_control_class = {
> +	.name		= "zram-control",
> +	.owner		= THIS_MODULE,
> +	.class_attrs	= zram_control_class_attrs,
> +};
> +
>  static int zram_exit_cb(int id, void *ptr, void *data)
>  {
>  	zram_remove(ptr);
> @@ -1181,6 +1291,7 @@ static int zram_exit_cb(int id, void *ptr, void *data)
>  
>  static void destroy_devices(void)
>  {
> +	class_unregister(&zram_control_class);
>  	idr_for_each(&zram_index_idr, &zram_exit_cb, NULL);
>  	idr_destroy(&zram_index_idr);
>  	unregister_blkdev(zram_major, "zram");
> @@ -1197,14 +1308,23 @@ static int __init zram_init(void)
>  		return -EINVAL;
>  	}
>  
> +	ret = class_register(&zram_control_class);
> +	if (ret) {
> +		pr_warn("Unable to register zram-control class\n");
> +		return ret;
> +	}
> +
>  	zram_major = register_blkdev(0, "zram");
>  	if (zram_major <= 0) {
>  		pr_warn("Unable to get major number\n");
> +		class_unregister(&zram_control_class);
>  		return -EBUSY;
>  	}
>  
>  	for (dev_id = 0; dev_id < num_devices; dev_id++) {
> +		mutex_lock(&zram_index_mutex);
>  		ret = zram_add(dev_id);
> +		mutex_unlock(&zram_index_mutex);
>  		if (ret != 0)
>  			goto out_error;
>  	}
> -- 
> 2.3.1.167.g7f4ba4b
> 

-- 
Kind regards,
Minchan Kim

  parent reply	other threads:[~2015-03-04  7:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-03 12:49 [PATCHv3 0/8] introduce dynamic device creation/removal Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 1/8] zram: cosmetic ZRAM_ATTR_RO code formatting tweak Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 2/8] zram: use idr instead of `zram_devices' array Sergey Senozhatsky
2015-03-03 22:01   ` Andrew Morton
2015-03-04  0:21     ` Sergey Senozhatsky
2015-03-04  7:06   ` Minchan Kim
2015-03-04  7:34     ` Sergey Senozhatsky
2015-03-04  7:49     ` Sergey Senozhatsky
2015-03-05  0:59       ` Minchan Kim
2015-03-03 12:49 ` [PATCH 3/8] zram: factor out device reset from reset_store() Sergey Senozhatsky
2015-03-05  2:28   ` Minchan Kim
2015-03-03 12:49 ` [PATCH 4/8] zram: reorganize code layout Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 5/8] zram: add dynamic device add/remove functionality Sergey Senozhatsky
2015-03-03 22:01   ` Andrew Morton
2015-03-04  0:18     ` Sergey Senozhatsky
2015-03-04  7:10   ` Minchan Kim [this message]
2015-03-04  7:29     ` Sergey Senozhatsky
2015-03-04  8:19       ` Sergey Senozhatsky
2015-03-04  8:36         ` Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 6/8] zram: remove max_num_devices limitation Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 7/8] zram: report every added and removed device Sergey Senozhatsky
2015-03-03 12:49 ` [PATCH 8/8] zram: trivial: correct flag operations comment Sergey Senozhatsky
2015-04-15 21:37 ` [PATCHv3 0/8] introduce dynamic device creation/removal Andrew Morton
2015-04-15 23:40   ` Minchan Kim
2015-04-16  0:30     ` Sergey Senozhatsky
2015-04-16  0:47   ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150304071030.GD5418@blaptop \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --subject='Re: [PATCH 5/8] zram: add dynamic device add/remove functionality' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).