2020-01-20 08:35:03

by Zhenzhong Duan

[permalink] [raw]
Subject: Question about dynamic minor number of misc device

Hi Maintainers,

I see there are 64 free slots(0-63) used for misc devices with dynamic
minor number. But PSMOUSE_MINOR(1) overlaps with that dynamic range.

So if the dynamic minor number exhaust, psaux driver will fail with
"could not register psaux device, error: -16", is this expected?
Should we preserve a slot for psaux and serio_raw which use static
minor number PSMOUSE_MINOR?

Thanks
Zhenzhong


2020-01-20 10:05:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 9:33 AM Zhenzhong Duan <[email protected]> wrote:
>
> Hi Maintainers,
>
> I see there are 64 free slots(0-63) used for misc devices with dynamic
> minor number. But PSMOUSE_MINOR(1) overlaps with that dynamic range.
>
> So if the dynamic minor number exhaust, psaux driver will fail with
> "could not register psaux device, error: -16", is this expected?
> Should we preserve a slot for psaux and serio_raw which use static
> minor number PSMOUSE_MINOR?

Is this a theoretical question, or are you actually running out of dynamic
minor numbers? I would guess that if we change the limit to only allow
dynamic minors 2 through 63, that would technically be a correct
change, but the result would be that in the same situation another random
driver fails, which is not much of an improvement, unless the dynamic
minor numbers also continue into the range above 255.

On a related note, I checked for drivers that call misc_register()
with a minor number that is not defined in include/linux/misc.h
and found a bunch, including some that have conflicting numbers,
conflicting names or numbers from the dynamic range:

drivers/staging/speakup/devsynth.c:#define SYNTH_MINOR 25
drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTH_MINOR 26 /*
might as well give it one more than /dev/synth */
drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTHU_MINOR 27 /*
might as well give it one more than /dev/synth */
drivers/macintosh/via-pmu.c:#define PMU_MINOR 154
drivers/macintosh/ans-lcd.h:#define ANSLCD_MINOR 156
drivers/auxdisplay/charlcd.c:#define LCD_MINOR 156
drivers/char/applicom.c:#define AC_MINOR 157
drivers/char/nwbutton.h:#define BUTTON_MINOR 158
arch/arm/include/asm/nwflash.h:#define FLASH_MINOR 160
drivers/sbus/char/envctrl.c:#define ENVCTRL_MINOR 162
drivers/sbus/char/flash.c:#define FLASH_MINOR 152
drivers/sbus/char/uctrl.c:#define UCTRL_MINOR 174
drivers/char/toshiba.c:#define TOSH_MINOR_DEV 181
arch/um/drivers/random.c:#define RNG_MISCDEV_MINOR 183 /*
official */
drivers/auxdisplay/panel.c:#define KEYPAD_MINOR 185
drivers/video/fbdev/pxa3xx-gcu.c:#define MISCDEV_MINOR 197
kernel/power/user.c:#define SNAPSHOT_MINOR 231
drivers/parisc/eisa_eeprom.c:#define EISA_EEPROM_MINOR 241

If you would like to help clean that up, you are definitely welcome
to send patches.

Arnd

2020-01-20 10:27:20

by Zhenzhong Duan

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 6:03 PM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Jan 20, 2020 at 9:33 AM Zhenzhong Duan <[email protected]> wrote:
> >
> > Hi Maintainers,
> >
> > I see there are 64 free slots(0-63) used for misc devices with dynamic
> > minor number. But PSMOUSE_MINOR(1) overlaps with that dynamic range.
> >
> > So if the dynamic minor number exhaust, psaux driver will fail with
> > "could not register psaux device, error: -16", is this expected?
> > Should we preserve a slot for psaux and serio_raw which use static
> > minor number PSMOUSE_MINOR?
>
> Is this a theoretical question, or are you actually running out of dynamic
> minor numbers? I would guess that if we change the limit to only allow
> dynamic minors 2 through 63, that would technically be a correct
> change, but the result would be that in the same situation another random
> driver fails, which is not much of an improvement, unless the dynamic
> minor numbers also continue into the range above 255.

It's theoretical question. Thanks for your explanation.

>
> On a related note, I checked for drivers that call misc_register()
> with a minor number that is not defined in include/linux/misc.h
> and found a bunch, including some that have conflicting numbers,
> conflicting names or numbers from the dynamic range:
>
> drivers/staging/speakup/devsynth.c:#define SYNTH_MINOR 25
> drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTH_MINOR 26 /*
> might as well give it one more than /dev/synth */
> drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTHU_MINOR 27 /*
> might as well give it one more than /dev/synth */
> drivers/macintosh/via-pmu.c:#define PMU_MINOR 154
> drivers/macintosh/ans-lcd.h:#define ANSLCD_MINOR 156
> drivers/auxdisplay/charlcd.c:#define LCD_MINOR 156
> drivers/char/applicom.c:#define AC_MINOR 157
> drivers/char/nwbutton.h:#define BUTTON_MINOR 158
> arch/arm/include/asm/nwflash.h:#define FLASH_MINOR 160
> drivers/sbus/char/envctrl.c:#define ENVCTRL_MINOR 162
> drivers/sbus/char/flash.c:#define FLASH_MINOR 152
> drivers/sbus/char/uctrl.c:#define UCTRL_MINOR 174
> drivers/char/toshiba.c:#define TOSH_MINOR_DEV 181
> arch/um/drivers/random.c:#define RNG_MISCDEV_MINOR 183 /*
> official */
> drivers/auxdisplay/panel.c:#define KEYPAD_MINOR 185
> drivers/video/fbdev/pxa3xx-gcu.c:#define MISCDEV_MINOR 197
> kernel/power/user.c:#define SNAPSHOT_MINOR 231
> drivers/parisc/eisa_eeprom.c:#define EISA_EEPROM_MINOR 241
>
> If you would like to help clean that up, you are definitely welcome
> to send patches.

Ok, should that be a patch for all drivers or seperate patch for each driver?

Thanks
Zhenzhong

2020-01-20 11:01:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 11:26 AM Zhenzhong Duan
<[email protected]> wrote:
> On Mon, Jan 20, 2020 at 6:03 PM Arnd Bergmann <[email protected]> wrote:
> > On Mon, Jan 20, 2020 at 9:33 AM Zhenzhong Duan <[email protected]> wrote:
> > On a related note, I checked for drivers that call misc_register()
> > with a minor number that is not defined in include/linux/misc.h
> > and found a bunch, including some that have conflicting numbers,
> > conflicting names or numbers from the dynamic range:
> >
> > drivers/staging/speakup/devsynth.c:#define SYNTH_MINOR 25
> > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTH_MINOR 26 /*
> > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTHU_MINOR 27 /*
> > drivers/macintosh/via-pmu.c:#define PMU_MINOR 154
> > drivers/macintosh/ans-lcd.h:#define ANSLCD_MINOR 156
> > drivers/auxdisplay/charlcd.c:#define LCD_MINOR 156
> > drivers/char/applicom.c:#define AC_MINOR 157
> > drivers/char/nwbutton.h:#define BUTTON_MINOR 158
> > arch/arm/include/asm/nwflash.h:#define FLASH_MINOR 160
> > drivers/sbus/char/envctrl.c:#define ENVCTRL_MINOR 162
> > drivers/sbus/char/flash.c:#define FLASH_MINOR 152
> > drivers/sbus/char/uctrl.c:#define UCTRL_MINOR 174
> > drivers/char/toshiba.c:#define TOSH_MINOR_DEV 181
> > arch/um/drivers/random.c:#define RNG_MISCDEV_MINOR
> > drivers/auxdisplay/panel.c:#define KEYPAD_MINOR 185
> > drivers/video/fbdev/pxa3xx-gcu.c:#define MISCDEV_MINOR 197
> > kernel/power/user.c:#define SNAPSHOT_MINOR 231
> > drivers/parisc/eisa_eeprom.c:#define EISA_EEPROM_MINOR 241
> >
> > If you would like to help clean that up, you are definitely welcome
> > to send patches.
>
> Ok, should that be a patch for all drivers or seperate patch for each driver?

I think one patch to move the ones with unique names would be fine,
but then separate patches for

- FLASH_MINOR move and rename to avoid conflict
- change speakup to dynamic minors
- support for high dynamic minor numbers if you are really motivated
(probably nobody needs these)

Arnd

2020-01-20 22:14:59

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 11:59:32AM +0100, Arnd Bergmann wrote:
> On Mon, Jan 20, 2020 at 11:26 AM Zhenzhong Duan
> <[email protected]> wrote:
> > On Mon, Jan 20, 2020 at 6:03 PM Arnd Bergmann <[email protected]> wrote:
> > > On Mon, Jan 20, 2020 at 9:33 AM Zhenzhong Duan <[email protected]> wrote:
> > > On a related note, I checked for drivers that call misc_register()
> > > with a minor number that is not defined in include/linux/misc.h
> > > and found a bunch, including some that have conflicting numbers,
> > > conflicting names or numbers from the dynamic range:
> > >
> > > drivers/staging/speakup/devsynth.c:#define SYNTH_MINOR 25
> > > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTH_MINOR 26 /*
> > > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTHU_MINOR 27 /*
> > > drivers/macintosh/via-pmu.c:#define PMU_MINOR 154
> > > drivers/macintosh/ans-lcd.h:#define ANSLCD_MINOR 156
> > > drivers/auxdisplay/charlcd.c:#define LCD_MINOR 156
> > > drivers/char/applicom.c:#define AC_MINOR 157
> > > drivers/char/nwbutton.h:#define BUTTON_MINOR 158
> > > arch/arm/include/asm/nwflash.h:#define FLASH_MINOR 160
> > > drivers/sbus/char/envctrl.c:#define ENVCTRL_MINOR 162
> > > drivers/sbus/char/flash.c:#define FLASH_MINOR 152
> > > drivers/sbus/char/uctrl.c:#define UCTRL_MINOR 174
> > > drivers/char/toshiba.c:#define TOSH_MINOR_DEV 181
> > > arch/um/drivers/random.c:#define RNG_MISCDEV_MINOR
> > > drivers/auxdisplay/panel.c:#define KEYPAD_MINOR 185
> > > drivers/video/fbdev/pxa3xx-gcu.c:#define MISCDEV_MINOR 197
> > > kernel/power/user.c:#define SNAPSHOT_MINOR 231
> > > drivers/parisc/eisa_eeprom.c:#define EISA_EEPROM_MINOR 241
> > >
> > > If you would like to help clean that up, you are definitely welcome
> > > to send patches.
> >
> > Ok, should that be a patch for all drivers or seperate patch for each driver?
>
> I think one patch to move the ones with unique names would be fine,
> but then separate patches for
>
> - FLASH_MINOR move and rename to avoid conflict
> - change speakup to dynamic minors
> - support for high dynamic minor numbers if you are really motivated
> (probably nobody needs these)

Are we sure that reassigning minor device number conflits isn't going
to break systems? Especially those on random, older, architectures
they might not be using udev.

- Ted

2020-01-21 07:58:04

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 11:13 PM Theodore Y. Ts'o <[email protected]> wrote:
>
> On Mon, Jan 20, 2020 at 11:59:32AM +0100, Arnd Bergmann wrote:
> > On Mon, Jan 20, 2020 at 11:26 AM Zhenzhong Duan
> > <[email protected]> wrote:
> > > On Mon, Jan 20, 2020 at 6:03 PM Arnd Bergmann <[email protected]> wrote:
> > > > On Mon, Jan 20, 2020 at 9:33 AM Zhenzhong Duan <[email protected]> wrote:
> > > > On a related note, I checked for drivers that call misc_register()
> > > > with a minor number that is not defined in include/linux/misc.h
> > > > and found a bunch, including some that have conflicting numbers,
> > > > conflicting names or numbers from the dynamic range:
> > > >
> > > > drivers/staging/speakup/devsynth.c:#define SYNTH_MINOR 25
> > > > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTH_MINOR 26 /*
> > > > drivers/staging/speakup/speakup_soft.c:#define SOFTSYNTHU_MINOR 27 /*
> > > > drivers/macintosh/via-pmu.c:#define PMU_MINOR 154
> > > > drivers/macintosh/ans-lcd.h:#define ANSLCD_MINOR 156
> > > > drivers/auxdisplay/charlcd.c:#define LCD_MINOR 156
> > > > drivers/char/applicom.c:#define AC_MINOR 157
> > > > drivers/char/nwbutton.h:#define BUTTON_MINOR 158
> > > > arch/arm/include/asm/nwflash.h:#define FLASH_MINOR 160
> > > > drivers/sbus/char/envctrl.c:#define ENVCTRL_MINOR 162
> > > > drivers/sbus/char/flash.c:#define FLASH_MINOR 152
> > > > drivers/sbus/char/uctrl.c:#define UCTRL_MINOR 174
> > > > drivers/char/toshiba.c:#define TOSH_MINOR_DEV 181
> > > > arch/um/drivers/random.c:#define RNG_MISCDEV_MINOR
> > > > drivers/auxdisplay/panel.c:#define KEYPAD_MINOR 185
> > > > drivers/video/fbdev/pxa3xx-gcu.c:#define MISCDEV_MINOR 197
> > > > kernel/power/user.c:#define SNAPSHOT_MINOR 231
> > > > drivers/parisc/eisa_eeprom.c:#define EISA_EEPROM_MINOR 241
> > > >
> > > > If you would like to help clean that up, you are definitely welcome
> > > > to send patches.
> > >
> > > Ok, should that be a patch for all drivers or seperate patch for each driver?
> >
> > I think one patch to move the ones with unique names would be fine,
> > but then separate patches for
> >
> > - FLASH_MINOR move and rename to avoid conflict
> > - change speakup to dynamic minors
> > - support for high dynamic minor numbers if you are really motivated
> > (probably nobody needs these)
>
> Are we sure that reassigning minor device number conflits isn't going
> to break systems? Especially those on random, older, architectures
> they might not be using udev.

To clarify: the only numbers that I think should be changed to dynamic
allocation are for drivers/staging/speakup. While this is a fairly old
subsystem, I would expect that it being staging means we can be a
little more progressive with the changes.

Arnd

2020-01-21 15:43:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Mon, Jan 20, 2020 at 05:13:23PM -0500, Theodore Y. Ts'o wrote:
> > - FLASH_MINOR move and rename to avoid conflict
> > - change speakup to dynamic minors
> > - support for high dynamic minor numbers if you are really motivated
> > (probably nobody needs these)
>
> Are we sure that reassigning minor device number conflits isn't going
> to break systems? Especially those on random, older, architectures
> they might not be using udev.

Note, I do not think udev has been in charge of creating /dev/ nodes for
well over 10 years now, that is up to devtmpfs.

thanks,

greg k-h

2020-01-21 16:32:51

by Theodore Ts'o

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

On Tue, Jan 21, 2020 at 08:56:37AM +0100, Arnd Bergmann wrote:
> > > I think one patch to move the ones with unique names would be fine,
> > > but then separate patches for
> > >
> > > - FLASH_MINOR move and rename to avoid conflict
> > > - change speakup to dynamic minors
> > > - support for high dynamic minor numbers if you are really motivated
> > > (probably nobody needs these)
> >
> > Are we sure that reassigning minor device number conflits isn't going
> > to break systems? Especially those on random, older, architectures
> > they might not be using udev.
>
> To clarify: the only numbers that I think should be changed to dynamic
> allocation are for drivers/staging/speakup. While this is a fairly old
> subsystem, I would expect that it being staging means we can be a
> little more progressive with the changes.

Sam,

Would you happen to know how commonly used the speakup system would be
--- in particular, on non-udev systems where changing the minor number
of the device node might break some folks? Does your hardware system
use speakup, or some other interface?

Also, who would be the best people to reach out at the
linux-speakup.org project to verify what the potential impact might be
of making this change. It looks like some of the web pages are a bit
dated, so I wasn't sure what's up to date.

Thanks!!

- Ted

2020-01-21 18:04:53

by Sam Hartman

[permalink] [raw]
Subject: Re: Question about dynamic minor number of misc device

>>>>> "Theodore" == Theodore Y Ts'o <[email protected]> writes:

Theodore> Sam,

Theodore> Would you happen to know how commonly used the speakup system would be
Theodore> --- in particular, on non-udev systems where changing the minor number
Theodore> of the device node might break some folks? Does your hardware system
Theodore> use speakup, or some other interface?

Speakup is used by things like the Debian Installer in speech mode.
I'd assume D-I uses udev.

However Speakup is also likely to be used by blind people who prefer
older environments--no GUI, no pulseaudio, that sort of thing.
No udev is kind of pushing that mindset to the extreme though.
Speakup is not typically used without a keyboard or similar, so you're
not going to see it on embedded systems.





Theodore> Also, who would be the best people to reach out at the
Theodore> linux-speakup.org project to verify what the potential impact might be
Theodore> of making this change. It looks like some of the web pages are a bit
Theodore> dated, so I wasn't sure what's up to date.

I might ask on [email protected].
My recollection is that the upstream is not very energetic, and that the
distros keep speakup working because it's quite important for some
people.
We broke it on hda_intel for the original Buster release and that
certainly generated lots of user feedback.

debian-accessibility is Debian specific. There is the more general
[email protected] (blind linux users), but that lists tends to
be so user focused that you might not get good feedback to a question
like this.

--Sam