From: Dave Airlie <airlied@xxxxxxxxxx> This isn't needed currently, but to reuse sync file for Vulkan permanent shared semaphore semantics, we need to be able to swap the fence backing a sync file. This patch adds a mutex to the sync file and uses to protect accesses to the fence and cb members. v1.1: fix the locking (Julia Lawall). v2: use rcu try one Signed-off-by: Dave Airlie <airlied at redhat.com> --- drivers/dma-buf/sync_file.c | 82 +++++++++++++++++++++++++++++++++++---------- include/linux/sync_file.h | 5 ++- 2 files changed, 69 insertions(+), 18 deletions(-) diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 2321035..8b34f21 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -28,6 +28,10 @@ static const struct file_operations sync_file_fops; +#define sync_file_held(obj) lockdep_is_held(&(obj)->lock) +#define sync_file_assert_held(obj) \ + lockdep_assert_held(&(obj)->lock) + static struct sync_file *sync_file_alloc(void) { struct sync_file *sync_file; @@ -47,6 +51,9 @@ static struct sync_file *sync_file_alloc(void) INIT_LIST_HEAD(&sync_file->cb.node); + RCU_INIT_POINTER(sync_file->fence, NULL); + + mutex_init(&sync_file->lock); return sync_file; err: @@ -80,7 +87,9 @@ struct sync_file *sync_file_create(struct dma_fence *fence) if (!sync_file) return NULL; - sync_file->fence = dma_fence_get(fence); + dma_fence_get(fence); + + RCU_INIT_POINTER(sync_file->fence, fence); snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d", fence->ops->get_driver_name(fence), @@ -124,13 +133,26 @@ struct dma_fence *sync_file_get_fence(int fd) if (!sync_file) return NULL; - fence = dma_fence_get(sync_file->fence); + if (!rcu_access_pointer(sync_file->fence)) + return NULL; + + rcu_read_lock(); + fence = dma_fence_get_rcu_safe(&sync_file->fence); + rcu_read_unlock(); + fput(sync_file->file); return fence; } EXPORT_SYMBOL(sync_file_get_fence); +static inline struct dma_fence * +sync_file_get_fence_locked(struct sync_file *sync_file) +{ + return rcu_dereference_protected(sync_file->fence, + sync_file_held(sync_file)); +} + static int sync_file_set_fence(struct sync_file *sync_file, struct dma_fence **fences, int num_fences) { @@ -143,7 +165,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) { - sync_file->fence = fences[0]; + RCU_INIT_POINTER(sync_file->fence, fences[0]); kfree(fences); } else { array = dma_fence_array_create(num_fences, fences, @@ -152,17 +174,20 @@ static int sync_file_set_fence(struct sync_file *sync_file, if (!array) return -ENOMEM; - sync_file->fence = &array->base; + RCU_INIT_POINTER(sync_file->fence, &array->base); } return 0; } +/* must be called with sync_file lock taken */ static struct dma_fence **get_fences(struct sync_file *sync_file, int *num_fences) { - if (dma_fence_is_array(sync_file->fence)) { - struct dma_fence_array *array = to_dma_fence_array(sync_file->fence); + struct dma_fence *fence = sync_file_get_fence_locked(sync_file); + + if (dma_fence_is_array(fence)) { + struct dma_fence_array *array = to_dma_fence_array(fence); *num_fences = array->num_fences; return array->fences; @@ -204,10 +229,13 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, if (!sync_file) return NULL; + mutex_lock(&a->lock); + mutex_lock(&b->lock); a_fences = get_fences(a, &a_num_fences); b_fences = get_fences(b, &b_num_fences); - if (a_num_fences > INT_MAX - b_num_fences) - return NULL; + if (a_num_fences > INT_MAX - b_num_fences) { + goto unlock; + } num_fences = a_num_fences + b_num_fences; @@ -268,11 +296,17 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, goto err; } + mutex_unlock(&b->lock); + mutex_unlock(&a->lock); + strlcpy(sync_file->name, name, sizeof(sync_file->name)); return sync_file; err: fput(sync_file->file); +unlock: + mutex_unlock(&b->lock); + mutex_unlock(&a->lock); return NULL; } @@ -281,10 +315,15 @@ static void sync_file_free(struct kref *kref) { struct sync_file *sync_file = container_of(kref, struct sync_file, kref); + struct dma_fence *fence; + + fence = rcu_dereference_protected(sync_file->fence, 1); + if (fence) { + if (test_bit(POLL_ENABLED, &fence->flags)) + dma_fence_remove_callback(fence, &sync_file->cb); + dma_fence_put(fence); + } - if (test_bit(POLL_ENABLED, &sync_file->fence->flags)) - dma_fence_remove_callback(sync_file->fence, &sync_file->cb); - dma_fence_put(sync_file->fence); kfree(sync_file); } @@ -299,16 +338,20 @@ static int sync_file_release(struct inode *inode, struct file *file) static unsigned int sync_file_poll(struct file *file, poll_table *wait) { struct sync_file *sync_file = file->private_data; + unsigned int ret_val; poll_wait(file, &sync_file->wq, wait); + mutex_lock(&sync_file->lock); if (!test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) { if (dma_fence_add_callback(sync_file->fence, &sync_file->cb, fence_check_cb_func) < 0) wake_up_all(&sync_file->wq); } + ret_val = dma_fence_is_signaled(sync_file->fence) ? POLLIN : 0; + mutex_unlock(&sync_file->lock); - return dma_fence_is_signaled(sync_file->fence) ? POLLIN : 0; + return ret_val; } static long sync_file_ioctl_merge(struct sync_file *sync_file, @@ -393,6 +436,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (info.flags || info.pad) return -EINVAL; + mutex_lock(&sync_file->lock); fences = get_fences(sync_file, &num_fences); /* @@ -404,13 +448,17 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (!info.num_fences) goto no_fences; - if (info.num_fences < num_fences) - return -EINVAL; + if (info.num_fences < num_fences) { + ret = -EINVAL; + goto out; + } size = num_fences * sizeof(*fence_info); fence_info = kzalloc(size, GFP_KERNEL); - if (!fence_info) - return -ENOMEM; + if (!fence_info) { + ret = -ENOMEM; + goto out; + } for (i = 0; i < num_fences; i++) sync_fill_fence_info(fences[i], &fence_info[i]); @@ -433,7 +481,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, out: kfree(fence_info); - + mutex_unlock(&sync_file->lock); return ret; } diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h index 3e3ab84..006412f 100644 --- a/include/linux/sync_file.h +++ b/include/linux/sync_file.h @@ -30,6 +30,7 @@ * @wq: wait queue for fence signaling * @fence: fence with the fences in the sync_file * @cb: fence callback information + * @lock: mutex to protect fence/cb - used for semaphores */ struct sync_file { struct file *file; @@ -41,8 +42,10 @@ struct sync_file { wait_queue_head_t wq; - struct dma_fence *fence; + struct dma_fence __rcu *fence; struct dma_fence_cb cb; + /* protects the fence pointer and cb */ + struct mutex lock; }; #define POLL_ENABLED DMA_FENCE_FLAG_USER_BITS -- 2.7.4