lkml.org 
[lkml]   [2015]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] zram: do not let user enforce new device dev_id
Date
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 <minchan@kernel.org>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@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.senozhatsky@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


\
 
 \ /
  Last update: 2015-03-06 13:41    [W:0.031 / U:2.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site