Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-03-02 Thread Alan Cox
> > This is a long term plan, of course, but I'd like to see sysfs functions go 
> > away
> > in a year or so. What do you think?
> 
> hoo boy.  Creating a /dev node and doing ioctls on it is really old
> school.  So old school that I've forgotten why we don't do it any more.
> 
> Hopefully Alan can recall the thinking?

Gee.. pass me the walking stick and the pipe !

The problem we used to have with it was re-use of /dev nodes and
permissions. For example if I create a ZRAM instance and then we crash
and my /dev is persistent then next boot I can potentially be opening
that left over node (or it may even be forgotten), which has old and
stale permissions on it.

With devtmpfs it's a bit easier and saner.

The old abuser of this was the old PCMCIA layer which has more exciting
versions of the problem as it used /tmp for some nodes.

Switching to sysfs probably fits better but the problem is the generic
one of revoking access rights to an object.

Alan
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-03-02 Thread Alan Cox
  This is a long term plan, of course, but I'd like to see sysfs functions go 
  away
  in a year or so. What do you think?
 
 hoo boy.  Creating a /dev node and doing ioctls on it is really old
 school.  So old school that I've forgotten why we don't do it any more.
 
 Hopefully Alan can recall the thinking?

Gee.. pass me the walking stick and the pipe !

The problem we used to have with it was re-use of /dev nodes and
permissions. For example if I create a ZRAM instance and then we crash
and my /dev is persistent then next boot I can potentially be opening
that left over node (or it may even be forgotten), which has old and
stale permissions on it.

With devtmpfs it's a bit easier and saner.

The old abuser of this was the old PCMCIA layer which has more exciting
versions of the problem as it used /tmp for some nodes.

Switching to sysfs probably fits better but the problem is the generic
one of revoking access rights to an object.

Alan
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Andrew Morton
On Sat, 28 Feb 2015 12:34:15 +0900 Sergey Senozhatsky 
 wrote:

> On (02/27/15 14:51), Andrew Morton wrote:
> > hoo boy.  Creating a /dev node and doing ioctls on it is really old
> > school.  So old school that I've forgotten why we don't do it any more.
> > 
> > Hopefully Alan can recall the thinking?
> > 
> 
> perhaps, something like
> 
>  static struct class_attribute zram_class_attrs[] = {
>  __ATTR(zram_control, S_IWUSR | S_IRUGO,
>  zram_control_show, zram_control_store),
>  __ATTR_NULL,
>  };
> 
>  struct class zram_class = {
>  .name   = "zram-control",
>  .class_attrs= zram_class_attrs,
>  };
> 
> 
>  class_register(_class);
> 
> 
> 
> or (even better) separate control files
> 
>  static struct class_attribute zram_class_attrs[] = {
>  __ATTR(zram_add, ),
>  __ATTR(zram_remove, ),
>  __ATTR_NULL,
>  };
> 
> 
> so we can just echo `device_id' to add/remove devices
> 
> echo 1 > /sys/class/zram-control/zram_add
> echo 1 > /sys/class/zram-control/zram_remove
> 
> 
> handling it in FOO_store() functions:
> 
>  static ssize_t zram_add_store(struct class *class,
>  struct class_attribute *attr,
>  char *buf)
> 
> 
> how about this?

That would be more conventional.  There's no pretty way of doing this
really.  

http://yarchive.net/comp/linux/ioctl.html does a pretty good job of the
thinking on ioctl - mainly the typelessness of it all.  That's very old
and doesn't discuss the 32/64 bit compat issues.

Your patch doesn't really have this problem at this stage, but once the
ioctl interface is added, new controls which are added later should use
it, and we're likely to get into the same issues.

--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/27/15 14:51), Andrew Morton wrote:
> hoo boy.  Creating a /dev node and doing ioctls on it is really old
> school.  So old school that I've forgotten why we don't do it any more.
> 
> Hopefully Alan can recall the thinking?
> 

perhaps, something like

 static struct class_attribute zram_class_attrs[] = {
 __ATTR(zram_control, S_IWUSR | S_IRUGO,
 zram_control_show, zram_control_store),
 __ATTR_NULL,
 };

 struct class zram_class = {
 .name   = "zram-control",
 .class_attrs= zram_class_attrs,
 };


 class_register(_class);



or (even better) separate control files

 static struct class_attribute zram_class_attrs[] = {
 __ATTR(zram_add, ),
 __ATTR(zram_remove, ),
 __ATTR_NULL,
 };


so we can just echo `device_id' to add/remove devices

echo 1 > /sys/class/zram-control/zram_add
echo 1 > /sys/class/zram-control/zram_remove


handling it in FOO_store() functions:

 static ssize_t zram_add_store(struct class *class,
 struct class_attribute *attr,
 char *buf)


how about this?


-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/28/15 10:33), Sergey Senozhatsky wrote:
> > hoo boy.  Creating a /dev node and doing ioctls on it is really old
> > school.  So old school that I've forgotten why we don't do it any more.
> > 
> > Hopefully Alan can recall the thinking?
> 
> oh. I thought this is how loop control works, and ioctl there doesn't look
> insane to me.
> 
> any quick hint how do you do this in a modern world so I'll redo the patch 
> set?
> 

I'll change it to sysfs RW control node.

-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/27/15 14:51), Andrew Morton wrote:
> hoo boy.  Creating a /dev node and doing ioctls on it is really old
> school.  So old school that I've forgotten why we don't do it any more.
> 
> Hopefully Alan can recall the thinking?

oh. I thought this is how loop control works, and ioctl there doesn't look
insane to me.

any quick hint how do you do this in a modern world so I'll redo the patch set?

-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Andrew Morton
On Thu, 26 Feb 2015 23:10:35 +0900 Sergey Senozhatsky 
 wrote:

> Hello,
> 
> this patchset introduces dynamic (on demand) zram device add-remove
> functionality via /dev/zram-control interface. Two ioctl commands are
> defined as of now (accessible in user-space via new zram.h header file):
> -- ZRAM_CTL_ADD
>   add new device (generates device_id automatically or uses provided
>   device_id)
> -- ZRAM_CTL_REMOVE
>   remove device (by device_id)
> 
> util-linux zramctl update will be done later, after we land this patchset.
> 
> 
> This also opens a possibility to drop some of sysfs device attrs and 
> FOO_show()
> code duplication in the future, and provide device stats/info via ioctl call
> instead, providing something like (via zram.h):
> 
>   struct zram_info {
>   __u64 orig_data_size;
>   __u64 mem_used_total;
>   __u64 max_comp_streams;
> 
>   [..]
>   };
> 
> 
> fill it under ->init_lock in zram_fill_info() (or any other name) function and
> return all device stats at once back to user-space in a single syscall.
> 
> This is a long term plan, of course, but I'd like to see sysfs functions go 
> away
> in a year or so. What do you think?

hoo boy.  Creating a /dev node and doing ioctls on it is really old
school.  So old school that I've forgotten why we don't do it any more.

