[PATCH 2/2] drm/i915: Soften error messages in i915_gem_object_create_from_data()

[Chris Wilson]

On Tue, Jul 28, 2015 at 05:29:09PM +0100, Dave Gordon wrote:
> On 28/07/15 14:27, Chris Wilson wrote:
> >Since we already return -EFAULT to the user, emitting an error message
> >*and* WARN is overkill. If the caller is upset, they can do so, but for
> >the purposes of debugging we need only log the erroneous values.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc:: Alex Dai <yu.dai at intel.com>
> >Cc: Dave Gordon <david.s.gordon at intel.com>
> >Cc: Tom O'Rourke <Tom.O'Rourke at intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_gem.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index c1ded76a6eb4..2039798f4403 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -5243,8 +5243,8 @@ i915_gem_object_create_from_data(struct drm_device *dev,
> >
> >  	i915_gem_object_unpin_pages(obj);
> >
> >-	if (WARN_ON(bytes != size)) {
> >-		DRM_ERROR("Incomplete copy, wrote %zu of %zu", bytes, size);
> >+	if (bytes != size) {
> >+		DRM_DEBUG_GEM("Incomplete copy, wrote %zu of %zu", bytes, size);
> >  		ret = -EFAULT;
> >  		goto fail;
> >  	}
> 
> I agree the WARN_ON() is overkill, but I think maybe the DRM_ERROR()
> is useful. The (one current) caller will report an error, but at
> that level it's just:
> 
>         DRM_ERROR("Failed to fetch GuC firmware from %s\n", ...)
> 
> with no more detail as to whether that was due to file-not-found,
> bad-file-size, out-of-memory, failed-to-get-pages, or any of the
> other errors that might arise.
> 
> At present this code is only called once, and I think this copy
> failure "shouldn't ever happen", so it won't be filling the logfile.
> But emitting the error here for the truly unexpected case (as
> opposed to the commonplace "out-of-memory" and suchlike) helps
> current and future callers avoid doing the detailed failure
> analysis.

The message is still there though, it's just a debug message for the
developer. And all it could mean is that the caller passed in a bad
value for the size.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre