[RFC,2/9] ntsync: Reserve a minor device number and ioctl range.
Commit Message
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
Documentation/admin-guide/devices.txt | 3 ++-
Documentation/userspace-api/ioctl/ioctl-number.rst | 2 ++
drivers/misc/ntsync.c | 3 ++-
include/linux/miscdevice.h | 1 +
4 files changed, 7 insertions(+), 2 deletions(-)
Comments
On Tuesday, 23 January 2024 18:54:02 CST Greg Kroah-Hartman wrote:
> On Tue, Jan 23, 2024 at 06:40:21PM -0600, Elizabeth Figura wrote:
> > Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
> > ---
>
> Note, we can't take patches without any changelog text, and you don't
> want us to :)
>
> > Documentation/admin-guide/devices.txt | 3 ++-
> > Documentation/userspace-api/ioctl/ioctl-number.rst | 2 ++
> > drivers/misc/ntsync.c | 3 ++-
> > include/linux/miscdevice.h | 1 +
> > 4 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/devices.txt
> > b/Documentation/admin-guide/devices.txt index 94c98be1329a..041404397ee5
> > 100644
> > --- a/Documentation/admin-guide/devices.txt
> > +++ b/Documentation/admin-guide/devices.txt
> > @@ -376,8 +376,9 @@
> >
> > 240 = /dev/userio Serio driver testing device
> > 241 = /dev/vhost-vsock Host kernel driver for virtio
vsock
> > 242 = /dev/rfkill Turning off radio transmissions
(rfkill)
> >
> > + 243 = /dev/ntsync NT synchronization primitive
device
> >
> > - 243-254 Reserved for local use
> > + 244-254 Reserved for local use
>
> Why do you need a fixed minor number? Can't your userspace handle
> dynamic numbers? What systems require a static value?
I believe I added this because it's necessary for MODULE_ALIAS (and, more
broadly, because I was following the example of vaguely comparable devices
like /dev/loop-control). I suppose I could instead just remove MODULE_ALIAS
(or even remove the ability to compile ntsync as a module entirely).
It's a bit difficult to figure out what's the preferred way to organize things
like this (there not being a lot of precedent for this kind of driver) so I'd
appreciate any direction.
--Zeb
On Tue, Jan 23, 2024 at 09:43:09PM -0600, Elizabeth Figura wrote:
> > Why do you need a fixed minor number? Can't your userspace handle
> > dynamic numbers? What systems require a static value?
>
> I believe I added this because it's necessary for MODULE_ALIAS (and, more
> broadly, because I was following the example of vaguely comparable devices
> like /dev/loop-control). I suppose I could instead just remove MODULE_ALIAS
> (or even remove the ability to compile ntsync as a module entirely).
Do you really need MODULE_ALIAS()? Having it for char devices to be
auto-loaded is not generally considered a good idea anymore as systems
should have the module loaded before userspace goes and asks for it.
It also reduces suprises when any random userspace program can cause
kernel modules to be loaded for no real reason.
> It's a bit difficult to figure out what's the preferred way to organize things
> like this (there not being a lot of precedent for this kind of driver) so I'd
> appreciate any direction.
For now, I'd just stick to a dynamic id, no module alias, and if it's
ever needed in the future, it can be added. But if you add it now, we
can't ever remove it as it's user-visible functionality.
thanks,
greg k-h
On Wednesday, 24 January 2024 06:32:13 CST Greg Kroah-Hartman wrote:
> On Tue, Jan 23, 2024 at 09:43:09PM -0600, Elizabeth Figura wrote:
> > > Why do you need a fixed minor number? Can't your userspace handle
> > > dynamic numbers? What systems require a static value?
> >
> > I believe I added this because it's necessary for MODULE_ALIAS (and, more
> > broadly, because I was following the example of vaguely comparable devices
> > like /dev/loop-control). I suppose I could instead just remove MODULE_ALIAS
> > (or even remove the ability to compile ntsync as a module entirely).
>
> Do you really need MODULE_ALIAS()? Having it for char devices to be
> auto-loaded is not generally considered a good idea anymore as systems
> should have the module loaded before userspace goes and asks for it.
>
> It also reduces suprises when any random userspace program can cause
> kernel modules to be loaded for no real reason.
I think there's no reason we can't make loading the system's problem. I'll remove it in v2.
@@ -376,8 +376,9 @@
240 = /dev/userio Serio driver testing device
241 = /dev/vhost-vsock Host kernel driver for virtio vsock
242 = /dev/rfkill Turning off radio transmissions (rfkill)
+ 243 = /dev/ntsync NT synchronization primitive device
- 243-254 Reserved for local use
+ 244-254 Reserved for local use
255 Reserved for MISC_DYNAMIC_MINOR
11 char Raw keyboard device (Linux/SPARC only)
@@ -378,6 +378,8 @@ Code Seq# Include File Comments
<mailto:thomas@winischhofer.net>
0xF6 all LTTng Linux Trace Toolkit Next Generation
<mailto:mathieu.desnoyers@efficios.com>
+0xF7 00-1F uapi/linux/ntsync.h NT synchronization primitives
+ <mailto:wine-devel@winehq.org>
0xF8 all arch/x86/include/uapi/asm/amd_hsmp.h AMD HSMP EPYC system management interface driver
<mailto:nchatrad@amd.com>
0xFD all linux/dm-ioctl.h
@@ -40,7 +40,7 @@ static const struct file_operations ntsync_fops = {
};
static struct miscdevice ntsync_misc = {
- .minor = MISC_DYNAMIC_MINOR,
+ .minor = NTSYNC_MINOR,
.name = NTSYNC_NAME,
.fops = &ntsync_fops,
};
@@ -51,3 +51,4 @@ MODULE_AUTHOR("Elizabeth Figura");
MODULE_DESCRIPTION("Kernel driver for NT synchronization primitives");
MODULE_LICENSE("GPL");
MODULE_ALIAS("devname:" NTSYNC_NAME);
+MODULE_ALIAS_MISCDEV(NTSYNC_MINOR);
@@ -71,6 +71,7 @@
#define USERIO_MINOR 240
#define VHOST_VSOCK_MINOR 241
#define RFKILL_MINOR 242
+#define NTSYNC_MINOR 243
#define MISC_DYNAMIC_MINOR 255
struct device;