[Intel-xe] [PATCH v2] drm/xe: Use milliseconds instead of jiffies in uapi for user fence

[Thomas Hellström]

On 5/28/23 18:14, Zbigniew Kempczyński wrote:
> Using jiffies as a timeout from userspace is weird even it theoretically
> exists possiblity of acquiring jiffies via getconf. Unfortunately this
> method is unreliable and returned value may vary from configured in the
> kernel config.
>
> Due to kernel constraints regarding milliseconds type (unsigned int)
> conversion to jiffies and uapi timeout defined as u64 return an error
> if timeout exceeds unsigned int size.
>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Cc: Andi Shyti <andi.shyti at linux.intel.com>

I agree we must move away from jiffies. If you want to continue with 
milliseconds but are worried about the truncation to uint,
then you could possibly use nsecs_to_jiffies, I guess.

Acked-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>


>
> ---
> v2: Drop msecs_to_jiffies_timeout() helper (Andi)
> ---
>   drivers/gpu/drm/xe/xe_wait_user_fence.c | 6 ++++--
>   include/uapi/drm/xe_drm.h               | 2 +-
>   2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_wait_user_fence.c b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> index 6c8a60c60087..6473b7b1273d 100644
> --- a/drivers/gpu/drm/xe/xe_wait_user_fence.c
> +++ b/drivers/gpu/drm/xe/xe_wait_user_fence.c
> @@ -98,7 +98,7 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>   	int err;
>   	bool no_engines = args->flags & DRM_XE_UFENCE_WAIT_SOFT_OP ||
>   		args->flags & DRM_XE_UFENCE_WAIT_VM_ERROR;
> -	unsigned long timeout = args->timeout;
> +	unsigned long timeout;
>   
>   	if (XE_IOCTL_ERR(xe, args->extensions) || XE_IOCTL_ERR(xe, args->pad) ||
>   	    XE_IOCTL_ERR(xe, args->reserved[0] || args->reserved[1]))
> @@ -149,9 +149,11 @@ int xe_wait_user_fence_ioctl(struct drm_device *dev, void *data,
>   		addr = vm->async_ops.error_capture.addr;
>   	}
>   
> -	if (XE_IOCTL_ERR(xe, timeout > MAX_SCHEDULE_TIMEOUT))
> +	if (XE_IOCTL_ERR(xe, args->timeout > UINT_MAX))
>   		return -EINVAL;
>   
> +	timeout = msecs_to_jiffies(args->timeout);
> +
>   	/*
>   	 * FIXME: Very simple implementation at the moment, single wait queue
>   	 * for everything. Could be optimized to have a wait queue for every
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index d5fc54b5be74..f7f43118d156 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -752,7 +752,7 @@ struct drm_xe_wait_user_fence {
>   #define DRM_XE_UFENCE_WAIT_U32		0xffffffffu
>   #define DRM_XE_UFENCE_WAIT_U64		0xffffffffffffffffu
>   	__u64 mask;
> -	/** @timeout: how long to wait before bailing, value in jiffies */
> +	/** @timeout: how long to wait before bailing, value in milliseconds */
>   	__s64 timeout;
>   	/**
>   	 * @num_engines: number of engine instances to wait on, must be zero