Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

7 participants
3604 discussions

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: add buffer flag update ioctl

by Laura Abbott

On 12/19/18 5:39 PM, Zengtao (B) wrote: > Hi laura: > >> -----Original Message----- >> From: Laura Abbott [mailto:labbott@redhat.com] >> Sent: Thursday, December 20, 2018 2:10 AM >> To: Zengtao (B) <prime.zeng(a)hisilicon.com>; sumit.semwal(a)linaro.org >> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>; Arve Hjønnevåg >> <arve(a)android.com>; Todd Kjos <tkjos(a)android.com>; Martijn Coenen >> <maco(a)android.com>; Joel Fernandes <joel(a)joelfernandes.org>; >> devel(a)driverdev.osuosl.org; dri-devel(a)lists.freedesktop.org; >> linaro-mm-sig(a)lists.linaro.org; linux-kernel(a)vger.kernel.org >> Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl >> >> On 12/19/18 9:19 AM, Zeng Tao wrote: >>> In some usecases, the buffer cached attribute is not determined at >>> allocation time, it's determined just before the real cpu mapping. >>> And from the memory view of point, a buffer should not have the >> cached >>> attribute util is really mapped by the cpu. So in this patch, we >>> introduced the new ioctl command to target the requirement. >>> >> >> This is racy and error prone. Can you explain more what problem you are >> trying to solve? > > My use case is like this: > 1. There are two process A and B, A takes case of ion buffer allocation, and > pass the buffer fd to B, then B maps and uses it. > 2. Process B need to map the buffer with different cached attribute for > different use case, for example, if the buffer is used for pure software algorithm, > then we need to map it as cached, otherwise non-cached, and B needs to deal > with both cases. > And unfortunately the mmap syscall takes no cached flags and we can't decide > the cache attribute when we are doing the mmap, so I introduce new the ioctl > even though I think the solution is not as good. > > Thanks for the explanation, this was about the use case I expected. I'm pretty sure I had this exact problem once upon a time and we didn't come up with a solution. I'd still like to get rid of uncached buffers in general and just use cached buffers (see http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-Nove…) What's your usecase for uncached buffers? >> >>> Signed-off-by: Zeng Tao <prime.zeng(a)hisilicon.com> >>> --- >>> drivers/staging/android/ion/ion-ioctl.c | 4 ++++ >>> drivers/staging/android/ion/ion.c | 17 +++++++++++++++++ >>> drivers/staging/android/ion/ion.h | 1 + >>> drivers/staging/android/uapi/ion.h | 22 >> ++++++++++++++++++++++ >>> 4 files changed, 44 insertions(+) >>> >>> diff --git a/drivers/staging/android/ion/ion-ioctl.c >>> b/drivers/staging/android/ion/ion-ioctl.c >>> index a8d3cc4..60bb702 100644 >>> --- a/drivers/staging/android/ion/ion-ioctl.c >>> +++ b/drivers/staging/android/ion/ion-ioctl.c >>> @@ -12,6 +12,7 @@ >>> >>> union ion_ioctl_arg { >>> struct ion_allocation_data allocation; >>> + struct ion_buffer_flag_data update; >>> struct ion_heap_query query; >>> }; >>> >>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd, >>> unsigned long arg) >>> >>> break; >>> } >>> + case ION_IOC_BUFFER_UPDATE: >>> + ret = ion_buffer_update(data.update.fd, data.update.flags); >>> + break; >>> case ION_IOC_HEAP_QUERY: >>> ret = ion_query_heaps(&data.query); >>> break; >>> diff --git a/drivers/staging/android/ion/ion.c >>> b/drivers/staging/android/ion/ion.c >>> index 9907332..f1404dc 100644 >>> --- a/drivers/staging/android/ion/ion.c >>> +++ b/drivers/staging/android/ion/ion.c >>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int >> heap_id_mask, unsigned int flags) >>> return fd; >>> } >>> >>> +int ion_buffer_update(unsigned int fd, unsigned int flags) { >>> + struct dma_buf *dmabuf; >>> + struct ion_buffer *buffer; >>> + >>> + dmabuf = dma_buf_get(fd); >>> + >>> + if (!dmabuf) >>> + return -EINVAL; >>> + >>> + buffer = dmabuf->priv; >>> + buffer->flags = flags; >>> + dma_buf_put(dmabuf); >>> + >>> + return 0; >>> +} >>> + >>> int ion_query_heaps(struct ion_heap_query *query) >>> { >>> struct ion_device *dev = internal_dev; diff --git >>> a/drivers/staging/android/ion/ion.h >>> b/drivers/staging/android/ion/ion.h >>> index c006fc1..99bf9ab 100644 >>> --- a/drivers/staging/android/ion/ion.h >>> +++ b/drivers/staging/android/ion/ion.h >>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page, >> size_t size, pgprot_t pgprot); >>> int ion_alloc(size_t len, >>> unsigned int heap_id_mask, >>> unsigned int flags); >>> +int ion_buffer_update(unsigned int fd, unsigned int flags); >>> >>> /** >>> * ion_heap_init_shrinker >>> diff --git a/drivers/staging/android/uapi/ion.h >>> b/drivers/staging/android/uapi/ion.h >>> index 5d70098..99753fc 100644 >>> --- a/drivers/staging/android/uapi/ion.h >>> +++ b/drivers/staging/android/uapi/ion.h >>> @@ -74,6 +74,20 @@ struct ion_allocation_data { >>> __u32 unused; >>> }; >>> >>> +/** >>> + * struct ion_buffer_flag_data - metadata passed from userspace for >>> +update >>> + * buffer flags >>> + * @fd: file descriptor of the buffer >>> + * @flags: flags passed to the buffer >>> + * >>> + * Provided by userspace as an argument to the ioctl */ >>> + >>> +struct ion_buffer_flag_data { >>> + __u32 fd; >>> + __u32 flags; >>> +} >>> + >>> #define MAX_HEAP_NAME 32 >>> >>> /** >>> @@ -116,6 +130,14 @@ struct ion_heap_query { >>> struct ion_allocation_data) >>> >>> /** >>> + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer >> flags >>> + * >>> + * Takes an ion_buffer_flag_data structure and returns the result of >>> +the >>> + * buffer flag update operation. >>> + */ >>> +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \ >>> + struct ion_buffer_flag_data) >>> +/** >>> * DOC: ION_IOC_HEAP_QUERY - information about available heaps >>> * >>> * Takes an ion_heap_query structure and populates information >> about >>> >

7 years, 9 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: add buffer flag update ioctl

by Laura Abbott

On 12/19/18 9:19 AM, Zeng Tao wrote: > In some usecases, the buffer cached attribute is not determined at > allocation time, it's determined just before the real cpu mapping. > And from the memory view of point, a buffer should not have the cached > attribute util is really mapped by the cpu. So in this patch, we > introduced the new ioctl command to target the requirement. > This is racy and error prone. Can you explain more what problem you are trying to solve? > Signed-off-by: Zeng Tao <prime.zeng(a)hisilicon.com> > --- > drivers/staging/android/ion/ion-ioctl.c | 4 ++++ > drivers/staging/android/ion/ion.c | 17 +++++++++++++++++ > drivers/staging/android/ion/ion.h | 1 + > drivers/staging/android/uapi/ion.h | 22 ++++++++++++++++++++++ > 4 files changed, 44 insertions(+) > > diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c > index a8d3cc4..60bb702 100644 > --- a/drivers/staging/android/ion/ion-ioctl.c > +++ b/drivers/staging/android/ion/ion-ioctl.c > @@ -12,6 +12,7 @@ > > union ion_ioctl_arg { > struct ion_allocation_data allocation; > + struct ion_buffer_flag_data update; > struct ion_heap_query query; > }; > > @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > break; > } > + case ION_IOC_BUFFER_UPDATE: > + ret = ion_buffer_update(data.update.fd, data.update.flags); > + break; > case ION_IOC_HEAP_QUERY: > ret = ion_query_heaps(&data.query); > break; > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 9907332..f1404dc 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags) > return fd; > } > > +int ion_buffer_update(unsigned int fd, unsigned int flags) > +{ > + struct dma_buf *dmabuf; > + struct ion_buffer *buffer; > + > + dmabuf = dma_buf_get(fd); > + > + if (!dmabuf) > + return -EINVAL; > + > + buffer = dmabuf->priv; > + buffer->flags = flags; > + dma_buf_put(dmabuf); > + > + return 0; > +} > + > int ion_query_heaps(struct ion_heap_query *query) > { > struct ion_device *dev = internal_dev; > diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h > index c006fc1..99bf9ab 100644 > --- a/drivers/staging/android/ion/ion.h > +++ b/drivers/staging/android/ion/ion.h > @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot); > int ion_alloc(size_t len, > unsigned int heap_id_mask, > unsigned int flags); > +int ion_buffer_update(unsigned int fd, unsigned int flags); > > /** > * ion_heap_init_shrinker > diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h > index 5d70098..99753fc 100644 > --- a/drivers/staging/android/uapi/ion.h > +++ b/drivers/staging/android/uapi/ion.h > @@ -74,6 +74,20 @@ struct ion_allocation_data { > __u32 unused; > }; > > +/** > + * struct ion_buffer_flag_data - metadata passed from userspace for update > + * buffer flags > + * @fd: file descriptor of the buffer > + * @flags: flags passed to the buffer > + * > + * Provided by userspace as an argument to the ioctl > + */ > + > +struct ion_buffer_flag_data { > + __u32 fd; > + __u32 flags; > +} > + > #define MAX_HEAP_NAME 32 > > /** > @@ -116,6 +130,14 @@ struct ion_heap_query { > struct ion_allocation_data) > > /** > + * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer flags > + * > + * Takes an ion_buffer_flag_data structure and returns the result of the > + * buffer flag update operation. > + */ > +#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \ > + struct ion_buffer_flag_data) > +/** > * DOC: ION_IOC_HEAP_QUERY - information about available heaps > * > * Takes an ion_heap_query structure and populates information about >

7 years, 9 months

[PATCH] dma-buf: fix debugfs versus rcu and fence dumping v2

by jglisse＠redhat.com

From: Jérôme Glisse <jglisse(a)redhat.com> The debugfs take reference on fence without dropping them. Also the rcu section are not well balance. Fix all that ... Changed since v1: - moved fobj logic around to be rcu safe Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org --- drivers/dma-buf/dma-buf.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 13884474d158..9688d99894d6 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1048,27 +1048,38 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) while (true) { seq = read_seqcount_begin(&robj->seq); rcu_read_lock(); - fobj = rcu_dereference(robj->fence); - shared_count = fobj ? fobj->shared_count : 0; fence = rcu_dereference(robj->fence_excl); + fence = dma_fence_get_rcu(fence); if (!read_seqcount_retry(&robj->seq, seq)) break; rcu_read_unlock(); } - - if (fence) + if (fence) { seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); - for (i = 0; i < shared_count; i++) { + dma_fence_put(fence); + } + + rcu_read_lock(); + fobj = rcu_dereference(robj->fence); + shared_count = fobj ? fobj->shared_count : 0; + for (i = 0, fence = NULL; i < shared_count; i++) { fence = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(fence)) continue; + rcu_read_unlock(); + seq_printf(s, "\tShared fence: %s %s %ssignalled\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); + + rcu_read_lock(); + fobj = rcu_dereference(robj->fence); + shared_count = fobj ? fobj->shared_count : 0; } rcu_read_unlock(); -- 2.17.2

7 years, 10 months

[PATCH] dma-buf: balance refcount inbalance

by jglisse＠redhat.com

From: Jérôme Glisse <jglisse(a)redhat.com> The debugfs take reference on fence without dropping them. Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org --- drivers/dma-buf/dma-buf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 13884474d158..69842145c223 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1069,6 +1069,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); } rcu_read_unlock(); -- 2.17.2

7 years, 10 months

[PATCH] dma-buf: fix debugfs versus rcu and fence dumping

by jglisse＠redhat.com

From: Jérôme Glisse <jglisse(a)redhat.com> The debugfs take reference on fence without dropping them. Also the rcu section are not well balance. Fix all that ... Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org --- drivers/dma-buf/dma-buf.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 13884474d158..f6f4de42ac49 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1051,24 +1051,31 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) fobj = rcu_dereference(robj->fence); shared_count = fobj ? fobj->shared_count : 0; fence = rcu_dereference(robj->fence_excl); + fence = dma_fence_get_rcu(fence); if (!read_seqcount_retry(&robj->seq, seq)) break; rcu_read_unlock(); } - - if (fence) + if (fence) { seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); + } + + rcu_read_lock(); for (i = 0; i < shared_count; i++) { fence = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(fence)) continue; + rcu_read_unlock(); seq_printf(s, "\tShared fence: %s %s %ssignalled\n", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); + rcu_read_lock(); } rcu_read_unlock(); -- 2.17.2

7 years, 10 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Remove unused header files

by Laura Abbott

On 11/30/18 6:43 AM, Yangtao Li wrote: > seq_file.h does not need to be included,so remove it. > Acked-by: Laura Abbott <labbott(a)redhat.com> > Signed-off-by: Yangtao Li <tiny.windzz(a)gmail.com> > --- > drivers/staging/android/ion/ion.c | 1 - > drivers/staging/android/ion/ion_system_heap.c | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index 99073325b0c0..0d61e9cd0887 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -23,7 +23,6 @@ > #include <linux/mm_types.h> > #include <linux/rbtree.h> > #include <linux/sched/task.h> > -#include <linux/seq_file.h> > #include <linux/slab.h> > #include <linux/uaccess.h> > #include <linux/vmalloc.h> > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index 548bb02c0ca6..9ce2c0d7ac17 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -11,7 +11,6 @@ > #include <linux/highmem.h> > #include <linux/mm.h> > #include <linux/scatterlist.h> > -#include <linux/seq_file.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > #include "ion.h" >

7 years, 10 months

[RFC] android: ion: How to properly clean caches for uncached allocations

by Liam Mark

The issue: Currently in ION if you allocate uncached memory it is possible that there are still dirty lines in the cache. And often these dirty lines in the cache are the zeros which were meant to clear out any sensitive kernel data. What this means is that if you allocate uncached memory from ION, and then subsequently write to that buffer (using the uncached mapping you are provided by ION) then the data you have written could be corrupted at some point in the future if a dirty line is evicted from the cache. Also this means there is a potential security issue. If an un-privileged userspace user allocated uncached memory (for example from the system heap) and then if they were to read from that buffer (through the un-cached mapping they are provided by ION), and if some of the zeros which were written to that memory are still in the cache then this un-privileged userspace user could read potentially sensitive kernel data. An unacceptable fix: I have included some sample code which should resolve this issue for the system heap and the cma heap on some architectures, however this code would not be acceptable for upstreaming since it uses hacks to clean the cache. Similar changes would need to be made to carveout heap and chunk heap. I would appreciate some feedback on the proper way for ION to clean the caches for memory it has allocated that is intended for uncached access. I realize that it may be tempting, as a solution to this problem, to simply strip uncached support from ION. I hope that we can try to find a solution which preserves uncached memory support as ION uncached memory is often used (though perhaps not in upstreamed code) in cases such as multimedia use cases where there is no CPU access required, in secure heap allocations, and in some cases where there is minimal CPU access and therefore uncached memory performs better. Signed-off-by: Liam Mark <lmark(a)codeaurora.org> --- drivers/staging/android/ion/ion.c | 16 ++++++++++++++++ drivers/staging/android/ion/ion.h | 5 ++++- drivers/staging/android/ion/ion_cma_heap.c | 3 +++ drivers/staging/android/ion/ion_page_pool.c | 8 ++++++-- drivers/staging/android/ion/ion_system_heap.c | 7 ++++++- 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 57e0d8035b2e..10e967b0a0f4 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -38,6 +38,22 @@ bool ion_buffer_cached(struct ion_buffer *buffer) return !!(buffer->flags & ION_FLAG_CACHED); } +void ion_pages_sync_for_device(struct page *page, size_t size, + enum dma_data_direction dir) +{ + struct scatterlist sg; + struct device dev = {0}; + + /* hack, use dummy device */ + arch_setup_dma_ops(&dev, 0, 0, NULL, false); + + sg_init_table(&sg, 1); + sg_set_page(&sg, page, size, 0); + /* hack, use phys address for dma address */ + sg_dma_address(&sg) = page_to_phys(page); + dma_sync_sg_for_device(&dev, &sg, 1, dir); +} + /* this function should only be called while dev->lock is held */ static void ion_buffer_add(struct ion_device *dev, struct ion_buffer *buffer) diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index a238f23c9116..227b9928d185 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -192,6 +192,9 @@ struct ion_heap { */ bool ion_buffer_cached(struct ion_buffer *buffer); +void ion_pages_sync_for_device(struct page *page, size_t size, + enum dma_data_direction dir); + /** * ion_buffer_fault_user_mappings - fault in user mappings of this buffer * @buffer: buffer @@ -333,7 +336,7 @@ struct ion_page_pool { struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order, bool cached); void ion_page_pool_destroy(struct ion_page_pool *pool); -struct page *ion_page_pool_alloc(struct ion_page_pool *pool); +struct page *ion_page_pool_alloc(struct ion_page_pool *pool, bool *from_pool); void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); /** ion_page_pool_shrink - shrinks the size of the memory cached in the pool diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index 49718c96bf9e..82e80621d114 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -59,6 +59,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, memset(page_address(pages), 0, size); } + if (!ion_buffer_cached(buffer)) + ion_pages_sync_for_device(pages, size, DMA_BIDIRECTIONAL); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index b3017f12835f..169a321778ed 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -63,7 +63,7 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high) return page; } -struct page *ion_page_pool_alloc(struct ion_page_pool *pool) +struct page *ion_page_pool_alloc(struct ion_page_pool *pool, bool *from_pool) { struct page *page = NULL; @@ -76,8 +76,12 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool) page = ion_page_pool_remove(pool, false); mutex_unlock(&pool->mutex); - if (!page) + if (!page) { page = ion_page_pool_alloc_pages(pool); + *from_pool = false; + } else { + *from_pool = true; + } return page; } diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index bc19cdd30637..3bb4604e032b 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -57,13 +57,18 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap, bool cached = ion_buffer_cached(buffer); struct ion_page_pool *pool; struct page *page; + bool from_pool; if (!cached) pool = heap->uncached_pools[order_to_index(order)]; else pool = heap->cached_pools[order_to_index(order)]; - page = ion_page_pool_alloc(pool); + page = ion_page_pool_alloc(pool, &from_pool); + + if (!from_pool && !ion_buffer_cached(buffer)) + ion_pages_sync_for_device(page, PAGE_SIZE << order, + DMA_BIDIRECTIONAL); return page; } -- 1.8.5.2 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

7 years, 10 months

Re: [Linaro-mm-sig] [PATCH] udmabuf: set read/write flag when exporting

by Gerd Hoffmann

On Thu, Nov 15, 2018 at 05:46:30PM -0800, Gurchetan Singh wrote: > Reviewed-by: Gurchetan Singh <gurchetansingh(a)chromium.org> > Tested-by: Gurchetan Singh <gurchetansingh(a)chromium.org> Thanks. Pushed to drm-misc-fixes. cheers, Gerd

7 years, 10 months

[PATCH] udmabuf: set read/write flag when exporting

by Gerd Hoffmann

Otherwise, mmap fails when done with PROT_WRITE. Suggested-by: Gurchetan Singh <gurchetansingh(a)chromium.org> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index e70328ab7e..d9ff246093 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -189,6 +189,7 @@ static long udmabuf_create(const struct udmabuf_create_list *head, exp_info.ops = &udmabuf_ops; exp_info.size = ubuf->pagecount << PAGE_SHIFT; exp_info.priv = ubuf; + exp_info.flags = O_RDWR; buf = dma_buf_export(&exp_info); if (IS_ERR(buf)) { -- 2.9.3

7 years, 10 months

Re: [Linaro-mm-sig] [PATCH 1/8] dma-buf: remove shared fence staging in reservation object

by Christian König

Ping! Adding a few people directly and more mailing lists. Can I get an acked-by/rb for this? It's only a cleanup and not much functional change. Regards, Christian. Am 04.10.2018 um 15:12 schrieb Christian König: > No need for that any more. Just replace the list when there isn't enough > room any more for the additional fence. > > Signed-off-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/reservation.c | 178 ++++++++++++++---------------------------- > include/linux/reservation.h | 4 - > 2 files changed, 58 insertions(+), 124 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index 6c95f61a32e7..5825fc336a13 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -68,105 +68,23 @@ EXPORT_SYMBOL(reservation_seqcount_string); > */ > int reservation_object_reserve_shared(struct reservation_object *obj) > { > - struct reservation_object_list *fobj, *old; > - u32 max; > + struct reservation_object_list *old, *new; > + unsigned int i, j, k, max; > > old = reservation_object_get_list(obj); > > if (old && old->shared_max) { > - if (old->shared_count < old->shared_max) { > - /* perform an in-place update */ > - kfree(obj->staged); > - obj->staged = NULL; > + if (old->shared_count < old->shared_max) > return 0; > - } else > + else > max = old->shared_max * 2; > - } else > - max = 4; > - > - /* > - * resize obj->staged or allocate if it doesn't exist, > - * noop if already correct size > - */ > - fobj = krealloc(obj->staged, offsetof(typeof(*fobj), shared[max]), > - GFP_KERNEL); > - if (!fobj) > - return -ENOMEM; > - > - obj->staged = fobj; > - fobj->shared_max = max; > - return 0; > -} > -EXPORT_SYMBOL(reservation_object_reserve_shared); > - > -static void > -reservation_object_add_shared_inplace(struct reservation_object *obj, > - struct reservation_object_list *fobj, > - struct dma_fence *fence) > -{ > - struct dma_fence *signaled = NULL; > - u32 i, signaled_idx; > - > - dma_fence_get(fence); > - > - preempt_disable(); > - write_seqcount_begin(&obj->seq); > - > - for (i = 0; i < fobj->shared_count; ++i) { > - struct dma_fence *old_fence; > - > - old_fence = rcu_dereference_protected(fobj->shared[i], > - reservation_object_held(obj)); > - > - if (old_fence->context == fence->context) { > - /* memory barrier is added by write_seqcount_begin */ > - RCU_INIT_POINTER(fobj->shared[i], fence); > - write_seqcount_end(&obj->seq); > - preempt_enable(); > - > - dma_fence_put(old_fence); > - return; > - } > - > - if (!signaled && dma_fence_is_signaled(old_fence)) { > - signaled = old_fence; > - signaled_idx = i; > - } > - } > - > - /* > - * memory barrier is added by write_seqcount_begin, > - * fobj->shared_count is protected by this lock too > - */ > - if (signaled) { > - RCU_INIT_POINTER(fobj->shared[signaled_idx], fence); > } else { > - BUG_ON(fobj->shared_count >= fobj->shared_max); > - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); > - fobj->shared_count++; > + max = 4; > } > > - write_seqcount_end(&obj->seq); > - preempt_enable(); > - > - dma_fence_put(signaled); > -} > - > -static void > -reservation_object_add_shared_replace(struct reservation_object *obj, > - struct reservation_object_list *old, > - struct reservation_object_list *fobj, > - struct dma_fence *fence) > -{ > - unsigned i, j, k; > - > - dma_fence_get(fence); > - > - if (!old) { > - RCU_INIT_POINTER(fobj->shared[0], fence); > - fobj->shared_count = 1; > - goto done; > - } > + new = kmalloc(offsetof(typeof(*new), shared[max]), GFP_KERNEL); > + if (!new) > + return -ENOMEM; > > /* > * no need to bump fence refcounts, rcu_read access > @@ -174,46 +92,45 @@ reservation_object_add_shared_replace(struct reservation_object *obj, > * references from the old struct are carried over to > * the new. > */ > - for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) { > - struct dma_fence *check; > + for (i = 0, j = 0, k = max; i < (old ? old->shared_count : 0); ++i) { > + struct dma_fence *fence; > > - check = rcu_dereference_protected(old->shared[i], > - reservation_object_held(obj)); > - > - if (check->context == fence->context || > - dma_fence_is_signaled(check)) > - RCU_INIT_POINTER(fobj->shared[--k], check); > + fence = rcu_dereference_protected(old->shared[i], > + reservation_object_held(obj)); > + if (dma_fence_is_signaled(fence)) > + RCU_INIT_POINTER(new->shared[--k], fence); > else > - RCU_INIT_POINTER(fobj->shared[j++], check); > + RCU_INIT_POINTER(new->shared[j++], fence); > } > - fobj->shared_count = j; > - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); > - fobj->shared_count++; > + new->shared_count = j; > + new->shared_max = max; > > -done: > preempt_disable(); > write_seqcount_begin(&obj->seq); > /* > * RCU_INIT_POINTER can be used here, > * seqcount provides the necessary barriers > */ > - RCU_INIT_POINTER(obj->fence, fobj); > + RCU_INIT_POINTER(obj->fence, new); > write_seqcount_end(&obj->seq); > preempt_enable(); > > if (!old) > - return; > + return 0; > > /* Drop the references to the signaled fences */ > - for (i = k; i < fobj->shared_max; ++i) { > - struct dma_fence *f; > + for (i = k; i < new->shared_max; ++i) { > + struct dma_fence *fence; > > - f = rcu_dereference_protected(fobj->shared[i], > - reservation_object_held(obj)); > - dma_fence_put(f); > + fence = rcu_dereference_protected(new->shared[i], > + reservation_object_held(obj)); > + dma_fence_put(fence); > } > kfree_rcu(old, rcu); > + > + return 0; > } > +EXPORT_SYMBOL(reservation_object_reserve_shared); > > /** > * reservation_object_add_shared_fence - Add a fence to a shared slot > @@ -226,15 +143,39 @@ reservation_object_add_shared_replace(struct reservation_object *obj, > void reservation_object_add_shared_fence(struct reservation_object *obj, > struct dma_fence *fence) > { > - struct reservation_object_list *old, *fobj = obj->staged; > + struct reservation_object_list *fobj; > + unsigned int i; > > - old = reservation_object_get_list(obj); > - obj->staged = NULL; > + dma_fence_get(fence); > + > + fobj = reservation_object_get_list(obj); > > - if (!fobj) > - reservation_object_add_shared_inplace(obj, old, fence); > - else > - reservation_object_add_shared_replace(obj, old, fobj, fence); > + preempt_disable(); > + write_seqcount_begin(&obj->seq); > + > + for (i = 0; i < fobj->shared_count; ++i) { > + struct dma_fence *old_fence; > + > + old_fence = rcu_dereference_protected(fobj->shared[i], > + reservation_object_held(obj)); > + if (old_fence->context == fence->context || > + dma_fence_is_signaled(old_fence)) { > + dma_fence_put(old_fence); > + goto replace; > + } > + } > + > + BUG_ON(fobj->shared_count >= fobj->shared_max); > + fobj->shared_count++; > + > +replace: > + /* > + * memory barrier is added by write_seqcount_begin, > + * fobj->shared_count is protected by this lock too > + */ > + RCU_INIT_POINTER(fobj->shared[i], fence); > + write_seqcount_end(&obj->seq); > + preempt_enable(); > } > EXPORT_SYMBOL(reservation_object_add_shared_fence); > > @@ -343,9 +284,6 @@ int reservation_object_copy_fences(struct reservation_object *dst, > new = dma_fence_get_rcu_safe(&src->fence_excl); > rcu_read_unlock(); > > - kfree(dst->staged); > - dst->staged = NULL; > - > src_list = reservation_object_get_list(dst); > old = reservation_object_get_excl(dst); > > diff --git a/include/linux/reservation.h b/include/linux/reservation.h > index 02166e815afb..54cf6773a14c 100644 > --- a/include/linux/reservation.h > +++ b/include/linux/reservation.h > @@ -68,7 +68,6 @@ struct reservation_object_list { > * @seq: sequence count for managing RCU read-side synchronization > * @fence_excl: the exclusive fence, if there is one currently > * @fence: list of current shared fences > - * @staged: staged copy of shared fences for RCU updates > */ > struct reservation_object { > struct ww_mutex lock; > @@ -76,7 +75,6 @@ struct reservation_object { > > struct dma_fence __rcu *fence_excl; > struct reservation_object_list __rcu *fence; > - struct reservation_object_list *staged; > }; > > #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base) > @@ -95,7 +93,6 @@ reservation_object_init(struct reservation_object *obj) > __seqcount_init(&obj->seq, reservation_seqcount_string, &reservation_seqcount_class); > RCU_INIT_POINTER(obj->fence, NULL); > RCU_INIT_POINTER(obj->fence_excl, NULL); > - obj->staged = NULL; > } > > /** > @@ -124,7 +121,6 @@ reservation_object_fini(struct reservation_object *obj) > > kfree(fobj); > } > - kfree(obj->staged); > > ww_mutex_destroy(&obj->lock); > }

7 years, 11 months

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig