[PATCH 0/2] make automatic device_id generation possible

[PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

Hello,

Make zram-contol/zram_add interface easier to use. Extend it to support
read and write operations.

Write operation remains the same:

	echo X > /sys/class/zram-control/zram_add

will add /dev/zramX (or return error).


Read operation is treated as 'pick up available device_id, add new
device and return device_id'.

Example:
	 cat /sys/class/zram-control/zram_add
	2
	 cat /sys/class/zram-control/zram_add
	3

Sergey Senozhatsky (2):
  zram: return zram device_id value from zram_add()
  zram: introduce automatic device_id generation

 Documentation/ABI/testing/sysfs-class-zram |  7 ++++--
 Documentation/blockdev/zram.txt            | 10 ++++++++
 drivers/block/zram/zram_drv.c              | 37 ++++++++++++++++++++++++++----
 3 files changed, 47 insertions(+), 7 deletions(-)

-- 
2.3.1.167.g7f4ba4b

[PATCH 1/2] zram: return zram device_id value from zram_add()

[Sergey Senozhatsky]

zram_add requires valid device_id to be provided, that can be a
bit inconvenient. Change zram_add() to return negative value upon
new device creation failure, and device_id (>= 0) value otherwise.

This prepares zram_add to perform automatic device_id assignment. New
device_id will be returned back to user-space (so user can reference
that device as /dev/zramX).

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 087b043..0978307 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1066,6 +1066,8 @@ static struct attribute_group zram_disk_attr_group = {
 	.attrs = zram_disk_attrs,
 };
 
+/* allocate and initialize new zram device. the function returns
+ * '>= 0' device_id upon success, and negative value otherwise. */
 static int zram_add(int device_id)
 {
 	struct zram *zram;
@@ -1151,7 +1153,7 @@ static int zram_add(int device_id)
 	zram->max_comp_streams = 1;
 
 	pr_info("Added device: %s\n", zram->disk->disk_name);
-	return 0;
+	return device_id;
 
 out_free_disk:
 	del_gendisk(zram->disk);
@@ -1322,7 +1324,7 @@ static int __init zram_init(void)
 		mutex_lock(&zram_index_mutex);
 		ret = zram_add(dev_id);
 		mutex_unlock(&zram_index_mutex);
-		if (ret != 0)
+		if (ret < 0)
 			goto out_error;
 	}
 
-- 
2.3.1.167.g7f4ba4b

[PATCH 2/2] zram: introduce automatic device_id generation

[Sergey Senozhatsky]

The existing device creation interface requires user to provide
new and unique device_id for every device being created. This
might be difficult to handle (f.e. in automated scripts).

Extend zram-control/zram_add interface to support read and write
operations. Write operation remains the same:

	echo X > /sys/class/zram-control/zram_add

will add /dev/zramX (or return error).

Read operation is treated as 'pick up available device_id, add new
device and return device_id'.

Example:
	 cat /sys/class/zram-control/zram_add
	2
	 cat /sys/class/zram-control/zram_add
	3

so user-space can proceed with /dev/zram2, /dev/zram3 init and
usage.

An attempt to use already used device_id (-EEXIST)

	echo 3 > /sys/class/zram-control/zram_add
	-bash: echo: write error: File exists

Returning zram_add error code back to user (-ENOMEM in this case)

	cat /sys/class/zram-control/zram_add
	cat: /sys/class/zram-control/zram_add: Cannot allocate memory

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-class-zram |  7 +++++--
 Documentation/blockdev/zram.txt            | 10 ++++++++++
 drivers/block/zram/zram_drv.c              | 31 +++++++++++++++++++++++++++---
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-zram b/Documentation/ABI/testing/sysfs-class-zram
index 99b2a1e..0b678b2 100644
--- a/Documentation/ABI/testing/sysfs-class-zram
+++ b/Documentation/ABI/testing/sysfs-class-zram
@@ -11,8 +11,11 @@ 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
+		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.
 
 		What:		/sys/class/zram-control/zram_add
 Date:		March 2015
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 4b140fa..6bbdded 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -112,6 +112,16 @@ 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.
+
+	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).
+
 8) Stats:
 	Per-device statistics are exported as various nodes under
 	/sys/block/zram<id>/
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0978307..e0b40bb 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1078,8 +1078,15 @@ static int zram_add(int device_id)
 	if (!zram)
 		return ret;
 
-	ret = idr_alloc(&zram_index_idr, zram, device_id,
-			device_id + 1, GFP_KERNEL);
+	if (device_id < 0) {
+		/* generate new device_id */
+		ret = idr_alloc(&zram_index_idr, zram, 0, 0, GFP_KERNEL);
+		device_id = ret;
+	} else {
+		/* use provided device_id */
+		ret = idr_alloc(&zram_index_idr, zram, device_id,
+				device_id + 1, GFP_KERNEL);
+	}
 	if (ret < 0)
 		goto out_free_dev;
 
@@ -1267,6 +1274,24 @@ static ssize_t zram_add_store(struct class *class,
 	return ret ? ret : count;
 }
 
+static ssize_t zram_add_show(struct class *class,
+			struct class_attribute *attr,
+			char *buf)
+{
+	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 */
+	ret = zram_add(-1);
+	mutex_unlock(&zram_index_mutex);
+
+	if (ret < 0)
+		return ret;
+	return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
+}
+
 static ssize_t zram_remove_store(struct class *class,
 			struct class_attribute *attr,
 			const char *buf,
@@ -1278,7 +1303,7 @@ static ssize_t zram_remove_store(struct class *class,
 }
 
 static struct class_attribute zram_control_class_attrs[] = {
-	__ATTR_WO(zram_add),
+	__ATTR_RW(zram_add),
 	__ATTR_WO(zram_remove),
 	__ATTR_NULL,
 };
-- 
2.3.1.167.g7f4ba4b

Re: [PATCH 2/2] zram: introduce automatic device_id generation

[Andrew Morton]

On Wed,  4 Mar 2015 23:16:41 +0900 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:

> +static ssize_t zram_add_show(struct class *class,
> +			struct class_attribute *attr,
> +			char *buf)
> +{
> +	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 */
> +	ret = zram_add(-1);
> +	mutex_unlock(&zram_index_mutex);
> +
> +	if (ret < 0)
> +		return ret;
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
> +}

Please don't invent new commenting styles.  Because doing so inevitably
creates a mixed-up mess, which is what we now have.

--- a/drivers/block/zram/zram_drv.c~zram-introduce-automatic-device_id-generation-fix
+++ a/drivers/block/zram/zram_drv.c
@@ -1281,9 +1281,10 @@ static ssize_t zram_add_show(struct clas
 	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 */
+	/*
+	 * read operation on zram_add is - pick up device_id automatically, add
+	 * corresponding device and return that device_id back to user
+	 */
 	ret = zram_add(-1);
 	mutex_unlock(&zram_index_mutex);
 
_

Re: [PATCH 2/2] zram: introduce automatic device_id generation

[Sergey Senozhatsky]

On (03/04/15 14:13), Andrew Morton wrote:
> > +static ssize_t zram_add_show(struct class *class,
> > +			struct class_attribute *attr,
> > +			char *buf)
> > +{
> > +	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 */
> > +	ret = zram_add(-1);
> > +	mutex_unlock(&zram_index_mutex);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > +	return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
> > +}
> 
> Please don't invent new commenting styles.  Because doing so inevitably
> creates a mixed-up mess, which is what we now have.
> 

sorry. sure, will take care of that next time. thanks for pointing that
out.

	-ss

> --- a/drivers/block/zram/zram_drv.c~zram-introduce-automatic-device_id-generation-fix
> +++ a/drivers/block/zram/zram_drv.c
> @@ -1281,9 +1281,10 @@ static ssize_t zram_add_show(struct clas
>  	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 */
> +	/*
> +	 * read operation on zram_add is - pick up device_id automatically, add
> +	 * corresponding device and return that device_id back to user
> +	 */
>  	ret = zram_add(-1);
>  	mutex_unlock(&zram_index_mutex);
>  
> _
> 
> 

Re: [PATCH 0/2] make automatic device_id generation possible

[Minchan Kim]

Hello Sergey,

On Wed, Mar 04, 2015 at 11:16:39PM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> Make zram-contol/zram_add interface easier to use. Extend it to support
> read and write operations.
> 
> Write operation remains the same:
> 
> 	echo X > /sys/class/zram-control/zram_add
> 
> will add /dev/zramX (or return error).
> 
> 
> Read operation is treated as 'pick up available device_id, add new
> device and return device_id'.
> 
> Example:
> 	 cat /sys/class/zram-control/zram_add
> 	2
> 	 cat /sys/class/zram-control/zram_add
> 	3

Thanks for handling my concern quickly and sorry for not sending
active feedback in realtime. Maybe I should turn on CONFIG_PREEMPT
in my brain.

I'm not against but I want to know why we should support
user-defined device id. What usecase do you have in mind?

Could we support automatic id support only at this moment?
Then, if some user complains about that in future, we could turn
on user-defined device id easily and we could know the usecase.

In summary, I want to support only "cat /sys/class/zram-control/zram_add"
unless you have feasible usecase.

What do you think about it?

> 
> Sergey Senozhatsky (2):
>   zram: return zram device_id value from zram_add()
>   zram: introduce automatic device_id generation
> 
>  Documentation/ABI/testing/sysfs-class-zram |  7 ++++--
>  Documentation/blockdev/zram.txt            | 10 ++++++++
>  drivers/block/zram/zram_drv.c              | 37 ++++++++++++++++++++++++++----
>  3 files changed, 47 insertions(+), 7 deletions(-)
> 
> -- 
> 2.3.1.167.g7f4ba4b
> 

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

Hello,

On (03/05/15 09:20), Minchan Kim wrote:
> I'm not against but I want to know why we should support
> user-defined device id. What usecase do you have in mind?
> 

hm, you never know what people can come up with. that's probably the
strongest support argument I can provide. I wish there was something
like - my friend Mike has a "device /dev/zram1 is always swap device,
device /dev/zram$(id -u) is a per-user zram device (he finds it useful,
because just looking at device id he can easily tell who owns that
device)" policy. but nothing like that. I just think that it can be
useful. no real use cases (well, partly because we don't support device
add/remove).

/* yet "/dev/zram$(id -u)" thing looks interesting */


user defined id support comes at a price of ~10 lines of code, or even
less. we waste much more code to show ->stats, and not all of them are
of any real use, to be fair. that just said, that dropping user defined
id is not a great deal. ok, let's see if we can come up with anything by
the end of this day and I'll send out a removal patch if nothing pop up.

	-ss

> Could we support automatic id support only at this moment?
> Then, if some user complains about that in future, we could turn
> on user-defined device id easily and we could know the usecase.

Re: [PATCH 0/2] make automatic device_id generation possible

[Minchan Kim]

On Thu, Mar 05, 2015 at 09:58:29AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (03/05/15 09:20), Minchan Kim wrote:
> > I'm not against but I want to know why we should support
> > user-defined device id. What usecase do you have in mind?
> > 
> 
> hm, you never know what people can come up with. that's probably the
> strongest support argument I can provide. I wish there was something
> like - my friend Mike has a "device /dev/zram1 is always swap device,
> device /dev/zram$(id -u) is a per-user zram device (he finds it useful,
> because just looking at device id he can easily tell who owns that
> device)" policy. but nothing like that. I just think that it can be
> useful. no real use cases (well, partly because we don't support device
> add/remove).
> 
> /* yet "/dev/zram$(id -u)" thing looks interesting */

Fair enough.

> 
> 
> user defined id support comes at a price of ~10 lines of code, or even
> less. we waste much more code to show ->stats, and not all of them are
> of any real use, to be fair. that just said, that dropping user defined
> id is not a great deal. ok, let's see if we can come up with anything by
> the end of this day and I'll send out a removal patch if nothing pop up.

As I told you, I'm never against. I just want to know usecase.
If we don't support it from the beginnig, someday, someone will complain
and we can catch up the usecase and support it easily with adding 10 line code.

This dyanmic add/revmove feature proves the idea. :)
Main reason I finally decided dynamic device management feature was
someone complained he should do rmmod/insmod zram.ko to increase
the number of zram device in runtime but one of zram device was
used for swap, which was hard to swapoff due to small memory
so there was no way to increase the number of zram device.
It appeals a lot to support dynamic zram creating and finally I catch up
the usecase. ;-)

> 
> 	-ss
> 
> > Could we support automatic id support only at this moment?
> > Then, if some user complains about that in future, we could turn
> > on user-defined device id easily and we could know the usecase.
> 

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 09:58), Sergey Senozhatsky wrote:
> /* yet "/dev/zram$(id -u)" thing looks interesting */
> 

hm, I can think of a huge build server with tons of users. /dev/zram$(id -u)
created during user login and destroyed during logout. so users use theirs own
zram devices with predictable device ids (which also makes it simpler for admin)
for compilation/etc., and don't pressure hdds that much.

	-ss

Re: [PATCH 0/2] make automatic device_id generation possible

[Minchan Kim]

On Thu, Mar 05, 2015 at 10:20:43AM +0900, Sergey Senozhatsky wrote:
> On (03/05/15 09:58), Sergey Senozhatsky wrote:
> > /* yet "/dev/zram$(id -u)" thing looks interesting */
> > 
> 
> hm, I can think of a huge build server with tons of users. /dev/zram$(id -u)
> created during user login and destroyed during logout. so users use theirs own
> zram devices with predictable device ids (which also makes it simpler for admin)
> for compilation/etc., and don't pressure hdds that much.

They upgraded the system and from now on, one of app tries automatic
id with zram for some reason. What happens if he gets some user id
before the user login? The system should have fallback in the case of
failing to create own userid assignment.

Hmm, Coexisting specific and automatic id assign seem to be not a
godd idea.


> 
> 	-ss

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 10:17), Minchan Kim wrote:
> > 
> > 
> > user defined id support comes at a price of ~10 lines of code, or even
> > less. we waste much more code to show ->stats, and not all of them are
> > of any real use, to be fair. that just said, that dropping user defined
> > id is not a great deal. ok, let's see if we can come up with anything by
> > the end of this day and I'll send out a removal patch if nothing pop up.
> 
> As I told you, I'm never against. I just want to know usecase.
> If we don't support it from the beginnig, someday, someone will complain
> and we can catch up the usecase and support it easily with adding 10 line code.
> 

sure, no problem. that's a good question -- should we support user
defined ids or not. thanks for asking.

I can imagine that that static num_devices limitation (along with max
num_devices == 32) was sort of a show stopper for some users (or an
unnecessary complication at least), like in 'my now favorite' build
server example :)

	-ss

> This dyanmic add/revmove feature proves the idea. :)
> Main reason I finally decided dynamic device management feature was
> someone complained he should do rmmod/insmod zram.ko to increase
> the number of zram device in runtime but one of zram device was
> used for swap, which was hard to swapoff due to small memory
> so there was no way to increase the number of zram device.
> It appeals a lot to support dynamic zram creating and finally I catch up
> the usecase. ;-)
> 

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 10:33), Minchan Kim wrote:
> > hm, I can think of a huge build server with tons of users. /dev/zram$(id -u)
> > created during user login and destroyed during logout. so users use theirs own
> > zram devices with predictable device ids (which also makes it simpler for admin)
> > for compilation/etc., and don't pressure hdds that much.
> 
> They upgraded the system and from now on, one of app tries automatic
> id with zram for some reason. What happens if he gets some user id
> before the user login? The system should have fallback in the case of
> failing to create own userid assignment.

we upgraded our scripts but landed some bugs there? it's up to particular
implementation. in your example, I assume, someone used zram with num_devices >= 1000?
that's impossible. current num_devices limitation is 32. and uid-s start from 1000.

these scripts should check if device has been created anyway, it just adds -EEXIST
check. in general "what if user space does something wrong" thing can be beaten by
"what if user space does everything right" argument. when script fails we just go
and fix that script, I guess.

	-ss

> 
> Hmm, Coexisting specific and automatic id assign seem to be not a
> godd idea.

Re: [PATCH 0/2] make automatic device_id generation possible

[Minchan Kim]

On Thu, Mar 05, 2015 at 10:47:52AM +0900, Sergey Senozhatsky wrote:
> On (03/05/15 10:33), Minchan Kim wrote:
> > > hm, I can think of a huge build server with tons of users. /dev/zram$(id -u)
> > > created during user login and destroyed during logout. so users use theirs own
> > > zram devices with predictable device ids (which also makes it simpler for admin)
> > > for compilation/etc., and don't pressure hdds that much.
> > 
> > They upgraded the system and from now on, one of app tries automatic
> > id with zram for some reason. What happens if he gets some user id
> > before the user login? The system should have fallback in the case of
> > failing to create own userid assignment.
> 
> we upgraded our scripts but landed some bugs there? it's up to particular
> implementation. in your example, I assume, someone used zram with num_devices >= 1000?
> that's impossible. current num_devices limitation is 32. and uid-s start from 1000.

I meant it.
If we support use-defined id and someone have used your idea so he can make zram
per-user as uid. After a while, new application stats automatic id assignment
so upcoming users can consume upcoming user id. yeah, automaic id will start
from 0 so it's very rare to reach 1000 but who knows?
My point is in your usecase, the script would be very fragile so it should
have second plan like automatic id. 

> 
> these scripts should check if device has been created anyway, it just adds -EEXIST
> check. in general "what if user space does something wrong" thing can be beaten by
> "what if user space does everything right" argument. when script fails we just go
> and fix that script, I guess.

Yes, I believe finally the script will go automatic id if it was broken.
So, why does we should support user-defined id if they finally should
turn around from user-defined to automatic?


> 
> 	-ss
> 
> > 
> > Hmm, Coexisting specific and automatic id assign seem to be not a
> > godd idea.

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 11:04), Minchan Kim wrote:
> > we upgraded our scripts but landed some bugs there? it's up to particular
> > implementation. in your example, I assume, someone used zram with num_devices >= 1000?
> > that's impossible. current num_devices limitation is 32. and uid-s start from 1000.
> 
> I meant it.
> If we support use-defined id and someone have used your idea so he can make zram
> per-user as uid. After a while, new application stats automatic id assignment
> so upcoming users can consume upcoming user id. yeah, automaic id will start
> from 0 so it's very rare to reach 1000 but who knows?
> My point is in your usecase, the script would be very fragile so it should
> have second plan like automatic id.

I don't see how it turns any script into a _necessarily fragile_ one
here. there might be buggy scripts, yes. there might be no.

> > these scripts should check if device has been created anyway, it just adds -EEXIST
> > check. in general "what if user space does something wrong" thing can be beaten by
> > "what if user space does everything right" argument. when script fails we just go
> > and fix that script, I guess.
> 
> Yes, I believe finally the script will go automatic id if it was broken.

No, fixing a script does not imply at all switching to automatic device_id
assignment mode.


I think we discuss something that is not quite relevant to zram -- theoretical
ways someone does and fix things, and the policies and mistakes someone come up
with.

let me go and ask guys who asked for on demand device creation if they see
any value in user defined ids or they will be happy with the automatic one.
hopefully they will response in a day or two. can we wait? I'm not really
opposed to the removal of this functionality, but at the same time I see
some benefits.

> So, why does we should support user-defined id if they finally should
> turn around from user-defined to automatic?
> 

	-ss

Re: [PATCH 0/2] make automatic device_id generation possible

[Minchan Kim]

On Thu, Mar 5, 2015 at 11:27 AM, Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com> wrote:
> On (03/05/15 11:04), Minchan Kim wrote:
>> > we upgraded our scripts but landed some bugs there? it's up to particular
>> > implementation. in your example, I assume, someone used zram with num_devices >= 1000?
>> > that's impossible. current num_devices limitation is 32. and uid-s start from 1000.
>>
>> I meant it.
>> If we support use-defined id and someone have used your idea so he can make zram
>> per-user as uid. After a while, new application stats automatic id assignment
>> so upcoming users can consume upcoming user id. yeah, automaic id will start
>> from 0 so it's very rare to reach 1000 but who knows?
>> My point is in your usecase, the script would be very fragile so it should
>> have second plan like automatic id.
>
> I don't see how it turns any script into a _necessarily fragile_ one
> here. there might be buggy scripts, yes. there might be no.
>
>> > these scripts should check if device has been created anyway, it just adds -EEXIST
>> > check. in general "what if user space does something wrong" thing can be beaten by
>> > "what if user space does everything right" argument. when script fails we just go
>> > and fix that script, I guess.
>>
>> Yes, I believe finally the script will go automatic id if it was broken.
>
> No, fixing a script does not imply at all switching to automatic device_id
> assignment mode.

If the script fix another way, not automatic id, it might be broken sometime
and they should find an another workaround. It will repeat forever, I guess.

Anyway, let's wait opinions from userland. Please involve them in this
thread rather than another channel.

Thanks, Sergey!

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/2] make automatic device_id generation possible

[Karel Zak]

On Thu, Mar 05, 2015 at 09:58:29AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (03/05/15 09:20), Minchan Kim wrote:
> > I'm not against but I want to know why we should support
> > user-defined device id. What usecase do you have in mind?
> > 
> 
> hm, you never know what people can come up with. that's probably the
> strongest support argument I can provide. I wish there was something
> like - my friend Mike has a "device /dev/zram1 is always swap device,
> device /dev/zram$(id -u) is a per-user zram device (he finds it useful,

I have doubts that promise stable device names is good idea. The usual
way is to care about FS/SWAP identifiers (LABEL=, or UUID=), and for
example udevd should be able to create a stable /dev/disk/by-*
symlinks.

So for your friend Mike is better to have UUID= in /etc/fstab and
force mkswap or mkfs to use still the same UUID.

> user defined id support comes at a price of ~10 lines of code, or even
> less. we waste much more code to show ->stats, and not all of them are

I think it's not about number of code lines, it's kernel, you have to
support it forever, etc. It's easy to add a new feature, but you don't 
have to do it right now :)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

Cc Sami Kerola, Timofey Titovets, Karel Zak, util-linux
(don't have Jóhann B. Guðmundsson's email). also published on google+.

On (03/05/15 09:20), Minchan Kim wrote:
> > Make zram-contol/zram_add interface easier to use. Extend it to support
> > read and write operations.
> > 
> > Write operation remains the same:
> > 
> > 	echo X > /sys/class/zram-control/zram_add
> > 
> > will add /dev/zramX (or return error).
> > 
> > 
> > Read operation is treated as 'pick up available device_id, add new
> > device and return device_id'.
> > 
> > Example:
> > 	 cat /sys/class/zram-control/zram_add
> > 	2
> > 	 cat /sys/class/zram-control/zram_add
> > 	3
> 
> Thanks for handling my concern quickly and sorry for not sending
> active feedback in realtime. Maybe I should turn on CONFIG_PREEMPT
> in my brain.
> 
> I'm not against but I want to know why we should support
> user-defined device id. What usecase do you have in mind?
> 
> Could we support automatic id support only at this moment?
> Then, if some user complains about that in future, we could turn
> on user-defined device id easily and we could know the usecase.
> 
> In summary, I want to support only "cat /sys/class/zram-control/zram_add"
> unless you have feasible usecase.
> 
> What do you think about it?

Hello guys,

call for opinions.

we currently discuss on-demand device creation/removal (you can find
mail thread here https://lkml.org/lkml/2015/3/4/1414) and we want to hear
some opinions.

to add a new device, zram implements two modes (schematically):

-- forced mode

which is "add device /dev/zram<id>".
user must provide new device id. here we expect user to ensure that devide_id
uniqueness and to handle errors if it's not. (zram will return -EEXIST if
device_id is already in use).

I can think of... a build server with tons of users and /dev/zram$(id -u)
devices being created for every user during login (and destroyed during
logout). so users don't pressure hdd, they have zram devices to play with,
these devices have predictable names, which also makes admin's life a bit
easier -- he can find out device user by simply checking its name (to
remove abandoned devices, for example. anyway). there might be other more
or less real life use-cases.

usage example:
        echo `id -u` > /sys/class/zram-control/zram_add


-- automatic mode

which is 
"pick device_id automatically, add device /dev/zram<id>,
and return <id> back to user".

usage example:
        cat /sys/class/zram-control/zram_add
        7
        cat /sys/class/zram-control/zram_add
        8


we tend to ditch `forced' mode and leave `automatic' mode only. before we
do that we'd like to see how many guys will be pissed off and... basically,
if 'forced' mode is of any actual use.


so the question is: should there be two modes or one is just enough?

	-ss

Re: [PATCH 0/2] make automatic device_id generation possible

[Karel Zak]

On Thu, Mar 05, 2015 at 11:04:36AM +0900, Minchan Kim wrote:
> On Thu, Mar 05, 2015 at 10:47:52AM +0900, Sergey Senozhatsky wrote:
> > On (03/05/15 10:33), Minchan Kim wrote:
> > > > hm, I can think of a huge build server with tons of users. /dev/zram$(id -u)
> > > > created during user login and destroyed during logout. so users use theirs own
> > > > zram devices with predictable device ids (which also makes it simpler for admin)
> > > > for compilation/etc., and don't pressure hdds that much.
> > > 
> > > They upgraded the system and from now on, one of app tries automatic
> > > id with zram for some reason. What happens if he gets some user id
> > > before the user login? The system should have fallback in the case of
> > > failing to create own userid assignment.
> > 
> > we upgraded our scripts but landed some bugs there? it's up to particular
> > implementation. in your example, I assume, someone used zram with num_devices >= 1000?
> > that's impossible. current num_devices limitation is 32. and uid-s start from 1000.
> 
> I meant it.
> If we support use-defined id and someone have used your idea so he can make zram
> per-user as uid. After a while, new application stats automatic id assignment
> so upcoming users can consume upcoming user id. yeah, automaic id will start
> from 0 so it's very rare to reach 1000 but who knows?

Why 1000?  

The UID_MIN and UID_MAX is nothing strictly defined, it's just option
in /etc/login.defs. I guess it's nothing unusually to have system
where UID_MIN is 500 :-)

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 13:02), Karel Zak wrote:
> > hm, you never know what people can come up with. that's probably the
> > strongest support argument I can provide. I wish there was something
> > like - my friend Mike has a "device /dev/zram1 is always swap device,
> > device /dev/zram$(id -u) is a per-user zram device (he finds it useful,
> 
> I have doubts that promise stable device names is good idea. The usual
> way is to care about FS/SWAP identifiers (LABEL=, or UUID=), and for
> example udevd should be able to create a stable /dev/disk/by-*
> symlinks.
> 
> So for your friend Mike is better to have UUID= in /etc/fstab and
> force mkswap or mkfs to use still the same UUID.

+1 for removal from Karel.
thanks.

	-ss

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 13:10), Karel Zak wrote:
> > > we upgraded our scripts but landed some bugs there? it's up to particular
> > > implementation. in your example, I assume, someone used zram with num_devices >= 1000?
> > > that's impossible. current num_devices limitation is 32. and uid-s start from 1000.
[..]
> Why 1000?  
> 
> The UID_MIN and UID_MAX is nothing strictly defined, it's just option
> in /etc/login.defs. I guess it's nothing unusually to have system
> where UID_MIN is 500 :-)
> 

I meant on my system min is 1000 (don't know if anyone change min value
for any reason of his own), and current num_devices limit is 32, anyway.

	-ss

Re: [PATCH 0/2] make automatic device_id generation possible

[Sergey Senozhatsky]

On (03/05/15 09:20), Minchan Kim wrote:
> In summary, I want to support only "cat /sys/class/zram-control/zram_add"
> unless you have feasible usecase.
> 
> What do you think about it?
> 

Hello Minchan,

I've tried to contact as many guys (who has previously demonstrated some
interest in on-demand device creation) as I could in every sane way (using
both lkml and google+). and looks like people see no value in this
functionality.

so I'm happy to remove it. cleanup patch will arrive later today. thanks
for raising this topic.

	-ss