On Thu, Sep 05, 2019 at 05:26:08PM -0400, Kenny Ho wrote: > On Thu, Sep 5, 2019 at 4:32 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > *snip* > > drm_dev_unregister gets called on hotunplug, so your cgroup-internal > > tracking won't get out of sync any more than the drm_minor list gets > > out of sync with drm_devices. The trouble with drm_minor is just that > > cgroup doesn't track allocations on drm_minor (that's just the uapi > > flavour), but on the underlying drm_device. So really doesn't make > > much sense to attach cgroup tracking to the drm_minor. > > Um... I think I get what you are saying, but isn't this a matter of > the cgroup controller doing a drm_dev_get when using the drm_minor? > Or that won't work because it's possible to have a valid drm_minor but > invalid drm_device in it? I understand it's an extra level of > indirection but since the convention for addressing device in cgroup > is using $major:$minor I don't see a way to escape this. (Tejun > actually already made a comment on my earlier RFC where I didn't > follow the major:minor convention strictly.) drm_device is the object that controls lifetime and everything, that's why you need to do a drm_dev_get and all that in some places. Going through the minor really feels like a distraction. And yes we have a bit a mess between cgroups insisting on using the minor, and drm_device having more than 1 minor for the same underlying physical resource. Just because the uapi is a bit a mess in that regard doesn't mean we should pull that mess into the kernel implementation imo. -Daniel > > Kenny > > > > > Just doing a drm_cg_register/unregister pair that's called from > > > > drm_dev_register/unregister, and then if you want, looking up the > > > > right minor (I think always picking the render node makes sense for > > > > this, and skipping if there's no render node) would make most sense. > > > > At least for the basic cgroup controllers which are generic across > > > > drivers. > > > > > > Why do we want to skip drm devices that does not have a render node > > > and not just use the primary instead? > > > > I guess we could also take the primary node, but drivers with only > > primary node are generaly display-only drm drivers. Not sure we want > > cgroups on those (but I guess can't hurt, and more consistent). But > > then we'd always need to pick the primary node for cgroup > > identification purposes. > > -Daniel > > > > > > > > Kenny > > > > > > > > > > > > > -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
