On Mon, Jun 12, 2017 at 05:34:59PM -0600, Logan Gunthorpe wrote:
1) The test we are looking at is overrunning the dynamic char dev major
numbers by a lot. On a successful run, it gets down to 216 where the
limit is 234. Thus, there are 18 too many major numbers assigned and the
fact that this test used to work is really just luck. (I've cc'd
developers from 49db08c3 for this issue.)
What test causes so many major numbers to be allocated? Is this
in-kernel test code? Do you really have a system that requires so many
different drivers that all want a dynamic char major?
thanks,
gre gk-h
What test causes so many major numbers to be allocated? Is this
in-kernel test code? Do you really have a system that requires so many
different drivers that all want a dynamic char major?
This is a 0day kernel robot test. I'm not sure the motivations of its
design but it seems to be similar to an allyesconfig. So all/most
modules are compiled in and allocating their char device regions on boot
of a qemu instance.
Logan
On Mon, Jun 12, 2017 at 10:29:20PM -0600, Logan Gunthorpe wrote:
On 12/06/17 10:18 PM, Greg Kroah-Hartman wrote:
quoted
What test causes so many major numbers to be allocated? Is this
in-kernel test code? Do you really have a system that requires so many
different drivers that all want a dynamic char major?
This is a 0day kernel robot test. I'm not sure the motivations of its
design but it seems to be similar to an allyesconfig. So all/most
modules are compiled in and allocating their char device regions on boot
of a qemu instance.
Ah, that makes sense. Well, someone can always work on expanding the
range of dynamic char major numbers if they are running out of them on a
real system, I'll gladly take patches for that :)
thanks,
greg k-h
On Tue, Jun 13, 2017 at 06:34:16AM +0200, Greg Kroah-Hartman wrote:
On Mon, Jun 12, 2017 at 10:29:20PM -0600, Logan Gunthorpe wrote:
quoted
On 12/06/17 10:18 PM, Greg Kroah-Hartman wrote:
quoted
What test causes so many major numbers to be allocated? Is this
in-kernel test code? Do you really have a system that requires so many
different drivers that all want a dynamic char major?
This is a 0day kernel robot test. I'm not sure the motivations of its
design but it seems to be similar to an allyesconfig. So all/most
modules are compiled in and allocating their char device regions on boot
of a qemu instance.
Ah, that makes sense. Well, someone can always work on expanding the
range of dynamic char major numbers if they are running out of them on a
real system, I'll gladly take patches for that :)
Or better yet, just turn all char major allocations into dynamic, which
would be really good for test systems. I thought someone proposed
patches for that a long time ago, but I can't find them anymore. That
would be the simplest solution here.
thnaks,
greg k-h
Or better yet, just turn all char major allocations into dynamic, which
would be really good for test systems. I thought someone proposed
patches for that a long time ago, but I can't find them anymore. That
would be the simplest solution here.
Would people not complain about that? I would not be surprised if some
crazy application is using hard coded major numbers in userspace. So
such a change could potentially break userspace...
Logan
On Tue, Jun 13, 2017 at 10:25:40AM -0600, Logan Gunthorpe wrote:
On 12/06/17 10:35 PM, Greg Kroah-Hartman wrote:
quoted
Or better yet, just turn all char major allocations into dynamic, which
would be really good for test systems. I thought someone proposed
patches for that a long time ago, but I can't find them anymore. That
would be the simplest solution here.
Would people not complain about that? I would not be surprised if some
crazy application is using hard coded major numbers in userspace. So
such a change could potentially break userspace...
For char devices, I doubt it, but we can't take the chance, which is why
you make it an option. Then, it's enabled for 'allmodconfig' builds,
which helps testers out.
thanks,
greg k-h
For char devices, I doubt it, but we can't take the chance, which is why
you make it an option. Then, it's enabled for 'allmodconfig' builds,
which helps testers out.
Well I took a look at this and it looks like a lot of work to modify all
the drivers to support a possible dynamic allocation and I'm not really
able to do all that right now.
However, correct me if I'm missing something, but it looks fairly
straightforward to extend the dynamic region above 256 in cases like
this. There are already fixed major numbers above 255 and the
infrastructure appears to support it. So what are your thoughts on the
change below? I'd be happy to clean it up into a proper patch if you
agree it's a workable option.
Thanks,
Logan
@@ -84,22 +107,11 @@ __register_chrdev_region(unsigned int major,
unsigned int baseminor,
mutex_lock(&chrdevs_lock);
- /* temporary */
if (major == 0) {
- for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
- if (chrdevs[i] == NULL)
- break;
- }
-
- if (i < CHRDEV_MAJOR_DYN_END)
- pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic
allocation range\n",
- name, i);
-
- if (i == 0) {
- ret = -EBUSY;
+ ret = find_dynamic_major();
+ if (ret < 0)
goto out;
- }
- major = i;
+ major = ret;
}
cd->major = major;
On Tue, Jun 13, 2017 at 10:25:40AM -0600, Logan Gunthorpe wrote:
quoted
On 12/06/17 10:35 PM, Greg Kroah-Hartman wrote:
quoted
Or better yet, just turn all char major allocations into dynamic, which
would be really good for test systems. I thought someone proposed
patches for that a long time ago, but I can't find them anymore. That
would be the simplest solution here.
Would people not complain about that? I would not be surprised if some
crazy application is using hard coded major numbers in userspace. So
such a change could potentially break userspace...
For char devices, I doubt it, but we can't take the chance, which is why
you make it an option. Then, it's enabled for 'allmodconfig' builds,
which helps testers out.
At least for /dev/null, /dev/zero, and perhaps /dev/tty it would
definitely break things if the major+minor number is not static. I have
multiple chroot environments having only some minimal needed static
/dev subdir, with naturally no daemons or filesystem creating those
on-demand. For the main /dev I use whatever the system sets up, so
devtmpfs with udev.
c'ya
sven-haegar
--
Three may keep a secret, if two of them are dead.
- Ben F.
On Tue, Jun 13, 2017 at 11:47:30AM -0600, Logan Gunthorpe wrote:
On 13/06/17 10:35 AM, Greg Kroah-Hartman wrote:
quoted
For char devices, I doubt it, but we can't take the chance, which is why
you make it an option. Then, it's enabled for 'allmodconfig' builds,
which helps testers out.
Well I took a look at this and it looks like a lot of work to modify all
the drivers to support a possible dynamic allocation and I'm not really
able to do all that right now.
No, don't modify any drivers, do this in the core chardev code.
quoted hunk
However, correct me if I'm missing something, but it looks fairly
straightforward to extend the dynamic region above 256 in cases like
this. There are already fixed major numbers above 255 and the
infrastructure appears to support it. So what are your thoughts on the
change below? I'd be happy to clean it up into a proper patch if you
agree it's a workable option.
Thanks,
Logan
@@ -84,22 +107,11 @@ __register_chrdev_region(unsigned int major,
unsigned int baseminor,
mutex_lock(&chrdevs_lock);
- /* temporary */
if (major == 0) {
- for (i = ARRAY_SIZE(chrdevs)-1; i > 0; i--) {
- if (chrdevs[i] == NULL)
- break;
- }
-
- if (i < CHRDEV_MAJOR_DYN_END)
- pr_warn("CHRDEV \"%s\" major number %d goes below the dynamic
allocation range\n",
- name, i);
-
- if (i == 0) {
- ret = -EBUSY;
+ ret = find_dynamic_major();
+ if (ret < 0)
goto out;
- }
- major = i;
+ major = ret;
}
cd->major = major;
On Tue, Jun 13, 2017 at 09:13:32PM +0200, Sven-Haegar Koch wrote:
On Tue, 13 Jun 2017, Greg Kroah-Hartman wrote:
quoted
On Tue, Jun 13, 2017 at 10:25:40AM -0600, Logan Gunthorpe wrote:
quoted
On 12/06/17 10:35 PM, Greg Kroah-Hartman wrote:
quoted
Or better yet, just turn all char major allocations into dynamic, which
would be really good for test systems. I thought someone proposed
patches for that a long time ago, but I can't find them anymore. That
would be the simplest solution here.
Would people not complain about that? I would not be surprised if some
crazy application is using hard coded major numbers in userspace. So
such a change could potentially break userspace...
For char devices, I doubt it, but we can't take the chance, which is why
you make it an option. Then, it's enabled for 'allmodconfig' builds,
which helps testers out.
At least for /dev/null, /dev/zero, and perhaps /dev/tty it would
definitely break things if the major+minor number is not static. I have
multiple chroot environments having only some minimal needed static
/dev subdir, with naturally no daemons or filesystem creating those
on-demand. For the main /dev I use whatever the system sets up, so
devtmpfs with udev.
No, it wouldn't be required, it would be an option for those people
using devtmpfs.
thanks,
greg k-h
No, don't modify any drivers, do this in the core chardev code.
Ok, well then maybe I misunderstood what you originally asked for
because I don't see how you can change a fixed allocation to a dynamic
one without touching the driver code which needs to know the major that
was assigned in the end.
At quick glance, looks good to me, care to clean it up and make it behind
a config option?
Sure. However, is a config option really necessary here? As is, the
extended dynamic region will only be used if there are too many chardev
majors and it shouldn't have _any_ effect on users that have a small
number. So it seems like not selecting that option is just telling the
kernel to be broken for no obvious trade-off.
Logan
On Tue, Jun 13, 2017 at 6:34 AM, Greg Kroah-Hartman
[off-list ref] wrote:
Ah, that makes sense. Well, someone can always work on expanding the
range of dynamic char major numbers if they are running out of them on a
real system, I'll gladly take patches for that :)
I started to take a stab at it at one point and incorporated some feedback
from Torvalds etc, it's here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/commit/?h=chrdev-warn&id=65e5b1e9eb3f777ab7535b74b490e882eeec79d7
It tries to use all "holes" in the chardev major map to shun in a bit
more devices when we run out of the high dynamic major range.
Making them all dynamic seemed dangerous because I was afraid
of userspace ABI breakage because of old userlands with
static mknod:s.
I lost interest when it turned out that the zeroday QEMU stuff was
generating random machines that have no counterpart in the real
world, and then the exercise seemed a bit academic.
The last failures were due to (AFAICT) some relationship between
major and minor numbers that I didn't untangle.
If there is interest I could try to revive it.
Yours,
Linus Walleij
Yes, thanks, I did find your work after a google search. However, seeing
it seems to recover only a handful of numbers with some significant
complexity required it seemed like a stopgap solution anyway. I posted a
proposal in this thread that simply extends the dynamic range into a
region above 256 which seems to work well. This means regular
configurations won't be affected an any configuration requiring more
than 20 dynamic majors will simply end up with high numbers.
So unless anyone can think of a reason why this won't work, it's my
preferred direction.
Making them all dynamic seemed dangerous because I was afraid
of userspace ABI breakage because of old userlands with
static mknod:s.