LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Minchan Kim <minchan@kernel.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: Tue, 3 Mar 2015 14:01:46 -0800	[thread overview]
Message-ID: <20150303140146.1b2345cf490a477303ff3270@linux-foundation.org> (raw)
In-Reply-To: <1425386990-6339-6-git-send-email-sergey.senozhatsky@gmail.com>

On Tue,  3 Mar 2015 21:49:47 +0900 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> 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.
> 
> 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.

This doesn't really explain why you think we need the patch.

- What's the current scheme for adding devices?

- What's wrong with it?

- Why is this better?

> 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;
> +}

The types here are confused.  `ret' is int, `count' is size_t, the
function returns ssize_t and heaven knows what type "ret ?  ret :
count" evaluates to!



  reply	other threads:[~2015-03-03 22:01 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 [this message]
2015-03-04  0:18     ` Sergey Senozhatsky
2015-03-04  7:10   ` Minchan Kim
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=20150303140146.1b2345cf490a477303ff3270@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minchan@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).