svn commit: r335094 - head/sys/ofed/drivers/infiniband/core

[Bruce Evans]

On Thu, 14 Jun 2018, Hans Petter Selasky wrote:

> On 06/14/18 10:46, Bruce Evans wrote:
>> Are these macros for conversion of host (FreeBSD) dev_t's or target (Linux)
>> ones?  If for the host, then I don't see any reason not to use the host 
>> APIs.
>> If for the target, then they shouldn't be used with the host dev_t.  If 
>> for
>> a mixture, then the translations are very confusing, especially when they
>> are the identity, and many more macros are needed to reduce the confusion.
>
> These values are only used inside the LinuxKPI for creating character 
> devices. They have no exposure to user-space.
>
> Basically:
>
> Z = MKDEV(X,Y)
> MAJOR(Z) must be equal to X
> MINOR(Z) must be equal to Y

So they are only for the host.

You should try harder to allocate the device number properly (not using
hard-coded numbers which are not even #defined in a central place).
compat/linux already has similar hard-coding for pts and more, but that
is more central and historically established.

Unique device numbers now seems to be allocated by devfs_alloc_cdp_inode().
Or call make_dev*() to create a full device with a device number allocated
by this.

I now see bugs in this too.  Major numbers for devices are supposed
to be 0.  But devfs_alloc_cdp_inode() constructs a minor number which
is assigned to cdp->cdp_inode.  cdp_inode is treated as a device number,
not as a minor number.  It is assigned directly to a device number in
dev2udev() and for assignment to va_rdev.  So after 256 allocations,
the minor bits leak into the major bits.  Eventually the leak breaks
uniqueness of hard-coded Linux major numbers.

I saw this earlier to today but thought the problem was a kernel without
the encoding fixes combined with an old userland using old major(), etc.,
to decode new dev_t encoding.  Actually, the encodings were consistently
old except in devfs_alloc_cdp_inode().  I created 514 ptys in /dev/pts/
and noticed that the numbers change from (major = N, minor = 255) to
(major = N, minor = 0).  With the previous kernel encoding, the minor
numbers would have increased sequentially, but old userland would not
have noticed the difference since it users old major(), etc., to decode.s

If you create a device by name, then its name needs to be unique instead
of its numbers, but this is easier to arrange.

Bruce