From: Dave Airlie <airlied@xxxxxxxxxx>
This patch allows the underlying fence in a sync_file to be changed
or set to NULL. This isn't currently required but for Vulkan
semaphores we need to be able to swap and reset the fence.
In order to faciliate this, it uses rcu to protect the fence,
along with a new mutex. The mutex also protects the callback.
It also checks for NULL when retrieving the rcu protected
fence in case it has been reset.
v1.1: fix the locking (Julia Lawall).
v2: use rcu try one
v3: fix poll to use proper rcu, fixup merge/fill ioctls
to not crash on NULL fence cases.
Signed-off-by: Dave Airlie <airlied at redhat.com>
---
drivers/dma-buf/sync_file.c | 112 +++++++++++++++++++++++++++++++++++---------
include/linux/sync_file.h | 5 +-
2 files changed, 94 insertions(+), 23 deletions(-)
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 2321035..dcba931 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,25 @@ 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 (!fence) {
+ *num_fences = 0;
+ return NULL;
+ }
+
+ 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 +234,16 @@ 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 || !b_num_fences)
+ goto unlock;
+
+ if (a_num_fences > INT_MAX - b_num_fences) {
+ goto unlock;
+ }
num_fences = a_num_fences + b_num_fences;
@@ -268,11 +304,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 +323,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 +346,25 @@ 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 = 0;
+ struct dma_fence *fence;
poll_wait(file, &sync_file->wq, wait);
- 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);
+ mutex_lock(&sync_file->lock);
+
+ fence = sync_file_get_fence_locked(sync_file);
+ if (fence) {
+ if (!test_and_set_bit(POLL_ENABLED, &fence->flags)) {
+ if (dma_fence_add_callback(fence, &sync_file->cb,
+ fence_check_cb_func) < 0)
+ wake_up_all(&sync_file->wq);
+ }
+ ret_val = dma_fence_is_signaled(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,8 +449,13 @@ 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);
+ /* if there are no fences in the sync_file just return */
+ if (!num_fences)
+ goto no_fences;
+
/*
* Passing num_fences = 0 means that userspace doesn't want to
* retrieve any sync_fence_info. If num_fences = 0 we skip filling
@@ -404,13 +465,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]);
@@ -423,7 +488,10 @@ 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(sync_file->fence);
+ if (num_fences)
+ info.status = dma_fence_is_signaled(sync_file->fence);
+ else
+ info.status = -ENOENT;
info.num_fences = num_fences;
if (copy_to_user((void __user *)arg, &info, sizeof(info)))
@@ -433,7 +501,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
