Re: [PATCH 4/7] sync_file: add support for sem_file

[Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>]

On Thu, Apr 13, 2017 at 11:41:41AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> This adds support for a file that has semaphore semantics
> (for Vulkan shared semaphores).
> 
> These objects are persistent objects that can have a
> fence that changes. When the object is signaled, a fence
> is attached to it, and when an object is waited on, the
> fence is removed. All interactions with these objects
> should be via command submission routines in the drm
> drivers. The sem_file is just for passing the sems between
> processes.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/dma-buf/sync_file.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/sync_file.h   |  16 +++++++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 2342d8b..a88d786 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -468,3 +468,104 @@ static const struct file_operations sync_file_fops = {
>  	.unlocked_ioctl = sync_file_ioctl,
>  	.compat_ioctl = sync_file_ioctl,
>  };
> +
> +static int sem_file_release(struct inode *inode, struct file *file)
> +{
> +	struct sem_file *sem_file = file->private_data;
> +	struct dma_fence *fence;
> +
> +	fence = rcu_dereference_protected(sem_file->base.fence, 1);
> +	dma_fence_put(fence);
> +	kfree(sem_file);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations sem_file_fops = {
> +	.release = sem_file_release,
> +};
> +
> +struct sem_file *sem_file_alloc(void)
> +{
> +	struct sem_file *sem_file;
> +	int ret;
> +
> +	sem_file = kzalloc(sizeof(*sem_file), GFP_KERNEL);
> +	if (!sem_file)
> +		return NULL;
> +
> +	ret = fence_file_init(&sem_file->base,
> +			      &sem_file_fops);
> +	if (ret)
> +		goto err;
> +
> +	RCU_INIT_POINTER(sem_file->base.fence, NULL);
> +	mutex_init(&sem_file->lock);
> +
> +	return sem_file;
> +
> +err:
> +	kfree(sem_file);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(sem_file_alloc);
> +
> +struct sem_file *sem_file_fdget(int fd)
> +{
> +	struct file *file = fget(fd);
> +
> +	if (!file)
> +		return NULL;
> +
> +	if (file->f_op != &sem_file_fops)
> +		goto err;
> +
> +	return file->private_data;
> +
> +err:
> +	fput(file);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(sem_file_fdget);
> +
> +#define sem_file_held(obj) lockdep_is_held(&(obj)->lock)
> +
> +struct dma_fence *sem_file_get_fence(struct sem_file *sem_file)
> +{
> +	struct dma_fence *fence;
> +
> +	if (!rcu_access_pointer(sem_file->base.fence)) {
> +		return NULL;
> +	}
> +
> +	rcu_read_lock();
> +	fence = dma_fence_get_rcu_safe(&sem_file->base.fence);
> +	rcu_read_unlock();
> +	return fence;
> +}
> +EXPORT_SYMBOL(sem_file_get_fence);
> +
> +static inline struct dma_fence *
> +sem_file_get_fence_locked(struct sem_file *sem_file)
> +{
> +	return rcu_dereference_protected(sem_file->base.fence,
> +					 sem_file_held(sem_file));
> +}
> +
> +int sem_file_replace_fence(struct sem_file *sem_file,
> +			   struct dma_fence *fence,
> +			   struct dma_fence **old_fence)
> +{
> +	struct dma_fence *ret_fence = NULL;
> +
> +	if (fence)
> +		dma_fence_get(fence);
> +
> +	mutex_lock(&sem_file->lock);
> +	ret_fence = sem_file_get_fence_locked(sem_file);
> +	RCU_INIT_POINTER(sem_file->base.fence, fence);
> +	mutex_unlock(&sem_file->lock);

Is xchg() universal?

struct dma_fence *sem_file_replace_fence(struct sem_file *sem_file,
					 struct dma_fence *fence)
{
	return xchg(&sem_file->base.fence, dma_fence_get(fence));
}

safe against the rcu read and kills off the mutex.

I think this is the cleaner approach, precisely because it stops me
having the delusion that the semaphores and sync_file are
interchangeable, and I won't ask if I can merge semaphores together, or
if I can inspect the state with the CPU.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx