Re: [PATCH RFC v4 01/16] drm: Add drm_minor_for_each

[Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org>]

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@ffwll.ch> 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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx