Hi Logan,
On 1/30/18 5:26 PM, Logan Gunthorpe wrote:
>
>
> On 30/01/18 05:56 PM, Srivatsa S. Bhat wrote:
>> If the restriction on the major number was intentional, perhaps we
>> should get the LTP testcase modified for kernel versions >= 4.14.
>> Otherwise, we should fix register_blkdev to preserve the old behavior.
>> (I guess the same thing applies to commit 8a932f73e5b "char_dev: order
>> /proc/devices by major number" as well).
>
> The restriction was put in place so the code that prints the devices doesn't
> have to run through every integer in order to print the devices in order.
>
> Given the existing documented fixed numbers in [1] and that future new char
> devices should be using dynamic allocation, this seemed like a reasonable
> restriction.
>
> It would be pretty trivial to increase the limit but, IMO, setting it to
> UINT_MAX seems a bit much. Especially given that a lot of the documentation
> and code still very much has remnants of the 256 limit. (The series that
> included this patch only just expanded the char dynamic range to above 256).
> So, I'd suggest the LTP test should change.
>
Sounds good! Thank you!
By the way, I happened to notice a few minor issues with the
find_dynamic_major() function added by this patch series:
static int find_dynamic_major(void)
{
int i;
struct char_device_struct *cd;
for (i = ARRAY_SIZE(chrdevs)-1; i > CHRDEV_MAJOR_DYN_END; i--) {
^^^^
As far as I can see, _DYN_END is inclusive, so shouldn't this be >= ?
if (chrdevs[i] == NULL)
return i;
}
for (i = CHRDEV_MAJOR_DYN_EXT_START;
i > CHRDEV_MAJOR_DYN_EXT_END; i--) {
^^^^
Same here; I believe this should be >=
for (cd = chrdevs[major_to_index(i)]; cd; cd = cd->next)
if (cd->major == i)
break;
if (cd == NULL || cd->major != i)
^^^^^^^^
It seems that this latter condition is unnecessary, as it will never be
true. (We'll reach here only if cd == NULL or cd->major == i).
return i;
}
return -EBUSY;
}
Regards,
Srivatsa
