All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: Alexei Starovoitov <ast@fb.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	Tejun Heo <tj@kernel.org>,
	cgroups@vger.kernel.org, Roman Gushchin <guro@fb.com>
Subject: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration
Date: Mon, 26 Mar 2018 10:30:23 +0300	[thread overview]
Message-ID: <152204942372.6045.18152790780379161422@jlahtine-desk.ger.corp.intel.com> (raw)
In-Reply-To: <20180323154615.GD24674@mdroper-desk.amr.corp.intel.com>

Quoting Matt Roper (2018-03-23 17:46:16)
> On Fri, Mar 23, 2018 at 02:15:38PM +0200, Joonas Lahtinen wrote:
> > Quoting Matt Roper (2018-03-17 02:08:57)
> > > This is the fourth iteration of the work previously posted here:
> > >   (v1) https://lists.freedesktop.org/archives/intel-gfx/2018-January/153156.html
> > >   (v2) https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg208170.html
> > >   (v3) https://lists.freedesktop.org/archives/intel-gfx/2018-March/157928.html
> > > 
> > > The high level goal of this work is to allow non-cgroup-controller parts
> > > of the kernel (e.g., device drivers) to register their own private
> > > policy data for specific cgroups.  That mechanism is then made use of in
> > > the i915 graphics driver to allow GPU priority to be assigned according
> > > to the cgroup membership of the owning process.  Please see the v1 cover
> > > letter linked above for a more in-depth explanation and justification.
> > 
> > Hi Matt,
> > 
> > For cross-subsystem changes such as this, it makes sense to Cc all
> > relevant maintainers, especially if there have been previous comments to
> > earlier revisions.
> > 
> > Please, do include and keep a reference to the userspace portion of the
> > changes when you suggest new uAPI to be added. At least I have some trouble
> > trying to track down the relevant interface consumer here.
> > 
> > I'm unsure how much sense it makes to commence with detailed i915 review
> > if we will be blocked by lack of userspace after that? I'm assuming
> > you've read through [1] already.
> 
> Hi Joonas,
> 
> I've sent the userspace code out a few times, but it looks like I forgot
> to include a copy with v4/v4.5.  Here's the version I provided with v3:
>   https://lists.freedesktop.org/archives/intel-gfx/2018-March/157935.html

Thanks. Keeping that in the relevant commit message of the patch that
introduces the new uAPI will make it harder to forget and easiest for
git blame, too.

> 
> Usually we don't consider things like i-g-t to be sufficient userspace
> consumers because we need a real-world consumer rather than a "toy"
> userspace.  However in this case, the i-g-t tool, although very simple,
> is really the only userspace consumer I expect there to ever be.
> Ultimately the true consumer of this cgroups work are bash scripts, sysv
> init scripts, systemd recipes, etc.  that just need a very simple tool
> to assign the specific values that make sense on a given system.
> There's no expectation that graphics clients or display servers would
> ever need to make use of these interfaces.

I was under the impression that a bit more generic GPU cgroups support
was receiving a lot of support in the early discussion? A dedicated
intel_cgroup sounds underwhelming, when comparing to idea of "gpu_nice",
for user adoption :)

Also, I might not be up-to-date about all things cgroups, but the way
intel_cgroup works, feels bit forced. We create a userspace context just
to communicate with the driver and the IOCTL will still have global
effects. I can't but think that i915 reading from the cgroups subsystem
for the current process would feel more intuitive to me.

Does the implementation mimic some existing cgroups tool or de-facto way
of doing things in cgroups world?

Regards, Joonas
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2018-03-26  7:30 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-17  0:08 [PATCH v4 0/8] cgroup private data and DRM/i915 integration Matt Roper
2018-03-17  0:08 ` [PATCH v4 1/8] cgroup: Allow registration and lookup of cgroup private data (v2) Matt Roper
2018-03-19  5:41   ` [Intel-gfx] " kbuild test robot
2018-03-19  5:41   ` [RFC PATCH] cgroup: cgroup_idr_lock can be static kbuild test robot
2018-03-17  0:08 ` [PATCH v4 2/8] cgroup: Introduce task_get_dfl_cgroup() (v2) Matt Roper
2018-03-17  0:09 ` [PATCH v4 3/8] cgroup: Introduce cgroup_priv_get_current Matt Roper
2018-03-17  0:09 ` [PATCH v4 4/8] drm/i915: Adjust internal priority definitions Matt Roper
2018-03-17  0:09 ` [PATCH v4 5/8] drm/i915: cgroup integration (v3) Matt Roper
2018-03-17  0:09 ` [PATCH v4 6/8] drm/i915: Introduce 'priority offset' for GPU contexts (v3) Matt Roper
2018-03-17  0:09 ` [PATCH v4 7/8] drm/i915: Introduce per-cgroup display boost setting Matt Roper
2018-03-17  0:09 ` [PATCH v4 8/8] drm/i915: Add context priority & priority offset to debugfs (v2) Matt Roper
2018-03-17  0:16 ` [PATCH i-g-t] tests: Introduce drv_cgroup (v2) Matt Roper
2018-03-17  0:28 ` ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration Patchwork
2018-03-19  7:43   ` Jani Nikula
2018-03-17  0:45 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-17  1:04 ` ✓ Fi.CI.BAT: success for tests: Introduce drv_cgroup (v2) Patchwork
2018-03-23 12:15 ` [PATCH v4 0/8] cgroup private data and DRM/i915 integration Joonas Lahtinen
2018-03-23 15:46   ` Matt Roper
2018-03-26  7:30     ` Joonas Lahtinen [this message]
2018-03-30  0:43       ` Matt Roper
2018-04-05 13:46         ` DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration) Joonas Lahtinen
2018-04-05 14:15           ` Joonas Lahtinen
2018-04-05 14:49             ` Matt Roper
2018-04-05 15:06               ` Matt Roper
2018-04-05 15:48                 ` Matt Roper
2018-04-05 17:32                   ` Felix Kuehling
2018-04-05 17:32                     ` Felix Kuehling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=152204942372.6045.18152790780379161422@jlahtine-desk.ger.corp.intel.com \
    --to=joonas.lahtinen@linux.intel.com \
    --cc=ast@fb.com \
    --cc=cgroups@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=guro@fb.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.