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

2017-03-08 Thread Cheah Kok Cheong
Dear Ian,
  Thanks for taking the time to reply.

On Wed, Mar 08, 2017 at 12:36:41PM +, 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 
> >---
> >
> >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:  )=-
> -=(  Web: http://www.mev.co.uk/  )=-


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

2017-03-08 Thread Cheah Kok Cheong
Dear Ian,
  Thanks for taking the time to reply.

On Wed, Mar 08, 2017 at 12:36:41PM +, 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 
> >---
> >
> >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:  )=-
> -=(  Web: http://www.mev.co.uk/  )=-


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

2017-03-08 Thread Ian Abbott

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.



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 
---

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:  )=-
-=(  Web: http://www.mev.co.uk/  )=-


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

2017-03-08 Thread Ian Abbott

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.



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 
---

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:  )=-
-=(  Web: http://www.mev.co.uk/  )=-


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

2017-03-07 Thread Cheah Kok Cheong
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.

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 
---

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.

 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 354d264..339854f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2857,9 +2857,9 @@ static int __init comedi_init(void)
 
pr_info("version " COMEDI_RELEASE " - http://www.comedi.org\n;);
 
-   if (comedi_num_legacy_minors > COMEDI_NUM_BOARD_MINORS) {
+   if (comedi_num_legacy_minors >= COMEDI_NUM_BOARD_MINORS) {
pr_err("invalid value for module parameter 
\"comedi_num_legacy_minors\".  Valid values are 0 through %i.\n",
-  COMEDI_NUM_BOARD_MINORS);
+  COMEDI_NUM_BOARD_MINORS - 1);
return -EINVAL;
}
 
-- 
2.7.4



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

2017-03-07 Thread Cheah Kok Cheong
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.

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 
---

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.

 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 354d264..339854f 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2857,9 +2857,9 @@ static int __init comedi_init(void)
 
pr_info("version " COMEDI_RELEASE " - http://www.comedi.org\n;);
 
-   if (comedi_num_legacy_minors > COMEDI_NUM_BOARD_MINORS) {
+   if (comedi_num_legacy_minors >= COMEDI_NUM_BOARD_MINORS) {
pr_err("invalid value for module parameter 
\"comedi_num_legacy_minors\".  Valid values are 0 through %i.\n",
-  COMEDI_NUM_BOARD_MINORS);
+  COMEDI_NUM_BOARD_MINORS - 1);
return -EINVAL;
}
 
-- 
2.7.4