[PATCH 3/7] sync_file: split out fence_file base class from sync_file.

Christian König deathsimple at vodafone.de
Wed Apr 19 12:02:47 UTC 2017


Am 13.04.2017 um 03:41 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
>
> This just splits out a common base object to be shared
> between sync_file and sem_files.

I don't think I like that approach.

A good argument of using sync_file instead of self coding it was to be 
able to be able to use the resulting fd with the atomic mode setting IOCTLs.

That advantage will completely vanish if we introduce a new fd type.

Regards,
Christian.

> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>   drivers/dma-buf/sync_file.c  | 49 +++++++++++++++++++++++++++-----------------
>   drivers/gpu/drm/drm_atomic.c |  4 ++--
>   include/linux/sync_file.h    | 17 ++++++++++-----
>   3 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 9b7ad7e..2342d8b 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -28,17 +28,28 @@
>   
>   static const struct file_operations sync_file_fops;
>   
> +static int fence_file_init(struct fence_file *fence_file,
> +			   const struct file_operations *fops)
> +{
> +	fence_file->file = anon_inode_getfile("fence_file", fops,
> +					     fence_file, 0);
> +	if (IS_ERR(fence_file->file))
> +		return PTR_ERR(fence_file->file);
> +	return 0;
> +}
> +
>   static struct sync_file *sync_file_alloc(void)
>   {
>   	struct sync_file *sync_file;
> +	int ret;
>   
>   	sync_file = kzalloc(sizeof(*sync_file), GFP_KERNEL);
>   	if (!sync_file)
>   		return NULL;
>   
> -	sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops,
> -					     sync_file, 0);
> -	if (IS_ERR(sync_file->file))
> +	ret = fence_file_init(&sync_file->base,
> +			      &sync_file_fops);
> +	if (ret)
>   		goto err;
>   
>   	init_waitqueue_head(&sync_file->wq);
> @@ -67,7 +78,7 @@ static void fence_check_cb_func(struct dma_fence *f, struct dma_fence_cb *cb)
>    *
>    * Creates a sync_file containg @fence. This function acquires and additional
>    * reference of @fence for the newly-created &sync_file, if it succeeds. The
> - * sync_file can be released with fput(sync_file->file). Returns the
> + * sync_file can be released with fput(sync_file->base.file). Returns the
>    * sync_file or NULL in case of error.
>    */
>   struct sync_file *sync_file_create(struct dma_fence *fence)
> @@ -78,7 +89,7 @@ struct sync_file *sync_file_create(struct dma_fence *fence)
>   	if (!sync_file)
>   		return NULL;
>   
> -	RCU_INIT_POINTER(sync_file->fence, dma_fence_get(fence));
> +	RCU_INIT_POINTER(sync_file->base.fence, dma_fence_get(fence));
>   
>   	snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d",
>   		 fence->ops->get_driver_name(fence),
> @@ -122,8 +133,8 @@ struct dma_fence *sync_file_get_fence(int fd)
>   	if (!sync_file)
>   		return NULL;
>   
> -	fence = dma_fence_get(rcu_dereference_protected(sync_file->fence, 1));
> -	fput(sync_file->file);
> +	fence = dma_fence_get(rcu_dereference_protected(sync_file->base.fence, 1));
> +	fput(sync_file->base.file);
>   
>   	return fence;
>   }
> @@ -141,7 +152,7 @@ static int sync_file_set_fence(struct sync_file *sync_file,
>   	 * we own the reference of the dma_fence_array creation.
>   	 */
>   	if (num_fences == 1) {
> -		RCU_INIT_POINTER(sync_file->fence, fences[0]);
> +		RCU_INIT_POINTER(sync_file->base.fence, fences[0]);
>   		kfree(fences);
>   	} else {
>   		array = dma_fence_array_create(num_fences, fences,
> @@ -150,7 +161,7 @@ static int sync_file_set_fence(struct sync_file *sync_file,
>   		if (!array)
>   			return -ENOMEM;
>   
> -		RCU_INIT_POINTER(sync_file->fence, &array->base);
> +		RCU_INIT_POINTER(sync_file->base.fence, &array->base);
>   	}
>   
>   	return 0;
> @@ -159,7 +170,7 @@ static int sync_file_set_fence(struct sync_file *sync_file,
>   static struct dma_fence **get_fences(struct sync_file *sync_file,
>   				     int *num_fences)
>   {
> -	struct dma_fence *fence = rcu_dereference_protected(sync_file->fence, 1);
> +	struct dma_fence *fence = rcu_dereference_protected(sync_file->base.fence, 1);
>   	if (dma_fence_is_array(fence)) {
>   		struct dma_fence_array *array = to_dma_fence_array(fence);
>   
> @@ -168,7 +179,7 @@ static struct dma_fence **get_fences(struct sync_file *sync_file,
>   	}
>   
>   	*num_fences = 1;
> -	return &sync_file->fence;
> +	return &sync_file->base.fence;
>   }
>   
>   static void add_fence(struct dma_fence **fences,
> @@ -271,7 +282,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
>   	return sync_file;
>   
>   err:
> -	fput(sync_file->file);
> +	fput(sync_file->base.file);
>   	return NULL;
>   
>   }
> @@ -281,7 +292,7 @@ static int sync_file_release(struct inode *inode, struct file *file)
>   	struct sync_file *sync_file = file->private_data;
>   	struct dma_fence *fence;
>   
> -	fence = rcu_dereference_protected(sync_file->fence, 1);
> +	fence = rcu_dereference_protected(sync_file->base.fence, 1);
>   	if (test_bit(POLL_ENABLED, &fence->flags))
>   		dma_fence_remove_callback(fence, &sync_file->cb);
>   	dma_fence_put(fence);
> @@ -295,7 +306,7 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait)
>   	struct sync_file *sync_file = file->private_data;
>   	struct dma_fence *fence;
>   
> -	fence = rcu_dereference_protected(sync_file->fence, 1);
> +	fence = rcu_dereference_protected(sync_file->base.fence, 1);
>   
>   	poll_wait(file, &sync_file->wq, wait);
>   
> @@ -348,15 +359,15 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file,
>   		goto err_put_fence3;
>   	}
>   
> -	fd_install(fd, fence3->file);
> -	fput(fence2->file);
> +	fd_install(fd, fence3->base.file);
> +	fput(fence2->base.file);
>   	return 0;
>   
>   err_put_fence3:
> -	fput(fence3->file);
> +	fput(fence3->base.file);
>   
>   err_put_fence2:
> -	fput(fence2->file);
> +	fput(fence2->base.file);
>   
>   err_put_fd:
>   	put_unused_fd(fd);
> @@ -420,7 +431,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
>   
>   no_fences:
>   	strlcpy(info.name, sync_file->name, sizeof(info.name));
> -	info.status = dma_fence_is_signaled(rcu_dereference_protected(sync_file->fence, 1));
> +	info.status = dma_fence_is_signaled(rcu_dereference_protected(sync_file->base.fence, 1));
>   	info.num_fences = num_fences;
>   
>   	if (copy_to_user((void __user *)arg, &info, sizeof(info)))
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a567310..16053a0 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2012,7 +2012,7 @@ static void complete_crtc_signaling(struct drm_device *dev,
>   	if (install_fds) {
>   		for (i = 0; i < num_fences; i++)
>   			fd_install(fence_state[i].fd,
> -				   fence_state[i].sync_file->file);
> +				   fence_state[i].sync_file->base.file);
>   
>   		kfree(fence_state);
>   		return;
> @@ -2036,7 +2036,7 @@ static void complete_crtc_signaling(struct drm_device *dev,
>   
>   	for (i = 0; i < num_fences; i++) {
>   		if (fence_state[i].sync_file)
> -			fput(fence_state[i].sync_file->file);
> +			fput(fence_state[i].sync_file->base.file);
>   		if (fence_state[i].fd >= 0)
>   			put_unused_fd(fence_state[i].fd);
>   
> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
> index adbc0b9..b0ae1cf 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -21,24 +21,31 @@
>   #include <linux/dma-fence-array.h>
>   
>   /**
> - * struct sync_file - sync file to export to the userspace
> + * struct fence_file - basic file to fence mapping
>    * @file:		file representing this fence
> + * @fence:		fence with the fences in the fence_file
> + */
> +struct fence_file {
> +	struct file		*file;
> +	struct dma_fence	__rcu *fence;
> +};
> +
> +/**
> + * struct sync_file - sync file to export to the userspace
> + * @base:               base fence file
>    * @name:		name of sync_file.  Useful for debugging
>    * @sync_file_list:	membership in global file list
>    * @wq:			wait queue for fence signaling
> - * @fence:		fence with the fences in the sync_file
>    * @cb:			fence callback information
>    */
>   struct sync_file {
> -	struct file		*file;
> +	struct fence_file       base;
>   	char			name[32];
>   #ifdef CONFIG_DEBUG_FS
>   	struct list_head	sync_file_list;
>   #endif
>   
>   	wait_queue_head_t	wq;
> -
> -	struct dma_fence	__rcu *fence;
>   	struct dma_fence_cb cb;
>   };
>   




More information about the amd-gfx mailing list