On Tue, Jan 23, 2024 at 06:40:21PM -0600, Elizabeth Figura wrote:
> Signed-off-by: Elizabeth Figura <[email protected]>
> ---
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?
thanks,
greg k-h
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 <[email protected]>
> > ---
>
> 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.