This patch forbids user to enforce device ids for newly added zram devices, as suggested by Minchan Kim. There seems to be a little interest in this functionality and its use-cases are rather non-obvious.
zram_add sysfs attr, thus, is now read only and has only automatic device id assignment mode. This operation is no longer allowed: echo 1 > /sys/class/zram-control/zram_add -bash: /sys/class/zram-control/zram_add: Permission denied Correct usage example: cat /sys/class/zram-control/zram_add 2 It also removes common zram_control() handler because device creation and removal do not share a lot of common steps anymore. Move zram_add and zram_remove code to zram_add_show() and zram_remove_store() correspondingly. LKML-reference: https://lkml.org/lkml/2015/3/4/1414 Reported-by: Minchan Kim <minc...@kernel.org> Signed-off-by: Sergey Senozhatsky <sergey.senozhat...@gmail.com> --- Documentation/ABI/testing/sysfs-class-zram | 8 +- Documentation/blockdev/zram.txt | 20 ++--- drivers/block/zram/zram_drv.c | 130 +++++++++++------------------ 3 files changed, 59 insertions(+), 99 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-class-zram b/Documentation/ABI/testing/sysfs-class-zram index 0b678b2..c39d287 100644 --- a/Documentation/ABI/testing/sysfs-class-zram +++ b/Documentation/ABI/testing/sysfs-class-zram @@ -11,11 +11,9 @@ Date: March 2015 KernelVersion: 4.1 Contact: Sergey Senozhatsky <sergey.senozhat...@gmail.com> Description: - RW attribuite. Write operation adds a specific /dev/zramX - device, where X is a device_id provided by user. - Read operation will automatically pick up avilable device_id - X, add /dev/zramX device and return that device_id X back to - user. + RO attribute. Read operation will cause zram to add a new + device and return its device id back to user (so one can + use /dev/zram<id>), or error code. What: /sys/class/zram-control/zram_add Date: March 2015 diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt index 6bbdded..902c97c 100644 --- a/Documentation/blockdev/zram.txt +++ b/Documentation/blockdev/zram.txt @@ -104,23 +104,17 @@ size of the disk when not in use so a huge zram is wasteful. 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 - -Additionally, zram also handles automatic device_id generation and assignment. +In order to add a new /dev/zramX device, perform read operation on zram_add +attribute. This will return either new device's device id (meaning that you +can use /dev/zram<id>) or error code. +Example: cat /sys/class/zram-control/zram_add 1 - cat /sys/class/zram-control/zram_add - 2 -this will pick up available device_id X, add corresponding /dev/zramX -device and return its device_id X back to user (or error code). +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 diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 6d27518..0194799 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -47,9 +47,6 @@ 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) \ @@ -1230,79 +1227,7 @@ static struct zram *zram_lookup(int dev_id) 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_add_show(struct class *class, struct class_attribute *attr, char *buf) @@ -1310,10 +1235,7 @@ static ssize_t zram_add_show(struct class *class, int ret; mutex_lock(&zram_index_mutex); - /* - * read operation on zram_add is - pick up device_id automatically, add - * corresponding device and return that device_id back to user - */ + /* pick up available device_id */ ret = zram_add(-1); mutex_unlock(&zram_index_mutex); @@ -1327,13 +1249,59 @@ static ssize_t zram_remove_store(struct class *class, const char *buf, size_t count) { - int ret = zram_control(ZRAM_CTL_REMOVE, buf); + struct zram *zram; + int ret, err, dev_id; + + mutex_lock(&zram_index_mutex); + + /* dev_id is gendisk->first_minor, which is `int' */ + ret = kstrtoint(buf, 10, &dev_id); + if (ret || dev_id < 0) { + ret = -EINVAL; + goto out; + } + + zram = zram_lookup(dev_id); + if (IS_ERR(zram)) { + ret = PTR_ERR(zram); + goto out; + } + + /* + * 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) + goto out; + + ret = zram_reset_device(zram); + if (ret == 0) { + /* ->disksize is RO and there are no ->bd_openers */ + zram_remove(zram); + goto out; + } + + /* + * 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; +out: + mutex_unlock(&zram_index_mutex); return ret ? ret : count; } static struct class_attribute zram_control_class_attrs[] = { - __ATTR_RW(zram_add), + __ATTR_RO(zram_add), __ATTR_WO(zram_remove), __ATTR_NULL, }; -- 2.3.1.184.g97c12a8 -- 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/