[RFC PATCH v3 02/11] cgroup: Add mechanism to register DRM devices

[Kenny Ho]

(sending again, I keep missing the reply-all in gmail.)

On Wed, Jun 26, 2019 at 11:56 AM Daniel Vetter <daniel at ffwll.ch> wrote:
>
> Why the separate, explicit registration step? I think a simpler design for
> drivers would be that we set up cgroups if there's anything to be
> controlled, and then for GEM drivers the basic GEM stuff would be set up
> automically (there's really no reason not to I think).

Is this what you mean with the comment about drm_dev_register below?
I think I understand what you are saying but not super clear.  Are you
suggesting the use of driver feature bits (drm_core_check_feature,
etc.) similar to the way Brian Welty did in his proposal in May?

> Also tying to the minor is a bit funky, since we have multiple of these.
> Need to make sure were at least consistent with whether we use the primary
> or render minor - I'd always go with the primary one like you do here.

Um... come to think of it, I can probably embed struct drmcgrp_device
into drm_device and that way I don't really need to keep a separate
array of
known_drmcgrp_devs and get rid of that max_minor thing.  Not sure why
I didn't think of this before.

> > +
> > +int drmcgrp_register_device(struct drm_device *dev)
>
> Imo this should be done as part of drm_dev_register (maybe only if the
> driver has set up a controller or something). Definitely with the
> unregister logic below. Also anything used by drivers needs kerneldoc.
>
>
> > +     /* init cgroups created before registration (i.e. root cgroup) */
> > +     if (root_drmcgrp != NULL) {
> > +             struct cgroup_subsys_state *pos;
> > +             struct drmcgrp *child;
> > +
> > +             rcu_read_lock();
> > +             css_for_each_descendant_pre(pos, &root_drmcgrp->css) {
> > +                     child = css_drmcgrp(pos);
> > +                     init_drmcgrp(child, dev);
> > +             }
> > +             rcu_read_unlock();
>
> I have no idea, but is this guaranteed to get them all?

I believe so, base on my understanding about
css_for_each_descendant_pre and how I am starting from the root
cgroup.  Hopefully I didn't miss anything.

Regards,
Kenny