On Thu, Mar 08, 2018 at 01:11:33PM +0000, Chris Wilson wrote: > Quoting Chris Wilson (2018-03-08 11:32:04) > > Quoting Matt Roper (2018-03-06 23:46:59) > > > There are cases where a system integrator may wish to raise/lower the > > > priority of GPU workloads being submitted by specific OS process(es), > > > independently of how the software self-classifies its own priority. > > > Exposing "priority offset" as an i915-specific cgroup parameter will > > > enable such system-level configuration. > > > > > > Normally GPU contexts start with a priority value of 0 > > > (I915_CONTEXT_DEFAULT_PRIORITY) and then may be adjusted up/down from > > > there via other mechanisms. We'd like to provide a system-level input > > > to the priority decision that will be taken into consideration, even > > > when userspace later attempts to set an absolute priority value via > > > I915_CONTEXT_PARAM_PRIORITY. The priority offset introduced here > > > provides a base value that will always be added to (or subtracted from) > > > the software's self-assigned priority value. > > > > > > This patch makes priority offset a cgroup-specific value; contexts will > > > be created with a priority offset based on the cgroup membership of the > > > process creating the context at the time the context is created. Note > > > that priority offset is assigned at context creation time; migrating a > > > process to a different cgroup or changing the offset associated with a > > > cgroup will only affect new context creation and will not alter the > > > behavior of existing contexts previously created by the process. > > > > > > v2: > > > - Rebase onto new cgroup_priv API > > > - Use current instead of drm_file->pid to determine which process to > > > lookup priority for. (Chris) > > > - Don't forget to subtract priority offset in context_getparam ioctl to > > > make it match setparam behavior. (Chris) > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > > > > For ctx->priority/ctx->priority_offset > > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > As a reminder, we do have the question of how to bound ctx->priority + > ctx->priority_offset. Currently, outside of the user range is privileged > space reserved for the kernel (both above and below). Now we can move > those even further to accommodate multiple non-overlapping cgroups. Yep, I mentioned this as an open question in the series coverletter. My understanding is that today we limit userspace-assigned priorities to [1023,1023] and that the kernel can use the userspace range plus -1024 (for the kernel idle context), 1024 (for boosting contexts the display is waiting on), and INT_MAX (for the preempt context). Are there any other special values we use today that I'm overlooking? I think we have the following options with how to proceed: * Option 1: Silently limit (priority+priority offset) to the existing [-1023,1023] range. That means that if, for example, a user context had a priority offset of 600 and tried to manually boost its context priority to 600, it would simply be treated as a 1023 for scheduling purposes. This approach is simple, but doesn't allow us to force contexts in different cgroups into non-overlapping priority ranges (i.e., lowest possible priority in one cgroup is still higher than the highest possible priority in another cgroup). It also isn't possible to define a class of applications as "more important than display" via cgroup, which I think might be useful in some cases. * Option 2: Allow priority offset to be set in a much larger range (e.g., [SHRT_MIN+1023,SHRT_MAX-1023]). This allows cgroups to have effective priority ranges that don't overlap. cgroup ranges can also be intentionally set above I915_PRIORITY_DISPLAY to allow us to define classes of applications whose work is more important than maintaining a stable framerate on the display. We'd also probably need to shift the kernel idle context down to something like INT_MIN instead of using -1024. * Option 3: No limits, leave behavior as it stands now in this patch series (unrestricted range). If you're privileged enough to define the cgroup settings for a system and you decide to shoot yourself in the foot by setting them to nonsense values, that's your own fault. Thoughts? Any other options to consider? > We also have the presumption that only privileged processes can raise a > priority, but I presume the cgroup property itself is protected. > -Chris The way the code is written write now, anyone who has write access to the cgroup itself (i.e., can migrate processes into/out of the cgroup) is allowed to adjust the i915-specific cgroup settings. So this depends on how the cgroups-v2 filesystem was initially mounted and/or how the filesystem permissions were adjusted after the fact; the details may vary from system to system depending on the policy decisions of the system integrator / system administrator. But I think an off-the-shelf Linux distro will only give the system administrator sufficient permissions to adjust cgroups (if it even mounts the cgroups-v2 filesystem in the first place), so anyone else that winds up with those privileges has had them intentionally delegated. Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
