On Fri, May 05, 2017 at 08:43:36AM -0700, Daniele Ceraolo Spurio wrote:
>
>
> On 04/05/17 14:31, Chris Wilson wrote:
> >On Thu, May 04, 2017 at 09:26:35PM +0000, Srivatsa, Anusha wrote:
> >>>+void i915_guc_load_error_log_capture(struct drm_i915_private *i915) {
> >>>+ void *log, *buf;
> >>>+ struct i915_vma *vma = i915->guc.log.vma;
> >>>+
> >>>+ if (i915->gpu_error.guc_load_fail_log || !vma)
> >>>+ return;
> >>>+
> >>>+ /*
> >>>+ * the vma should be already pinned and mapped for log runtime
> >>>+ * management but let's play safe
> >>>+ */
> >>>+ log = i915_gem_object_pin_map(vma->obj, I915_MAP_WC);
> >>>+ if (IS_ERR(log)) {
> >>>+ DRM_ERROR("Failed to pin guc_log vma\n");
> >>>+ return;
> >>>+ }
> >>>+
> >>>+ buf = kzalloc(GUC_LOG_SIZE, GFP_KERNEL);
> >>>+ if (buf) {
> >>>+ memcpy(buf, log, GUC_LOG_SIZE);
> >>>+ i915->gpu_error.guc_load_fail_log = buf;
> >>>+ } else {
> >>>+ DRM_ERROR("Failed to copy guc log\n");
> >>>+ }
> >>>+
> >>>+ i915_gem_object_unpin_map(vma->obj);
> >
> >You are trading a swappable object for unswappable kernel memory. If you
> >want to have the guc log after guc is disabled, just keep the log object
> >around.
> >-Chris
> >
>
> I had considered that, but in the end I wasn't sure if that was
> acceptable in case we end up modifying the code to recycle the
> object for future load attempts, although that is very unlikely. I
> was however unconvinced myself of using kzalloc and that's mainly
> why this was an RFC :)
> I'll flip it to take an extra reference on the object.
Just make sure you unpin it. I would suggest killing the guc->log.vma
and taking the object and placing it somewhere else as you have for the
buf, but not i915->gpu_error as that doesn't fit (this isn't a runtime
error that we are tracking). i915->guc.last_load_log?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
