Re: [PATCH 0/3] drm: Use full allocated minor range for DRM

["Michał Winiarski" <michal.winiarski@intel.com>]

On Fri, Aug 19, 2022 at 08:16:07AM +0000, Simon Ser wrote:
> (It seems like the list was dropped in my reply, sorry about that.
> Re-adding it now.)
> 
> On Thursday, August 18th, 2022 at 14:06, Michał Winiarski <michal.winiarski@intel.com> wrote:
> 
> > On Thu, Aug 18, 2022 at 07:39:13AM +0000, Simon Ser wrote:
> > 
> > > Hm, I'm a bit worried about the user-space implications of this… e.g. libdrm
> > > can check for the major/minor to find out the type of a node. Dropping CONTROL
> > > from the enum will break that.
> > 
> > Yeah, but that would only cause problems if there are more than 64 devices in
> > the system, and the user-space in question is smart enough to support that.
> > 
> > IIUC libdrm only looks for 16 devices:
> > https://gitlab.freedesktop.org/mesa/drm/-/blob/main/xf86drm.h#L47
> > 
> > I'm not very familiar with mesa codebase, but I think it has something similar:
> > https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c#L52
> > 
> > I expect other clients to also have something similar (loop over minors, 0-63
> > for primary, 128-191 for render).
> > 
> > So this shouldn't really cause a regression, it's just that "old" userspace
> > won't be able to use more devices (but it's also not able to use more devices
> > without this series).
> 
> Unfortunately I think there are more assumptions all over the place, see e.g.
> drmGetMinorType:
> https://gitlab.freedesktop.org/mesa/drm/-/blob/main/xf86drm.c#L986
> 
> Also I'm not very found of dropping DRM_NODE_CONTROL from the kernel enum --
> this results in DRM_NODE_RENDER=1 in the kernel but DRM_NODE_RENDER=2 in
> user-space which sounds pretty error-prone.
> 
> > I could go with 0-63 primary, 64-127 empty, 128-191 render, 192-255 primary,
> > 256-319 empty, (...)
> > But it just seems like a waste to burn 1/3 of minors.
> 
> Could potentially work I guess.
> 
> > Perhaps it would also be possible to go with:
> > 0-63 primary, 64-127 empty, 128-191 render, 192-512K continuous range
> > where we distribute minors first-come first-serve, without any link to type (so
> > usually we'd get continuous card192, renderD193, and so on)
> 
> We would need to re-design drmGetMinorType if we go down this path.

It needs to be changed either way.
Even if we keep reserving 1/3 of minors, drmGetMinorType is still broken (will
return -1, for minors > 191).

Let's respin this with dropping patch 1 and reserving control minors.

-Michał