| Message ID | 20190322234118.65980-2-carlos.santa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | GEN8+ GPU Watchdog Reset Support | expand |
Quoting Carlos Santa (2019-03-22 23:41:14) > From: Michel Thierry <michel.thierry@intel.com> > > Users/tests relying on the total reset count will start seeing a smaller > number since most of the hangs can be handled by engine reset. > Note that if reset engine x, context a running on engine y will be unaware > and unaffected. > > To start the discussion, include just a total engine reset count. If it > is deemed useful, it can be extended to report each engine separately. > > Our igt's gem_reset_stats test will need changes to ignore the pad field, > since it can now return reset_engine_count. > > v2: s/engine_reset/reset_engine/, use union in uapi to not break compatibility. > v3: Keep rejecting attempts to use pad as input (Antonio) > v4: Rebased. > v5: Rebased. > Get rid of the union to store pad/engine count (Chris) > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Antonio Argenziano <antonio.argenziano@intel.com> > Cc: Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > Signed-off-by: Carlos Santa <carlos.santa@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 12 ++++++++++-- > include/uapi/drm/i915_drm.h | 4 ++++ > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 21208a865380..9625b5f7faf7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -1350,6 +1350,8 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_i915_reset_stats *args = data; > struct i915_gem_context *ctx; > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > int ret; > > if (args->flags || args->pad) > @@ -1368,10 +1370,16 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, > * we should wrap the hangstats with a seqlock. > */ > > - if (capable(CAP_SYS_ADMIN)) > + if (capable(CAP_SYS_ADMIN)) { > args->reset_count = i915_reset_count(&dev_priv->gpu_error); > - else > + for_each_engine(engine, dev_priv, id) > + args->reset_engine_count += > + i915_reset_engine_count(&dev_priv->gpu_error, > + engine); Do we really care about device-vs-engine here? Amalgamating all engine resets into one variable is barely any more information than amalgamating them with the device count. > + } else { > args->reset_count = 0; > + args->reset_engine_count = 0; > + } > > args->batch_active = atomic_read(&ctx->guilty_count); > args->batch_pending = atomic_read(&ctx->active_count); > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index aa2d4c73a97d..5e7bc6412880 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -1459,6 +1459,9 @@ struct drm_i915_reset_stats { > /* All resets since boot/module reload, for all contexts */ > __u32 reset_count; > > + /* Engine resets since boot/module reload, for all contexts */ > + __u32 reset_engine_count; You cannot insert elements into the middle of an uABI struct. You can only extend. To avoid problems with sw layout, always introduce a new struct. > + > /* Number of batches lost when active in GPU, for this context */ > __u32 batch_active; > > @@ -1466,6 +1469,7 @@ struct drm_i915_reset_stats { > __u32 batch_pending; > > __u32 pad; > + > }; > > struct drm_i915_gem_userptr { > -- > 2.17.1 >
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 21208a865380..9625b5f7faf7 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -1350,6 +1350,8 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, struct drm_i915_private *dev_priv = to_i915(dev); struct drm_i915_reset_stats *args = data; struct i915_gem_context *ctx; + struct intel_engine_cs *engine; + enum intel_engine_id id; int ret; if (args->flags || args->pad) @@ -1368,10 +1370,16 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, * we should wrap the hangstats with a seqlock. */ - if (capable(CAP_SYS_ADMIN)) + if (capable(CAP_SYS_ADMIN)) { args->reset_count = i915_reset_count(&dev_priv->gpu_error); - else + for_each_engine(engine, dev_priv, id) + args->reset_engine_count += + i915_reset_engine_count(&dev_priv->gpu_error, + engine); + } else { args->reset_count = 0; + args->reset_engine_count = 0; + } args->batch_active = atomic_read(&ctx->guilty_count); args->batch_pending = atomic_read(&ctx->active_count); diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index aa2d4c73a97d..5e7bc6412880 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1459,6 +1459,9 @@ struct drm_i915_reset_stats { /* All resets since boot/module reload, for all contexts */ __u32 reset_count; + /* Engine resets since boot/module reload, for all contexts */ + __u32 reset_engine_count; + /* Number of batches lost when active in GPU, for this context */ __u32 batch_active; @@ -1466,6 +1469,7 @@ struct drm_i915_reset_stats { __u32 batch_pending; __u32 pad; + }; struct drm_i915_gem_userptr {