[PATCH] dma-buf: Use dma_fence_unwrap_for_each when importing fences
Christian König
christian.koenig at amd.com
Sun Aug 7 16:35:20 UTC 2022
Am 02.08.22 um 23:01 schrieb Jason Ekstrand:
> Ever since 68129f431faa ("dma-buf: warn about containers in dma_resv object"),
> dma_resv_add_shared_fence will warn if you attempt to add a container fence.
> While most drivers were fine, fences can also be added to a dma_resv via the
> recently added DMA_BUF_IOCTL_IMPORT_SYNC_FILE. Use dma_fence_unwrap_for_each
> to add each fence one at a time.
>
> Fixes: 594740497e99 ("dma-buf: Add an API for importing sync files (v10)")
> Signed-off-by: Jason Ekstrand <jason.ekstrand at collabora.com>
> Reported-by: Sarah Walker <Sarah.Walker at imgtec.com>
> Cc: Christian König <christian.koenig at amd.com>
> ---
> drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 630133284e2b..8d5d45112f52 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/dma-buf.h>
> #include <linux/dma-fence.h>
> +#include <linux/dma-fence-unwrap.h>
> #include <linux/anon_inodes.h>
> #include <linux/export.h>
> #include <linux/debugfs.h>
> @@ -391,8 +392,10 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf,
> const void __user *user_data)
> {
> struct dma_buf_import_sync_file arg;
> - struct dma_fence *fence;
> + struct dma_fence *fence, *f;
> enum dma_resv_usage usage;
> + struct dma_fence_unwrap iter;
> + unsigned int num_fences;
> int ret = 0;
>
> if (copy_from_user(&arg, user_data, sizeof(arg)))
> @@ -411,13 +414,21 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf,
> usage = (arg.flags & DMA_BUF_SYNC_WRITE) ? DMA_RESV_USAGE_WRITE :
> DMA_RESV_USAGE_READ;
>
> - dma_resv_lock(dmabuf->resv, NULL);
> + num_fences = 0;
> + dma_fence_unwrap_for_each(f, &iter, fence)
> + ++num_fences;
>
> - ret = dma_resv_reserve_fences(dmabuf->resv, 1);
> - if (!ret)
> - dma_resv_add_fence(dmabuf->resv, fence, usage);
> + if (num_fences > 0) {
> + dma_resv_lock(dmabuf->resv, NULL);
>
> - dma_resv_unlock(dmabuf->resv);
> + ret = dma_resv_reserve_fences(dmabuf->resv, num_fences);
That looks like it is misplaced.
You *must* only lock the reservation object once and then add all fences
in one go.
Thinking now about it we probably had a bug around that before as well.
Going to double check tomorrow.
Regards,
Christian.
> + if (!ret) {
> + dma_fence_unwrap_for_each(f, &iter, fence)
> + dma_resv_add_fence(dmabuf->resv, f, usage);
> + }
> +
> + dma_resv_unlock(dmabuf->resv);
> + }
>
> dma_fence_put(fence);
>
More information about the dri-devel
mailing list