Hopefully Alan can recall the thinking?
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Andrew Morton
On Sat, 28 Feb 2015 12:34:15 +0900 Sergey Senozhatsky 
sergey.senozhatsky.w...@gmail.com wrote:

 On (02/27/15 14:51), Andrew Morton wrote:
  hoo boy.  Creating a /dev node and doing ioctls on it is really old
  school.  So old school that I've forgotten why we don't do it any more.
  
  Hopefully Alan can recall the thinking?
  
 
 perhaps, something like
 
  static struct class_attribute zram_class_attrs[] = {
  __ATTR(zram_control, S_IWUSR | S_IRUGO,
  zram_control_show, zram_control_store),
  __ATTR_NULL,
  };
 
  struct class zram_class = {
  .name   = zram-control,
  .class_attrs= zram_class_attrs,
  };
 
 
  class_register(zram_class);
 
 
 
 or (even better) separate control files
 
  static struct class_attribute zram_class_attrs[] = {
  __ATTR(zram_add, ),
  __ATTR(zram_remove, ),
  __ATTR_NULL,
  };
 
 
 so we can just echo `device_id' to add/remove devices
 
 echo 1  /sys/class/zram-control/zram_add
 echo 1  /sys/class/zram-control/zram_remove
 
 
 handling it in FOO_store() functions:
 
  static ssize_t zram_add_store(struct class *class,
  struct class_attribute *attr,
  char *buf)
 
 
 how about this?

That would be more conventional.  There's no pretty way of doing this
really.  

http://yarchive.net/comp/linux/ioctl.html does a pretty good job of the
thinking on ioctl - mainly the typelessness of it all.  That's very old
and doesn't discuss the 32/64 bit compat issues.

Your patch doesn't really have this problem at this stage, but once the
ioctl interface is added, new controls which are added later should use
it, and we're likely to get into the same issues.

--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/27/15 14:51), Andrew Morton wrote:
 hoo boy.  Creating a /dev node and doing ioctls on it is really old
 school.  So old school that I've forgotten why we don't do it any more.
 
 Hopefully Alan can recall the thinking?

oh. I thought this is how loop control works, and ioctl there doesn't look
insane to me.

any quick hint how do you do this in a modern world so I'll redo the patch set?

-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/27/15 14:51), Andrew Morton wrote:
 hoo boy.  Creating a /dev node and doing ioctls on it is really old
 school.  So old school that I've forgotten why we don't do it any more.
 
 Hopefully Alan can recall the thinking?
 

perhaps, something like

 static struct class_attribute zram_class_attrs[] = {
 __ATTR(zram_control, S_IWUSR | S_IRUGO,
 zram_control_show, zram_control_store),
 __ATTR_NULL,
 };

 struct class zram_class = {
 .name   = zram-control,
 .class_attrs= zram_class_attrs,
 };


 class_register(zram_class);



or (even better) separate control files

 static struct class_attribute zram_class_attrs[] = {
 __ATTR(zram_add, ),
 __ATTR(zram_remove, ),
 __ATTR_NULL,
 };


so we can just echo `device_id' to add/remove devices

echo 1  /sys/class/zram-control/zram_add
echo 1  /sys/class/zram-control/zram_remove


handling it in FOO_store() functions:

 static ssize_t zram_add_store(struct class *class,
 struct class_attribute *attr,
 char *buf)


how about this?


-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Sergey Senozhatsky
On (02/28/15 10:33), Sergey Senozhatsky wrote:
  hoo boy.  Creating a /dev node and doing ioctls on it is really old
  school.  So old school that I've forgotten why we don't do it any more.
  
  Hopefully Alan can recall the thinking?
 
 oh. I thought this is how loop control works, and ioctl there doesn't look
 insane to me.
 
 any quick hint how do you do this in a modern world so I'll redo the patch 
 set?
 

I'll change it to sysfs RW control node.

-ss
--
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/


Re: [PATCH 0/8] introduce dynamic device creation/removal

2015-02-27 Thread Andrew Morton
On Thu, 26 Feb 2015 23:10:35 +0900 Sergey Senozhatsky 
sergey.senozhat...@gmail.com wrote:

 Hello,
 
 this patchset introduces dynamic (on demand) zram device add-remove
 functionality via /dev/zram-control interface. Two ioctl commands are
 defined as of now (accessible in user-space via new zram.h header file):
 -- ZRAM_CTL_ADD
   add new device (generates device_id automatically or uses provided
   device_id)
 -- ZRAM_CTL_REMOVE
   remove device (by device_id)
 
 util-linux zramctl update will be done later, after we land this patchset.
 
 
 This also opens a possibility to drop some of sysfs device attrs and 
 FOO_show()
 code duplication in the future, and provide device stats/info via ioctl call
 instead, providing something like (via zram.h):
 
   struct zram_info {
   __u64 orig_data_size;
   __u64 mem_used_total;
   __u64 max_comp_streams;
 
   [..]
   };
 
 
 fill it under -init_lock in zram_fill_info() (or any other name) function and
 return all device stats at once back to user-space in a single syscall.
 
 This is a long term plan, of course, but I'd like to see sysfs functions go 
 away
 in a year or so. What do you think?

hoo boy.  Creating a /dev node and doing ioctls on it is really old
school.  So old school that I've forgotten why we don't do it any more.

Hopefully Alan can recall the thinking?
--
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/


[PATCH 0/8] introduce dynamic device creation/removal

2015-02-26 Thread Sergey Senozhatsky
Hello,

this patchset introduces dynamic (on demand) zram device add-remove
functionality via /dev/zram-control interface. Two ioctl commands are
defined as of now (accessible in user-space via new zram.h header file):
-- ZRAM_CTL_ADD
add new device (generates device_id automatically or uses provided
device_id)
-- ZRAM_CTL_REMOVE
remove device (by device_id)

util-linux zramctl update will be done later, after we land this patchset.


This also opens a possibility to drop some of sysfs device attrs and FOO_show()
code duplication in the future, and provide device stats/info via ioctl call
instead, providing something like (via zram.h):

struct zram_info {
__u64 orig_data_size;
__u64 mem_used_total;
__u64 max_comp_streams;

[..]
};


fill it under ->init_lock in zram_fill_info() (or any other name) function and
return all device stats at once back to user-space in a single syscall.

This is a long term plan, of course, but I'd like to see sysfs functions go away
in a year or so. What do you think?


Sergey Senozhatsky (8):
  zram: cosmetic ZRAM_ATTR_RO code formatting tweak
  zram: use idr instead of `zram_devices' array
  zram: factor out device reset from reset_store()
  zram: add dynamic device add/remove functionality
  zram: return zram device_id value from zram_add()
  zram: allow automatic new zram device_id assignment
  zram: remove max_num_devices limitation
  zram: report every added and removed device

 drivers/block/zram/zram_drv.c | 326 --
 drivers/block/zram/zram_drv.h |   6 -
 include/linux/miscdevice.h|   1 +
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/zram.h |  17 +++
 5 files changed, 238 insertions(+), 113 deletions(-)
 create mode 100644 include/uapi/linux/zram.h

-- 
2.3.1.167.g7f4ba4b

--
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/


[PATCH 0/8] introduce dynamic device creation/removal

2015-02-26 Thread Sergey Senozhatsky
Hello,

this patchset introduces dynamic (on demand) zram device add-remove
functionality via /dev/zram-control interface. Two ioctl commands are
defined as of now (accessible in user-space via new zram.h header file):
-- ZRAM_CTL_ADD
add new device (generates device_id automatically or uses provided
device_id)
-- ZRAM_CTL_REMOVE
remove device (by device_id)

util-linux zramctl update will be done later, after we land this patchset.


This also opens a possibility to drop some of sysfs device attrs and FOO_show()
code duplication in the future, and provide device stats/info via ioctl call
instead, providing something like (via zram.h):

struct zram_info {
__u64 orig_data_size;
__u64 mem_used_total;
__u64 max_comp_streams;

[..]
};


fill it under -init_lock in zram_fill_info() (or any other name) function and
return all device stats at once back to user-space in a single syscall.

This is a long term plan, of course, but I'd like to see sysfs functions go away
in a year or so. What do you think?


Sergey Senozhatsky (8):
  zram: cosmetic ZRAM_ATTR_RO code formatting tweak
  zram: use idr instead of `zram_devices' array
  zram: factor out device reset from reset_store()
  zram: add dynamic device add/remove functionality
  zram: return zram device_id value from zram_add()
  zram: allow automatic new zram device_id assignment
  zram: remove max_num_devices limitation
  zram: report every added and removed device

 drivers/block/zram/zram_drv.c | 326 --
 drivers/block/zram/zram_drv.h |   6 -
 include/linux/miscdevice.h|   1 +
 include/uapi/linux/Kbuild |   1 +
 include/uapi/linux/zram.h |  17 +++
 5 files changed, 238 insertions(+), 113 deletions(-)
 create mode 100644 include/uapi/linux/zram.h

-- 
2.3.1.167.g7f4ba4b

--
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/