lkml.org 
[lkml]   [2007]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH -mm take5 6/7] add ioctls for adding/removing target
Hi Keiichi,

On 6/13/07, Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
>
> We add ioctls for adding/removing target.
> If we use NETCONSOLE_ADD_TARGET ioctl,
> we can dynamically add netconsole target.
> If we use NETCONSOLE_REMOVE_TARGET ioctl,
> we can dynamically remoe netconsole target.

*ugh*. I was wondering what a show-stopper this particular patch
was -- introduces a couple of ioctl()'s, exports a new structure to
userspace, adds a hitherto-unneeded header file, brings in
tty_struct/tty_operations and ends up adding so much complexity/
bloat to netconsole.c. Not only that, it must live together (and
side-by-side) with the sysfs interface also, because the two of them
do different things: sysfs to be able to modify target parameters at
run-time and the ioctl()'s to dynamically add/remove targets. We
can't really mkdir(2) or rmdir(2) in sysfs so the ioctl()'s are needed.

So may I suggest:

Just lose *both* the sysfs and ioctl() interfaces and use _configfs_.
It is *precisely* the thing you need in your driver here -- the ability
to create / destroy kernel objects (or config_items in configfs lingo)
from _userspace_ via simple mkdir(2) and rmdir(2). And configfs
makes changing multiple configurable parameters atomically trivial
too, via rename(2) ... not to mention a sysfs+ioctls -> configfs
conversion would help your patchset lose some weight too :-)

Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-06-13 22:43    [W:1.768 / U:0.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site