Re: [PATCH 2/2] drm/i915: Fix the return value of pipe crc read function.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Aug 02, 2016 at 09:42:07PM -0700, Rodrigo Vivi wrote:
> A read(fd, buf, len) function should return the number
> of bytes read. In our case we need to return the
> number of bytes we copy to user, instead of returning
> the number of bytes we read internally.
> 
> It was really strange when I saw i-g-t test case using
> len '54' but getting '56' as return. First thought was
> how do we read more than we asked? But also I checked
> and there was really only 54. Until I realized it
> was all our fault. EFAULT!
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 7052c47..b7b8d79 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3659,7 +3659,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
>  
>  	spin_unlock_irq(&pipe_crc->lock);
>  
> -	return bytes_read;
> +	return PIPE_CRC_LINE_LEN;

Nope. We can read multiple entries in one go.

bytes_read += snprintf()

so it's mostly good. The only case where it fails if we don't have
enough space for the snprintf(), as snprintf() will then return the
number of bytes it would have written. That can actually happen on
account of the hex vs. decimal mess with the frame counter.

>  }
>  
>  static const struct file_operations i915_pipe_crc_fops = {
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx