Re: [PATCH v2 2/2] Staging: comedi: comedi_fops: Fix "out of minor numbers for board device files"

[Cheah Kok Cheong]

Dear Ian,
  Thanks for taking the time to reply.

On Wed, Mar 08, 2017 at 12:36:41PM +0000, Ian Abbott wrote:
> On 07/03/17 18:13, Cheah Kok Cheong wrote:
> >If comedi module is loaded with the following max allowed parameter
> >[comedi_num_legacy_minors=48], subsequent loading of an auto-configured
> >device will fail at auto-configuration. If there's no fall back in
> >place then module loading will fail.
> >
> >In this case, a default to auto-configure comedi_test module failed
> >to auto-configure with the following messages. It loaded but fell back
> >to unconfigured mode.
> >
> >comedi_test comedi_testd: ran out of minor numbers for board device files
> >comedi_test comedi_testd: driver 'comedi_test' could not create device.
> >comedi_test: unable to auto-configure device
> >
> >This is due to changes in commit 38b9722a4414
> >("staging: comedi: avoid releasing legacy minors automatically") which
> >will not allocate a minor number when comedi_num_legacy_minors equals
> >COMEDI_NUM_BOARD_MINORS. COMEDI_NUM_BOARD_MINORS is defined to be
> >0x30 which is 48.
> 
> Sorry, I don't consider this to be a bug.  The number of minor device
> numbers available for auto-configured devices is 48 minus
> comedi_num_legacy_minors.  Using up all available minor device numbers is a
> useful test case for running out of minor device numbers, although this
> relies on knowing the limit is 48.  Perhaps the description of the module
> parameter could be improved to mention the limits, as could the error
> message when running out of minor device numbers.
> 
> Commit 38b9722a4414 is irrelevant here.  Prior to that commit, the first
> 'comedi_num_legacy_minors' minor numbers were still allocated to legacy
> devices created during module initialization, leaving 48 minus
> comedi_num_legacy_minors minors (possibly none) available for
> auto-configured devices.
>

Thanks for the detailed explanation. It is rather strange to allow a user
to use up number 0 to 47 for legacy devices and leaving none for auto
configured devices. Since it's intentional as you've mentioned as a test
case, I agree it's best if some form of description or documentation
is in place. I'm sure this will be in good hands.

Sorry for any inconvenience caused.

Thanks.

Brgds,
CheahKC
  
> >This goes for a simple fix which limit comedi_num_legacy_minors to 47
> >instead of tinkering with comedi_alloc_board_minor() and
> >comedi_release_hardware_device().
> >
> >Fix: commit 38b9722a4414 ("staging: comedi: avoid releasing legacy minors
> >automatically")
> >
> >Signed-off-by: Cheah Kok Cheong <thrust73@xxxxxxxxx>
> >---
> >
> >V2:
> >-Amend commit log to specify that comedi_test module failed
> > to auto-configure and fell back to unconfigured mode.
> > For other devices with no fall back, module loading will fail.
> 
> -- 
> -=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
> -=(                          Web: http://www.mev.co.uk/  )=-