Re: [RFC PATCH v3 01/11] cgroup: Introduce cgroup for drm subsystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 26, 2019 at 9:35 PM Kenny Ho <y2kenny@xxxxxxxxx> wrote:
>
> On Wed, Jun 26, 2019 at 11:49 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
> >
> > Bunch of naming bikesheds
>
> I appreciate the suggestions, naming is hard :).
>
> > > +#include <linux/cgroup.h>
> > > +
> > > +struct drmcgrp {
> >
> > drm_cgroup for more consistency how we usually call these things.
>
> I was hoping to keep the symbol short if possible.  I started with
> drmcg (following blkcg),  but I believe that causes confusion with
> other aspect of the drm subsystem.  I don't have too strong of an
> opinion on this but I'd prefer not needing to keep refactoring.  So if
> there are other opinions on this, please speak up.

I think drmcg sounds good to me. That aligns at least with memcg,
blkcg in cgroups, so as good reason as any. drmcgrp just felt kinda
awkward in-between solution, not as easy to read as drm_cgroup, but
also not as short as drmcg and cgrp is just letter jumbo I can never
remember anyway what it means :-)
-Daniel

> > > +
> > > +static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
> >
> > In drm we generally put _get/_put at the end, cgroup seems to do the same.
>
> ok, I will refactor.
>
> > > +{
> > > +     if (drmcgrp)
> > > +             css_put(&drmcgrp->css);
> > > +}
> > > +
> > > +static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
> >
> > I'd also call this drm_cgroup_parent or so.
> >
> > Also all the above needs a bit of nice kerneldoc for the final version.
> > -Daniel
>
> Noted, will do, thanks.
>
> Regards,
> Kenny



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux