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

[Dave Gordon <david.s.gordon@intel.com>]

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@chris-wilson.co.uk>
> Cc:: Alex Dai <yu.dai@intel.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tom O'Rourke <Tom.O'Rourke@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.

Or maybe it's a good place for your (other) new macro?

	DRM_NOTICE_ON(bytes != size,
		"Incomplete copy, wrote %zu of %zu", bytes, size);

.Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel