Re: [PATCH RFC v2 6/7] drm/i915: Introduce 'priority offset' for GPU contexts

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

 



Quoting Matt Roper (2018-02-01 19:53:14)
> 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.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_cgroup.c      | 46 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h         |  7 +++++
>  drivers/gpu/drm/i915/i915_gem_context.c |  6 +++--
>  drivers/gpu/drm/i915/i915_gem_context.h |  9 +++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  5 files changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cgroup.c b/drivers/gpu/drm/i915/i915_cgroup.c
> index 778af895fc00..73f3cfe31d66 100644
> --- a/drivers/gpu/drm/i915/i915_cgroup.c
> +++ b/drivers/gpu/drm/i915/i915_cgroup.c
> @@ -11,6 +11,8 @@
>  
>  struct i915_cgroup_data {
>         struct cgroup_driver_data base;
> +
> +       int priority_offset;
>  };
>  
>  static inline struct i915_cgroup_data *
> @@ -72,6 +74,8 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>         struct cgroup *cgrp;
>         struct file *f;
>         struct inode *inode = NULL;
> +       struct cgroup_driver_data *cgrpdata;
> +       struct i915_cgroup_data *i915data;
>         int ret;
>  
>         if (!dev_priv->i915_cgroups) {
> @@ -125,6 +129,17 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>                 return ret;
>  
>         switch (req->param) {
> +       case I915_CGROUP_PARAM_PRIORITY_OFFSET:
> +               cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp,
> +                                                 NULL);

dev_priv is i915. So i915->i915_cgroups?

Could you just have a i915_get_cgroup_data() returning struct
i915_croup_data?


> +               if (IS_ERR(cgrpdata))
> +                       return PTR_ERR(cgrpdata);
> +
> +               DRM_DEBUG_DRIVER("Setting cgroup priority offset to %lld\n",
> +                                req->value);
> +               i915data = cgrp_to_i915(cgrpdata);
> +               i915data->priority_offset = req->value;

We'll have to consider what bounds we think are sensible, and how
internal policy interacts. Presumably another cgroup-param :)

> +               break;
>         default:
>                 DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
>                 return -EINVAL;
> @@ -132,3 +147,34 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>  
>         return 0;
>  }
> +
> +/**
> + * i915_cgroup_get_prio_offset() - get prio offset for current proc's cgroup
> + * @dev_priv: drm device
> + * @file_priv: file handle for calling process
> + *
> + * RETURNS:
> + * Priority offset associated with the calling process' cgroup in the default
> + * (v2) hierarchy, otherwise 0 if no explicit priority has been assigned.
> + */
> +int
> +i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                           struct drm_i915_file_private *file_priv)
> +{
> +       struct cgroup *cgrp;
> +       struct cgroup_driver_data *cgrpdata;
> +
> +       /* Ignore internally-created contexts not associated with a process */
> +       if (!file_priv)
> +               return 0;
> +
> +       cgrp = drm_file_get_cgroup(file_priv->file);
> +       if (WARN_ON(!cgrp))
> +               return 0;

Ah, this needs to pull from current then not drm_file->pid.

> +
> +       cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp, NULL);
> +       if (IS_ERR(cgrpdata))
> +              return 0;
> +
> +       return cgrp_to_i915(cgrpdata)->priority_offset;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 60e3ff6a48bb..313191e5278a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2920,6 +2920,8 @@ int i915_cgroup_init(struct drm_i915_private *dev_priv);
>  int i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>                                struct drm_file *file);
>  void i915_cgroup_shutdown(struct drm_i915_private *dev_priv);
> +int i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                               struct drm_i915_file_private *file_priv);
>  #else
>  static inline int
>  i915_cgroup_init(struct drm_i915_private *dev_priv)
> @@ -2933,6 +2935,11 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>  {
>         return -EINVAL;
>  }
> +static inline int
> +i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                           struct drm_i915_file_private *file_priv) {
> +       return 0;
> +}
>  #endif
>  
>  /* i915_drv.c */
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e7536ff51..700d7bc7cfc8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -274,7 +274,9 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>         kref_init(&ctx->ref);
>         list_add_tail(&ctx->link, &dev_priv->contexts.list);
>         ctx->i915 = dev_priv;
> -       ctx->priority = I915_PRIORITY_NORMAL;
> +       ctx->priority_offset =
> +               i915_cgroup_get_prio_offset(dev_priv, file_priv);
> +       ctx->priority = I915_PRIORITY_NORMAL + ctx->priority_offset;
>  
>         INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
>         INIT_LIST_HEAD(&ctx->handles_list);
> @@ -816,7 +818,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>                                  !capable(CAP_SYS_NICE))
>                                 ret = -EPERM;
>                         else
> -                               ctx->priority = priority;
> +                               ctx->priority = priority + ctx->priority_offset;

And subtract the offset in getparam. So user priority in == reported
priority out, and they are none the wiser that it is being adjusted
internally (out of their direct control).
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux