[RFC 0/1] drm/i915: Add scheduling priority to per-context parameters

[RFC 0/1] drm/i915: Add scheduling priority to per-context parameters

[Dave Gordon]

A proposal for an ioctl to allow a user process to alter its GPU
scheduling priority. It's defined as a new per-context parameter,
and each execbuffer submitted thereafter inherits its priority
from the context. This allows a single process to associate
different priorities with different contexts, perhaps being used
for different purposes (e.g. updating the GUI vs. scanning SETI
data for alien transmissions).

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 1/1] drm/i915: Add scheduling priority to per-context parameters

[Dave Gordon]

The next use for the i915 get/set per-context parameters ioctl,
ahead of the introduction of the forthcoming GPU scheduler.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_context.c | 17 +++++++++++++++++
 include/uapi/drm/i915_drm.h             |  1 +
 3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 279e258..104b711 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -850,6 +850,33 @@ struct i915_ctx_hang_stats {
 	bool banned;
 };
 
+/*
+ * User-settable GFX scheduler priorities are on a scale of 1 (lowest
+ * priority) to 1023 (highest priority). The special value 0 means
+ * "let the system decide my priority automatically"; this is the
+ * default if the user process does not explicitly request a different
+ * priority. Any process may decrease its scheduling priority, but
+ * only a sufficiently-privileged process may increase it. However,
+ * it is always permissible to reset it to "system default", even if
+ * is currently lower than that. Thus, if the system-assigned default
+ * were, say, 256, a process could decrease it to 128, and then to 64.
+ * It could NOT then increase it to 128 again, but COULD request a
+ * priority of 0 -- which would actually reset it to 256, allowing
+ * the process to then request 128 again. (This avoids the issue with
+ * nice(2) priorities, namely that non-super-users can not increase
+ * scheduling priorities of their own processes, even if they were the
+ * ones that decreased the priorities in the first place).
+ */
+
+#define	i915_SCHED_PRIORITY_DEFAULT	0
+#define	i915_SCHED_PRIORITY_MIN		1
+#define	i915_SCHED_PRIORITY_MAX		1023
+
+struct i915_ctx_sched_info {
+	/* Scheduling priority */
+	unsigned long priority;
+};
+
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_HANDLE 0
 
@@ -881,6 +908,7 @@ struct intel_context {
 	int flags;
 	struct drm_i915_file_private *file_priv;
 	struct i915_ctx_hang_stats hang_stats;
+	struct i915_ctx_sched_info sched_info;
 	struct i915_hw_ppgtt *ppgtt;
 
 	/* Legacy ring buffer submission */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 74aa0c9..2728a0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -901,6 +901,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 	case I915_CONTEXT_PARAM_NO_ZEROMAP:
 		args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
 		break;
+	case I915_CONTEXT_PARAM_PRIORITY:
+		args->value = ctx->sched_info.priority;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -938,6 +941,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 		else
 			ctx->hang_stats.ban_period_seconds = args->value;
 		break;
+
 	case I915_CONTEXT_PARAM_NO_ZEROMAP:
 		if (args->size) {
 			ret = -EINVAL;
@@ -946,6 +950,19 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 			ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
 		}
 		break;
+
+	case I915_CONTEXT_PARAM_PRIORITY:
+		if (args->size)
+			ret = -EINVAL;
+		else if (args->value > I915_SCHED_PRIORITY_MAX)
+			ret = -EINVAL;
+		else if (args->value > ctx->sched_info.priority &&
+			 !capable(CAP_SYS_ADMIN))
+			ret = -EPERM;
+		else
+			ctx->sched_info.priority = args->value;
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd5aa47..a860263 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1126,6 +1126,7 @@ struct drm_i915_gem_context_param {
 	__u64 param;
 #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
 #define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
+#define I915_CONTEXT_PARAM_PRIORITY   0x3
 	__u64 value;
 };
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/1] drm/i915: Add scheduling priority to per-context parameters

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

Hi Dave,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: x86_64-allyesconfig (attached as .config)
reproduce:
        git checkout 5be87551bfc8fabcb9cad4762a45c0f12d0c6cdf
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_gem_context.c: In function 'i915_gem_context_setparam_ioctl':
>> drivers/gpu/drm/i915/i915_gem_context.c:950:26: error: 'I915_SCHED_PRIORITY_MAX' undeclared (first use in this function)
      else if (args->value > I915_SCHED_PRIORITY_MAX)
                             ^
   drivers/gpu/drm/i915/i915_gem_context.c:950:26: note: each undeclared identifier is reported only once for each function it appears in

vim +/I915_SCHED_PRIORITY_MAX +950 drivers/gpu/drm/i915/i915_gem_context.c

   944			}
   945			break;
   946	
   947		case I915_CONTEXT_PARAM_PRIORITY:
   948			if (args->size)
   949				ret = -EINVAL;
 > 950			else if (args->value > I915_SCHED_PRIORITY_MAX)
   951				ret = -EINVAL;
   952			else if (args->value > ctx->sched_info.priority &&
   953				 !capable(CAP_SYS_ADMIN))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 50361 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 1/1 v2] drm/i915: Add scheduling priority to per-context parameters

[Dave Gordon]

Hmmm ... the email seems to have been damaged during composition :(
I probably shouldn't try to use vi(1) [where '~' means 
toggle-letter-case] over an ssh link [where '~' is an escape, of sorts] 
from another Linux machine inside a PuTTY terminal under Windows [where 
various keys send escape sequences containing '~'] :( Anyway, this
version has the #defines as they actually appeared in the source,
i.e. starting with UPPERCASE 'I' and not lowercase 'i'!

The next use for the i915 get/set per-context parameters ioctl,
ahead of the introduction of the forthcoming GPU scheduler.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
  drivers/gpu/drm/i915/i915_drv.h         | 28 ++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/i915_gem_context.c | 17 +++++++++++++++++
  include/uapi/drm/i915_drm.h             |  1 +
  3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h 
b/drivers/gpu/drm/i915/i915_drv.h
index 279e258..104b711 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -850,6 +850,33 @@ struct i915_ctx_hang_stats {
  	bool banned;
  };

+/*
+ * User-settable GFX scheduler priorities are on a scale of 1 (lowest
+ * priority) to 1023 (highest priority). The special value 0 means
+ * "let the system decide my priority automatically"; this is the
+ * default if the user process does not explicitly request a different
+ * priority. Any process may decrease its scheduling priority, but
+ * only a sufficiently-privileged process may increase it. However,
+ * it is always permissible to reset it to "system default", even if
+ * is currently lower than that. Thus, if the system-assigned default
+ * were, say, 256, a process could decrease it to 128, and then to 64.
+ * It could NOT then increase it to 128 again, but COULD request a
+ * priority of 0 -- which would actually reset it to 256, allowing
+ * the process to then request 128 again. (This avoids the issue with
+ * nice(2) priorities, namely that non-super-users can not increase
+ * scheduling priorities of their own processes, even if they were the
+ * ones that decreased the priorities in the first place).
+ */
+
+#define	I915_SCHED_PRIORITY_DEFAULT	0
+#define	I915_SCHED_PRIORITY_MIN		1
+#define	I915_SCHED_PRIORITY_MAX		1023
+
+struct i915_ctx_sched_info {
+	/* Scheduling priority */
+	unsigned long priority;
+};
+
  /* This must match up with the value previously used for 
execbuf2.rsvd1. */
  #define DEFAULT_CONTEXT_HANDLE 0

@@ -881,6 +908,7 @@ struct intel_context {
  	int flags;
  	struct drm_i915_file_private *file_priv;
  	struct i915_ctx_hang_stats hang_stats;
+	struct i915_ctx_sched_info sched_info;
  	struct i915_hw_ppgtt *ppgtt;

  	/* Legacy ring buffer submission */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 74aa0c9..2728a0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -901,6 +901,9 @@ int i915_gem_context_getparam_ioctl(struct 
drm_device *dev, void *data,
  	case I915_CONTEXT_PARAM_NO_ZEROMAP:
  		args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
  		break;
+	case I915_CONTEXT_PARAM_PRIORITY:
+		args->value = ctx->sched_info.priority;
+		break;
  	default:
  		ret = -EINVAL;
  		break;
@@ -938,6 +941,7 @@ int i915_gem_context_setparam_ioctl(struct 
drm_device *dev, void *data,
  		else
  			ctx->hang_stats.ban_period_seconds = args->value;
  		break;
+
  	case I915_CONTEXT_PARAM_NO_ZEROMAP:
  		if (args->size) {
  			ret = -EINVAL;
@@ -946,6 +950,19 @@ int i915_gem_context_setparam_ioctl(struct 
drm_device *dev, void *data,
  			ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
  		}
  		break;
+
+	case I915_CONTEXT_PARAM_PRIORITY:
+		if (args->size)
+			ret = -EINVAL;
+		else if (args->value > I915_SCHED_PRIORITY_MAX)
+			ret = -EINVAL;
+		else if (args->value > ctx->sched_info.priority &&
+			 !capable(CAP_SYS_ADMIN))
+			ret = -EPERM;
+		else
+			ctx->sched_info.priority = args->value;
+		break;
+
  	default:
  		ret = -EINVAL;
  		break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fd5aa47..a860263 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1126,6 +1126,7 @@ struct drm_i915_gem_context_param {
  	__u64 param;
  #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
  #define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
+#define I915_CONTEXT_PARAM_PRIORITY   0x3
  	__u64 value;
  };

-- 
1.9.1



_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 1/1 v2] drm/i915: Add scheduling priority to per-context parameters

[Chris Wilson]

On Thu, Oct 01, 2015 at 04:56:26PM +0100, Dave Gordon wrote:
> Hmmm ... the email seems to have been damaged during composition :(
> I probably shouldn't try to use vi(1) [where '~' means
> toggle-letter-case] over an ssh link [where '~' is an escape, of
> sorts] from another Linux machine inside a PuTTY terminal under
> Windows [where various keys send escape sequences containing '~'] :(
> Anyway, this
> version has the #defines as they actually appeared in the source,
> i.e. starting with UPPERCASE 'I' and not lowercase 'i'!
> 
> The next use for the i915 get/set per-context parameters ioctl,
> ahead of the introduction of the forthcoming GPU scheduler.
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_context.c | 17 +++++++++++++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 279e258..104b711 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -850,6 +850,33 @@ struct i915_ctx_hang_stats {
>  	bool banned;
>  };
> 
> +/*
> + * User-settable GFX scheduler priorities are on a scale of 1 (lowest
> + * priority) to 1023 (highest priority). The special value 0 means
> + * "let the system decide my priority automatically"; this is the
> + * default if the user process does not explicitly request a different
> + * priority. Any process may decrease its scheduling priority, but
> + * only a sufficiently-privileged process may increase it. However,
> + * it is always permissible to reset it to "system default", even if
> + * is currently lower than that. Thus, if the system-assigned default
> + * were, say, 256, a process could decrease it to 128, and then to 64.
> + * It could NOT then increase it to 128 again, but COULD request a
> + * priority of 0 -- which would actually reset it to 256, allowing
> + * the process to then request 128 again. (This avoids the issue with
> + * nice(2) priorities, namely that non-super-users can not increase
> + * scheduling priorities of their own processes, even if they were the
> + * ones that decreased the priorities in the first place).

I would prefer not to couple it in such a way. Have a continuous range
-1024,1024 (default 0), but only allow a privileged process to request
a positive priority value. So any process can set any negative/zero value
at any time (thereby getting a small boost in priority at times), but only
the select few can completely gazzump them by setting a positive value.

That is a little more intuitive from my perspective. Have you considered
skipping the nice() step entirely and jump to a setpriority() model?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx