Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3243 discussions

Re: [Linaro-mm-sig] [PATCH] dma-buf/resv: fix exclusive fence get

by Chris Wilson

Quoting Qiang Yu (2019-09-22 08:49:00) > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig(a)amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825(a)gmail.com> > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; Oops. Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> -Chris

6 years, 2 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Replace strncpy() for stracpy()

by Dan Carpenter

On Sun, Sep 08, 2019 at 02:34:50PM +1000, Adam Zerella wrote: > Using strncpy() does not always terminate the destination string. > stracpy() is a alternative function that does, by using this new > function we will no longer need to insert a null separator. > > Signed-off-by: Adam Zerella <adam.zerella(a)gmail.com> > --- > drivers/staging/android/ion/ion.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index e6b1ca141b93..17901bd626be 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -433,8 +433,7 @@ static int ion_query_heaps(struct ion_heap_query *query) > max_cnt = query->cnt; > > plist_for_each_entry(heap, &dev->heaps, node) { > - strncpy(hdata.name, heap->name, MAX_HEAP_NAME); > - hdata.name[sizeof(hdata.name) - 1] = '\0'; > + stracpy(hdata.name, heap->name, MAX_HEAP_NAME); stracpy() only takes two arguments. This doesn't compile. regards, dan carpenter

6 years, 3 months

Use dma_fence_array for implementing shared dma_resv fences

by Christian König

This is the new dma_fence_array based container for shared fences in the dma_resv object. Advantage of this approach is that you can grab a reference to the current set of shared fences at any time, which allows us to drop the sequence number increment and makes the whole RCU handling much more easier. Disadvantage is that RCU users now have to grab a reference instead of using the sequence counter. As far as I can see i915 was actually the only driver doing this. So we optimize for adding more fences instead of reading them now. Another behavior change worth noting is that the shared fences are now only visible after unlocking the dma_resv object or calling dma_resv_fences_commit() manually. Please review and/or comment, Christian.

6 years, 3 months

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -528,20 +352,9 @@ void dma_resv_prune_fences(struct dma_resv *obj) > dma_fence_put(fence); > } > > - list = dma_resv_get_list(obj); > - if (!list) > - return; > - > - for (i = 0; i < list->shared_count; ++i) { > - fence = rcu_dereference_protected(list->shared[i], > - dma_resv_held(obj)); > - > - if (!dma_fence_is_signaled(fence)) > - continue; > - > - RCU_INIT_POINTER(list->shared[i], dma_fence_get_stub()); > - dma_fence_put(fence); > - } > + fence = dma_resv_fences_deref(obj, &obj->readers); > + if (dma_fence_is_signaled(fence)) > + dma_resv_fences_set(obj, &obj->readers, NULL); Something to note is that a dma-fence-array is not automatically signaled and dma_fence_is_signaled() does not check the array. -Chris

6 years, 3 months

[RFC] replacing dma_resv API

by Christian König

Hi everyone, In previous discussion it surfaced that different drivers use the shared and explicit fences in the dma_resv object with different meanings. This is problematic when we share buffers between those drivers and requirements for implicit and explicit synchronization leaded to quite a number of workarounds related to this. So I started an effort to get all drivers back to a common understanding of what the fences in the dma_resv object mean and be able to use the object for different kind of workloads independent of the classic DRM command submission interface. The result is this patch set which modifies the dma_resv API to get away from a single explicit fence and multiple shared fences, towards a notation where we have explicit categories for writers, readers and others. To do this I came up with a new container called dma_resv_fences which can store both a single fence as well as multiple fences in a dma_fence_array. This turned out to actually be even be quite a bit simpler, since we don't need any complicated dance between RCU and sequence count protected updates any more. Instead we can just grab a reference to the dma_fence_array under RCU and so keep the current state of synchronization alive until we are done with it. This results in both a small performance improvement since we don't need so many barriers any more, as well as fewer lines of code in the actual implementation. Please review and/or comment, Christian.

6 years, 4 months

[PATCH 3/3] udmabuf: check that flags has no unsupported bits set

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> --- drivers/dma-buf/udmabuf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 6c3ec8fcef01..ca1364102b18 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -131,6 +131,9 @@ static long udmabuf_create(const struct udmabuf_create_list *head, int seals, ret = -EINVAL; u32 i, flags; + if (head->flags & ~UDMABUF_FLAGS_CLOEXEC) + return -EINVAL; + ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); if (!ubuf) return -ENOMEM; -- 2.18.1

6 years, 4 months

[PATCH 2/3] udmabuf: check that __pad is zero

