| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150304001806.GA1864@swordfish>
Date: Wed, 4 Mar 2015 09:18:06 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Minchan Kim <minchan@...nel.org>,
Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [PATCH 5/8] zram: add dynamic device add/remove functionality
On (03/03/15 14:01), Andrew Morton 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?
oh, yes. you are right.
We currently don't support on-demand device creation. The only way to have
N zram devices is to specify num_devices module parameter (default value 1).
That means that if, for some reason, at some point, user wants to have N + 1
devies he/she must umount all the existing devices, unload the module, load
the module passing num_devices equals to N + 1. And do this again, if needed.
-ss
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.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@...il.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@...il.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@...il.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!
>
>
--
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