Re: DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration)
Just realized Joonas had some other comments down at the bottom that I missed originally... On Thu, Apr 05, 2018 at 08:06:23AM -0700, Matt Roper wrote: > On Thu, Apr 05, 2018 at 07:49:44AM -0700, Matt Roper wrote: > > On Thu, Apr 05, 2018 at 05:15:13PM +0300, Joonas Lahtinen wrote: > > > + Some more Cc's based on IRC discussion > > > > > > Quoting Joonas Lahtinen (2018-04-05 16:46:51) > > > > + Dave for commenting from DRM subsystem perspective. I strongly believe > > > > there would be benefit from agreeing on some foundation of DRM subsystem > > > > level program GPU niceness [-20,19] and memory limit [0,N] pages. > > > > +Chris Wilson > > +more Cc's based on IRC discussion. > > > Matt > > > > > If we ignore backward compatibility and ABI issues for now and assume > > all drivers can move to [-20, 19] for application-accessible priority > > ranges (i.e., self opt-in via existing context parameter ioctls and > > such), I think we still need a much larger range for the priority offset > > assigned via cgroup. One of the goals with the cgroup work is to give > > the system integrator the ability to define classes of applications with > > non-overlapping ranges (i.e., in some systems an app in the "very > > important" cgroup that self-lowers itself as far as possible should > > still be prioritized higher than an app in the "best effort" cgroup that > > self-boosts itself as far as possible). Chris Wilson and I discussed > > that a bit on this thread if you want to see the context: > > > > https://lists.freedesktop.org/archives/intel-gfx/2018-March/158204.html > > > > That discussion was based on i915's current application-accessible range > > of [-1023,1023]. If we shift to a much smaller [-20,19] range for > > applications to use directly, then we might want to allow cgroup offset > > to be something like [-1000,1000] to ensure the system integrator has > > enough flexibility? > > > > Also note that "display boost" (i.e., a priority boost for contexts that > > are blocking a flip) may vary depending on system setup, so setting > > being able to define the display boost's effective priority via cgroup > > is important as well. > > > > > > Matt > > > > > > > > > > > > > > Quoting Matt Roper (2018-03-30 03:43:13) > > > > > On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > > > > > > 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 > > > >
Re: DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration)
On Thu, Apr 05, 2018 at 07:49:44AM -0700, Matt Roper wrote: > On Thu, Apr 05, 2018 at 05:15:13PM +0300, Joonas Lahtinen wrote: > > + Some more Cc's based on IRC discussion > > > > Quoting Joonas Lahtinen (2018-04-05 16:46:51) > > > + Dave for commenting from DRM subsystem perspective. I strongly believe > > > there would be benefit from agreeing on some foundation of DRM subsystem > > > level program GPU niceness [-20,19] and memory limit [0,N] pages. > > +Chris Wilson +more Cc's based on IRC discussion. Matt > > If we ignore backward compatibility and ABI issues for now and assume > all drivers can move to [-20, 19] for application-accessible priority > ranges (i.e., self opt-in via existing context parameter ioctls and > such), I think we still need a much larger range for the priority offset > assigned via cgroup. One of the goals with the cgroup work is to give > the system integrator the ability to define classes of applications with > non-overlapping ranges (i.e., in some systems an app in the "very > important" cgroup that self-lowers itself as far as possible should > still be prioritized higher than an app in the "best effort" cgroup that > self-boosts itself as far as possible). Chris Wilson and I discussed > that a bit on this thread if you want to see the context: > > https://lists.freedesktop.org/archives/intel-gfx/2018-March/158204.html > > That discussion was based on i915's current application-accessible range > of [-1023,1023]. If we shift to a much smaller [-20,19] range for > applications to use directly, then we might want to allow cgroup offset > to be something like [-1000,1000] to ensure the system integrator has > enough flexibility? > > Also note that "display boost" (i.e., a priority boost for contexts that > are blocking a flip) may vary depending on system setup, so setting > being able to define the display boost's effective priority via cgroup > is important as well. > > > Matt > > > > > > > > > Quoting Matt Roper (2018-03-30 03:43:13) > > > > On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > > > > > 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 > > > > > > simp
Re: DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration)
On Thu, Apr 05, 2018 at 05:15:13PM +0300, Joonas Lahtinen wrote: > + Some more Cc's based on IRC discussion > > Quoting Joonas Lahtinen (2018-04-05 16:46:51) > > + Dave for commenting from DRM subsystem perspective. I strongly believe > > there would be benefit from agreeing on some foundation of DRM subsystem > > level program GPU niceness [-20,19] and memory limit [0,N] pages. +Chris Wilson If we ignore backward compatibility and ABI issues for now and assume all drivers can move to [-20, 19] for application-accessible priority ranges (i.e., self opt-in via existing context parameter ioctls and such), I think we still need a much larger range for the priority offset assigned via cgroup. One of the goals with the cgroup work is to give the system integrator the ability to define classes of applications with non-overlapping ranges (i.e., in some systems an app in the "very important" cgroup that self-lowers itself as far as possible should still be prioritized higher than an app in the "best effort" cgroup that self-boosts itself as far as possible). Chris Wilson and I discussed that a bit on this thread if you want to see the context: https://lists.freedesktop.org/archives/intel-gfx/2018-March/158204.html That discussion was based on i915's current application-accessible range of [-1023,1023]. If we shift to a much smaller [-20,19] range for applications to use directly, then we might want to allow cgroup offset to be something like [-1000,1000] to ensure the system integrator has enough flexibility? Also note that "display boost" (i.e., a priority boost for contexts that are blocking a flip) may vary depending on system setup, so setting being able to define the display boost's effective priority via cgroup is important as well. Matt > > > > Quoting Matt Roper (2018-03-30 03:43:13) > > > On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > > > > 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 expe
Re: DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration)
+ Some more Cc's based on IRC discussion Quoting Joonas Lahtinen (2018-04-05 16:46:51) > + Dave for commenting from DRM subsystem perspective. I strongly believe > there would be benefit from agreeing on some foundation of DRM subsystem > level program GPU niceness [-20,19] and memory limit [0,N] pages. > > Quoting Matt Roper (2018-03-30 03:43:13) > > On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > > > 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 :) > > > > I'm open to moving the cgroup_priv registration/lookup to the DRM core > > if other drivers are interested in using this mechanism and if we can > > come to an agreement on a standard priority offset range to support, how > > display boost should work for all drivers, etc. There might be some > > challenges mapping a DRM-defined priority range down to a different > > range that makes sense for individual driver schedulers, especially > > since some drivers already expose a different priority scheme to > > userspace via other interfaces like i915 does with GEM context priority. > > > > So far I haven't really heard any interest outside the Intel camp, but > > hopefully other driver teams can speak up if they're for/against this. > > I don't want to try to artificially standardize this if other drivers > > want to go a different direction with priority/scheduling that's too > > different from the current Intel-driven design. > > I don't think there are that many directions to go about GPU context > priority, considering we have the EGL_IMG_context_priority extension, so > it'll only be about granularity of the scale. > > I would suggest to go with the nice like scale for easy user adoption, > then just apply that as the N most significant bits.
DRM cgroups integration (Was: Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration)
+ Dave for commenting from DRM subsystem perspective. I strongly believe there would be benefit from agreeing on some foundation of DRM subsystem level program GPU niceness [-20,19] and memory limit [0,N] pages. Quoting Matt Roper (2018-03-30 03:43:13) > On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > > 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 :) > > I'm open to moving the cgroup_priv registration/lookup to the DRM core > if other drivers are interested in using this mechanism and if we can > come to an agreement on a standard priority offset range to support, how > display boost should work for all drivers, etc. There might be some > challenges mapping a DRM-defined priority range down to a different > range that makes sense for individual driver schedulers, especially > since some drivers already expose a different priority scheme to > userspace via other interfaces like i915 does with GEM context priority. > > So far I haven't really heard any interest outside the Intel camp, but > hopefully other driver teams can speak up if they're for/against this. > I don't want to try to artificially standardize this if other drivers > want to go a different direction with priority/scheduling that's too > different from the current Intel-driven design. I don't think there are that many directions to go about GPU context priority, considering we have the EGL_IMG_context_priority extension, so it'll only be about granularity of the scale. I would suggest to go with the nice like scale for easy user adoption, then just apply that as the N most significant bits. The contexts could then of course further adjust their priority from what is set by the "gpu_nice" application with the remaining bits. I'm strongly feeling this should be a DRM level "gpu_nice". And the binding to cgroups should come through DRM core. If it doesn't, limiting the amount of memory used becomes awkward as
Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration
On Mon, Mar 26, 2018 at 10:30:23AM +0300, Joonas Lahtinen wrote: > 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 :) I'm open to moving the cgroup_priv registration/lookup to the DRM core if other drivers are interested in using this mechanism and if we can come to an agreement on a standard priority offset range to support, how display boost should work for all drivers, etc. There might be some challenges mapping a DRM-defined priority range down to a different range that makes sense for individual driver schedulers, especially since some drivers already expose a different priority scheme to userspace via other interfaces like i915 does with GEM context priority. So far I haven't really heard any interest outside the Intel camp, but hopefully other driver teams can speak up if they're for/against this. I don't want to try to artificially standardize this if other drivers want to go a different direction with priority/scheduling that's too different from the current Intel-driven design. > 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. I think you're referring to the earlier discussion about exposing priority directly via the cgroups filesystem? That would certainly be simpler from a userspace perspective, but it's not the direction that the cgroups maintainer wants to see things go. Adding files directly to the cgroups filesystem is supposed to be something that's reserved for official cgroups controllers. The GPU priority concept we're trying to add here doesn't align with the requirements for creating a controller, so the preferred approach is to create a custom interface (syscall or ioctl) that simply takes a cgroup as a parameter. There's precendent with similar interfaces in areas like BPF (where the bpf() system call can accept a cgroup as a
Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration
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
Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration
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 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. Matt > > Regards, Joonas > > [1] > https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/8] cgroup private data and DRM/i915 integration
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. Regards, Joonas [1] https://01.org/linuxgraphics/gfx-docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 0/8] cgroup private data and DRM/i915 integration
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. v4 of this series brings large changes to the cgroup core half of the series and moderate changes to the i915 side. cgroup core changes: * The cgroup private data interface and implementation has changed again. The internal implementation is ida+radixtree based to allow much faster lookups than the previous hashtable approach. Private data is now registered and looked up via kref, which should allow the same private data to be set on multiple cgroups at the same time. The interface now looks like: - cgroup_priv_getkey() Obtain an integer key that will be used to store/lookup cgroup private data. This only needs to be called once at startup, driver init, etc.; after that the same key is used to store private data against any cgroup - cgroup_priv_destroykey(key) Release a private data key and drop references to any private data associated with the key on all cgroups. This function is very heavy, but will generally only be called by module-based users of the interface when the module is unloaded. - cgroup_priv_install(cgrp, key, ref) Store private data for a cgroup under the given key and increment the reference count. - cgroup_priv_get(cgrp, key) Take and return a reference to a cgroup's private data associated with the given key. - cgroup_priv_get_current(key) Same as cgroup_priv_get, but operates on the current task's cgroup. - cgroup_priv_release(cgrp, key) Remove private data from a cgroup and decrement its reference count. * Dropped the cgroup_permission() function. My i915 usage of this functionality will take a different approach for determining access control. i915 cgroup-based priority changes: * cgroup priority offset is now bounded such that (context+cgroup) adjustments fall within the range [-0x7f,0x7f]. This only takes 24 bits, leaving several effective priority bits for future flags or bookkeeping. * Display boost is added as a second cgroup parameter; each cgroup's processes can get differing boosts if a display operation is waiting on their completion. If we have non-overlapping cgroup priority ranges, this allows a system administrator to decide whether workloads from the lower priority cgroup(s) should still jump past the workloads in some/all of the higher priority cgroups. * Access control for cgroup settings has been changed. Instead of following cgroup filesystem permissions, we now restrict the access to either the DRM master or capable(CAP_SYS_RESOURCE). Cc: Tejun Heo Cc: Alexei Starovoitov Cc: Roman Gushchin Cc: Chris Wilson Matt Roper (8): cgroup: Allow registration and lookup of cgroup private data (v2) cgroup: Introduce task_get_dfl_cgroup() (v2) cgroup: Introduce cgroup_priv_get_current drm/i915: Adjust internal priority definitions drm/i915: cgroup integration (v3) drm/i915: Introduce 'priority offset' for GPU contexts (v3) drm/i915: Introduce per-cgroup display boost setting drm/i915: Add context priority & priority offset to debugfs (v2) drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_cgroup.c | 205 +++ drivers/gpu/drm/i915/i915_debugfs.c | 3 + drivers/gpu/drm/i915/i915_drv.c | 7 ++ drivers/gpu/drm/i915/i915_drv.h | 33 - drivers/gpu/drm/i915/i915_gem_context.c | 11 +- drivers/gpu/drm/i915/i915_gem_context.h | 9 ++ drivers/gpu/drm/i915/i915_request.h | 18 ++- drivers/gpu/drm/i915/intel_display.c| 5 +- include/linux/cgroup-defs.h | 8 ++ include/linux/cgroup.h | 37 ++ include/uapi/drm/i915_drm.h | 14 +++ kernel/cgroup/cgroup.c | 208 +++- 13 files changed, 545 insertions(+), 14 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://list