by Gerd Hoffmann

Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9635897458a0..6c3ec8fcef01 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -137,6 +137,8 @@ static long udmabuf_create(const struct udmabuf_create_list *head, pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { + if (list[i].__pad) + goto err; if (!IS_ALIGNED(list[i].offset, PAGE_SIZE)) goto err; if (!IS_ALIGNED(list[i].size, PAGE_SIZE)) -- 2.18.1

6 years, 4 months

[PATCH 1/3] udmabuf: add documentation

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- include/uapi/linux/udmabuf.h | 52 ++++++++++++++++++++++++++-- Documentation/driver-api/dma-buf.rst | 8 +++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..9fe440abf2f9 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -5,8 +5,39 @@ #include <linux/types.h> #include <linux/ioctl.h> +/** + * DOC: udmabuf + * + * udmabuf is a device driver which allows userspace to create + * dmabufs. The memory used for these dmabufs must be backed by + * memfd. The memfd must have F_SEAL_SHRINK and it must not have + * F_SEAL_WRITE. + * + * The driver has two ioctls, one to create a dmabuf from a single + * memory block and one to create a dmabuf from a list of memory + * blocks. + * + * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create) + * + * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list) + */ + +#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) +#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) + #define UDMABUF_FLAGS_CLOEXEC 0x01 +/** + * struct udmabuf_create - create a dmabuf from a single memory block. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + * + * @flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create { __u32 memfd; __u32 flags; @@ -14,6 +45,15 @@ struct udmabuf_create { __u64 size; }; +/** + * struct udmabuf_create_item - one memory block list item. + * + * @memfd: The file handle. + * @__pad: Padding field (unused). + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + */ struct udmabuf_create_item { __u32 memfd; __u32 __pad; @@ -21,13 +61,19 @@ struct udmabuf_create_item { __u64 size; }; +/** + * struct udmabuf_create_list - create a dmabuf from a memory block list. + * + * @count: The number of list elements. + * @list: The memory block list + * + * @flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create_list { __u32 flags; __u32 count; struct udmabuf_create_item list[]; }; -#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) -#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) - #endif /* _UAPI_LINUX_UDMABUF_H */ diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index b541e97c7ab1..1f62c30a14b0 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -166,3 +166,11 @@ DMA Fence uABI/Sync File .. kernel-doc:: include/linux/sync_file.h :internal: +Userspace DMA Buffer driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :doc: udmabuf + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :internal: -- 2.18.1

6 years, 4 months

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -117,17 +120,10 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, > busy_check_writer(rcu_dereference(obj->base.resv->fence_excl)); > > /* Translate shared fences to READ set of engines */ > - list = rcu_dereference(obj->base.resv->fence); > - if (list) { > - unsigned int shared_count = list->shared_count, i; > - > - for (i = 0; i < shared_count; ++i) { > - struct dma_fence *fence = > - rcu_dereference(list->shared[i]); > - > - args->busy |= busy_check_reader(fence); > - } > - } > + readers = dma_resv_fences_get_rcu(&obj->base.resv->readers); > + dma_fence_array_for_each(fence, cursor, readers) > + args->busy |= busy_check_reader(fence); > + dma_fence_put(readers); That's underwhelming, the full-mb shows up in scaling tests (I'll test the impact of this series later). Something like, do { read = 0; fences = dma_resv_fences_get_deref(&obj->base.resv->readers); dma_fence_array_for_each(fence, cursor, fences) read |= busy_check_reader(fence); smp_rmb(); } while (dma_resv_fences_get_deref(obj->readers) != fences) do { fences = dma_resv_fences_get_deref(&obj->base.resv->fences); write = busy_check_writer(fences); smp_rmb(); } while (dma_resv_fences_get_deref(obj->writes) != fences) args->busy = write | read; Perhaps? -Chris

6 years, 4 months

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -528,20 +352,9 @@ void dma_resv_prune_fences(struct dma_resv *obj) > dma_fence_put(fence); > } > > - list = dma_resv_get_list(obj); > - if (!list) > - return; > - > - for (i = 0; i < list->shared_count; ++i) { > - fence = rcu_dereference_protected(list->shared[i], > - dma_resv_held(obj)); > - > - if (!dma_fence_is_signaled(fence)) > - continue; > - > - RCU_INIT_POINTER(list->shared[i], dma_fence_get_stub()); > - dma_fence_put(fence); > - } > + fence = dma_resv_fences_deref(obj, &obj->readers); > + if (dma_fence_is_signaled(fence)) > + dma_resv_fences_set(obj, &obj->readers, NULL); Needs fence==NULL safeguards. It's not as pruny as it was before. Or did you add some magic to fence-array? -Chris

6 years, 4 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig