Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

32 participants
5827 discussions

Re: [Linaro-mm-sig] Fwd: [PATCHv3] gpu: ion: Add support for sharing buffers with dma buf kernel handles

by Johan Mossberg

Hi Rebecca, Is this patch going to be merged to the ion mainline? We (ST-Ericsson) are going to include it in our ion either way as we need it but if you're not planning on including it in mainline ion I'll use the previous version that did not change the api. Don't want to diverge from the standard api unless absolutely necessary. Johan On 01/11/2013 12:38 PM, Johan Mossberg wrote: > Currently ion can only share buffers with dma buf fd:s. Fd:s can not be > used inside the kernel as they are process specific so support for > sharing buffers with dma buf kernel handles is needed to support kernel > only use cases. An example use case could be a GPU driver using ion > that wants to share its output buffers with a 3d party display > controller driver supporting dma buf. > > Signed-off-by: Johan Mossberg<johan.mossberg(a)stericsson.com> > --- > drivers/gpu/ion/ion.c | 26 ++++++++++++++++++++------ > include/linux/ion.h | 12 ++++++++++-- > 2 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c > index 0fc02fd..8fd61b3 100644 > --- a/drivers/gpu/ion/ion.c > +++ b/drivers/gpu/ion/ion.c > @@ -949,19 +949,19 @@ struct dma_buf_ops dma_buf_ops = { > .kunmap = ion_dma_buf_kunmap, > }; > > -int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle) > +struct dma_buf *ion_share_dma_buf(struct ion_client *client, > + struct ion_handle *handle) > { > struct ion_buffer *buffer; > struct dma_buf *dmabuf; > bool valid_handle; > - int fd; > > mutex_lock(&client->lock); > valid_handle = ion_handle_validate(client, handle); > mutex_unlock(&client->lock); > if (!valid_handle) { > WARN(1, "%s: invalid handle passed to share.\n", __func__); > - return -EINVAL; > + return ERR_PTR(-EINVAL); > } > > buffer = handle->buffer; > @@ -969,15 +969,29 @@ int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle) > dmabuf = dma_buf_export(buffer, &dma_buf_ops, buffer->size, O_RDWR); > if (IS_ERR(dmabuf)) { > ion_buffer_put(buffer); > - return PTR_ERR(dmabuf); > + return dmabuf; > } > + > + return dmabuf; > +} > +EXPORT_SYMBOL(ion_share_dma_buf); > + > +int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) > +{ > + struct dma_buf *dmabuf; > + int fd; > + > + dmabuf = ion_share_dma_buf(client, handle); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > fd = dma_buf_fd(dmabuf, O_CLOEXEC); > if (fd < 0) > dma_buf_put(dmabuf); > > return fd; > } > -EXPORT_SYMBOL(ion_share_dma_buf); > +EXPORT_SYMBOL(ion_share_dma_buf_fd); > > struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd) > { > @@ -1086,7 +1100,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > if (copy_from_user(&data, (void __user *)arg, sizeof(data))) > return -EFAULT; > - data.fd = ion_share_dma_buf(client, data.handle); > + data.fd = ion_share_dma_buf_fd(client, data.handle); > if (copy_to_user((void __user *)arg, &data, sizeof(data))) > return -EFAULT; > if (data.fd < 0) > diff --git a/include/linux/ion.h b/include/linux/ion.h > index a55d11f..e2503e9 100644 > --- a/include/linux/ion.h > +++ b/include/linux/ion.h > @@ -214,11 +214,19 @@ void *ion_map_kernel(struct ion_client *client, struct ion_handle *handle); > void ion_unmap_kernel(struct ion_client *client, struct ion_handle *handle); > > /** > - * ion_share_dma_buf() - given an ion client, create a dma-buf fd > + * ion_share_dma_buf() - share buffer as dma-buf > * @client: the client > * @handle: the handle > */ > -int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle); > +struct dma_buf *ion_share_dma_buf(struct ion_client *client, > + struct ion_handle *handle); > + > +/** > + * ion_share_dma_buf_fd() - given an ion client, create a dma-buf fd > + * @client: the client > + * @handle: the handle > + */ > +int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle); > > /** > * ion_import_dma_buf() - given an dma-buf fd from the ion exporter get handle > -- > 1.8.0 > > > _______________________________________________ > Linaro-mm-sig mailing list > Linaro-mm-sig(a)lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-mm-sig > >

13 years

Support for high-order pages on ARM

by Mitchel Humpherys

Hi, While working with high-order page allocations (using `alloc_pages') I've encountered some issues* with certain APIs and wanted to get a better understanding of support for those APIs with high-order pages on ARM. In short, I'm trying to give userspace access to those pages by using `vm_insert_page' in an mmap handler. Without further ado, some questions: o vm_insert_page doesn't seem to work with high-order pages (it eventually calls __flush_dcache_page which assumes pages of size PAGE_SIZE). Is this analysis correct or am I missing something? Things work fine if I use `remap_pfn_range' instead of `vm_insert_page'. Things also seem to work if I use `vm_insert_page' with an array of struct page * of size PAGE_SIZE (derived from the high-order pages by picking out the PAGE_SIZE pages with nth_page)... o There's a comment in __dma_alloc (dma-alloc.c) to the effect that __GFP_COMP is not supported on ARM. Is this true? The commit that introduced this comment (ea2e7057) was actually ported from avr32 (3611553ef) so I'm curious about the basis for this claim... I've tried pages of order 8 and order 4. The gfp flags I'm passing to `alloc_pages' are (GFP_KERNEL | __GFP_HIGHMEM | __GFP_COMP). Thanks! * Some issues = in userspace mmap the buffer whose underlying mmap handler is the one mentioned above, memset that to something and then immediately check that the bytes are equal to whatever we just memset. (With huge pages and vm_insert_page this test fails). -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

13 years

[PATCH] lib/scatterlist: add simple page iterator

by Imre Deak

Add an iterator to walk through a scatter list a page at a time starting at a specific page offset. As opposed to the mapping iterator this is meant to be small, performing well even in simple loops like collecting all pages on the scatterlist into an array or setting up an iommu table based on the pages' DMA address. Signed-off-by: Imre Deak <imre.deak(a)intel.com> --- include/linux/scatterlist.h | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) [ Resending with proper email addresses. ] diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 4bd6c06..d22851c 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -231,6 +231,54 @@ size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, */ #define SG_MAX_SINGLE_ALLOC (PAGE_SIZE / sizeof(struct scatterlist)) +struct sg_page_iter { + struct scatterlist *sg; + int sg_pgoffset; + struct page *page; +}; + +static inline int +sg_page_cnt(struct scatterlist *sg) +{ + BUG_ON(sg->offset || sg->length & ~PAGE_MASK); + + return sg->length >> PAGE_SHIFT; +} + +static inline void +sg_page_iter_next(struct sg_page_iter *iter) +{ + while (iter->sg && iter->sg_pgoffset >= sg_page_cnt(iter->sg)) { + iter->sg_pgoffset -= sg_page_cnt(iter->sg); + iter->sg = sg_next(iter->sg); + } + + if (iter->sg) { + iter->page = nth_page(sg_page(iter->sg), iter->sg_pgoffset); + iter->sg_pgoffset++; + } +} + +static inline void +sg_page_iter_start(struct sg_page_iter *iter, struct scatterlist *sglist, + unsigned long pgoffset) +{ + iter->sg = sglist; + iter->sg_pgoffset = pgoffset; + iter->page = NULL; + + sg_page_iter_next(iter); +} + +/* + * Simple sg page iterator, starting off at the given page offset. Each entry + * on the sglist must start at offset 0 and can contain only full pages. + * iter->page will point to the current page, iter->sg_pgoffset to the page + * offset within the sg holding that page. + */ +#define for_each_sg_page(sglist, iter, pgoffset) \ + for (sg_page_iter_start((iter), (sglist), (pgoffset)); \ + (iter)->sg; sg_page_iter_next(iter)) /* * Mapping sg iterator -- 1.7.9.5

13 years

[PATCH] gpu: ion: Use memblock_remove when reserving carveout memory

by Johan Mossberg

__arm_ioremap, which is used by the carveout heap, refuses to map memory already mapped as lowmem, probably to avoid having multiple mappings with different cache settings pointing at the same memory. By using memblock_remove rather than memblock_reserve the carveout heap's memory will not be included in lowmem i.e no problem. Signed-off-by: Johan Mossberg <johan.mossberg(a)stericsson.com> --- drivers/gpu/ion/ion.c | 35 +++++++++++++++++++---------------- 1 files changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index 0fc02fd..b8b43bb 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -1323,29 +1323,32 @@ void __init ion_reserve(struct ion_platform_data *data) int i; for (i = 0; i < data->nr; i++) { + int ret; + if (data->heaps[i].size == 0) continue; if (data->heaps[i].base == 0) { - phys_addr_t paddr; - paddr = memblock_alloc_base(data->heaps[i].size, - data->heaps[i].align, - MEMBLOCK_ALLOC_ANYWHERE); - if (!paddr) { - pr_err("%s: error allocating memblock for " - "heap %d\n", - __func__, i); + data->heaps[i].base = memblock_find_in_range_node( + 0, + MEMBLOCK_ALLOC_ANYWHERE, + data->heaps[i].size, + data->heaps[i].align, + MAX_NUMNODES); + if (data->heaps[i].base == 0) { + pr_err("%s: error allocating memblock for heap %d\n", + __func__, data->heaps[i].id); continue; } - data->heaps[i].base = paddr; - } else { - int ret = memblock_reserve(data->heaps[i].base, - data->heaps[i].size); - if (ret) - pr_err("memblock reserve of %x@%lx failed\n", - data->heaps[i].size, - data->heaps[i].base); } + + ret = memblock_remove(data->heaps[i].base, data->heaps[i].size); + if (ret < 0) { + pr_err("%s: could not remove heap %d memory from kernel\n", + __func__, data->heaps[i].id); + continue; + } + pr_info("%s: %s reserved base %lx size %d\n", __func__, data->heaps[i].name, data->heaps[i].base, -- 1.7.4.3

13 years

Re: [Linaro-mm-sig] [Intel-gfx] [PATCH 1/4] drm: add helper to walk a dma scatter list a page at a time

by Daniel Vetter

Hi Imre! On Sat, Feb 09, 2013 at 05:27:33PM +0200, Imre Deak wrote: > Add a helper to walk through a scatter list a page at a time. Needed by > upcoming patches fixing the scatter list walking logic in the i915 driver. Nice patch, but I think this would make a rather nice addition to the common scatterlist api in scatterlist.h, maybe called sg_page_iter. There's already another helper which does cpu mappings, but it has a different use-case (gives you the page mapped, which we don't neeed and can cope with not page-aligned sg tables). With dma-buf using sg tables I expect more users of such a sg page iterator to pop up. Most possible users of this will hang around on linaro-mm-sig, so please also cc that besides the usual suspects. Cheers, Daniel > > Signed-off-by: Imre Deak <imre.deak(a)intel.com> > --- > include/drm/drmP.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index fad21c9..0c0c213 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, void *data, > extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * request); > extern int drm_sg_free(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +struct drm_sg_iter { > + struct scatterlist *sg; > + int sg_offset; Imo using sg_pfn_offset (i.e. sg_offset >> PAGE_SIZE) would make it clearer that this is all about iterating page-aligned sg tables. > + struct page *page; > +}; > + > +static inline int > +__drm_sg_iter_seek(struct drm_sg_iter *iter) > +{ > + while (iter->sg && iter->sg_offset >= iter->sg->length) { > + iter->sg_offset -= iter->sg->length; > + iter->sg = sg_next(iter->sg); And adding a WARN_ON(sg->legnth & ~PAGE_MASK); here would enforce that. > + } > + > + return iter->sg ? 0 : -1; > +} > + > +static inline struct page * > +drm_sg_iter_next(struct drm_sg_iter *iter) > +{ > + struct page *page; > + > + if (__drm_sg_iter_seek(iter)) > + return NULL; > + > + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); > + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; > + > + return page; > +} > + > +static inline struct page * > +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, > + unsigned long offset) > +{ > + iter->sg = sg; > + iter->sg_offset = offset; > + > + return drm_sg_iter_next(iter); > +} > + > +#define drm_for_each_sg_page(iter, sg, pgoffset) \ > + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ > + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) Again, for the initialization I'd go with page numbers, not an offset in bytes. > > /* ATI PCIGART support (ati_pcigart.h) */ > extern int drm_ati_pcigart_init(struct drm_device *dev, > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx(a)lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

13 years

[PATCH] gpu: ion: Fix a kmalloc leak in carveout heap

by Nishanth Peethambaran

Free the sg_table allocated. Signed-off-by: Nishanth Peethambaran <nishanth(a)broadcom.com> --- drivers/gpu/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/ion/ion_carveout_heap.c b/drivers/gpu/ion/ion_carveout_heap.c index ce8d311..e8164a6 100644 --- a/drivers/gpu/ion/ion_carveout_heap.c +++ b/drivers/gpu/ion/ion_carveout_heap.c @@ -107,6 +107,7 @@ void ion_carveout_heap_unmap_dma(struct ion_heap *heap, struct ion_buffer *buffer) { sg_free_table(buffer->sg_table); + kfree(buffer->sg_table); } void *ion_carveout_heap_map_kernel(struct ion_heap *heap, -- 1.7.9.5

13 years

[PATCH] gpu: ion: Use safe version of list iterator

by Nishanth Peethambaran

On allocation or kmalloc failure in system heap allocate, the exit path iterates over the allocated page infos and frees the allocated pages and page info. The same page info structure is used as loop iterator. Use the safe version of list iterator. Signed-off-by: Nishanth Peethambaran <nishanth(a)broadcom.com> --- drivers/gpu/ion/ion_system_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/ion/ion_system_heap.c b/drivers/gpu/ion/ion_system_heap.c index c1061a8..d079e2b 100644 --- a/drivers/gpu/ion/ion_system_heap.c +++ b/drivers/gpu/ion/ion_system_heap.c @@ -200,7 +200,7 @@ static int ion_system_heap_allocate(struct ion_heap *heap, err1: kfree(table); err: - list_for_each_entry(info, &pages, list) { + list_for_each_entry_safe(info, tmp_info, &pages, list) { free_buffer_page(sys_heap, buffer, info->page, info->order); kfree(info); } -- 1.7.9.5

13 years

[PATCH] ARM: DMA-mapping: fix memory leak in IOMMU dma-mapping implementation

by Marek Szyprowski

This patch removes page_address() usage in IOMMU-aware dma-mapping implementation and replaced it with direct use of the cpu virtual address provided by the caller. page_address() returned incorrect address for pages remapped in atomic pool, what caused memory leak. Reported-by: Hiroshi Doyu <hdoyu(a)nvidia.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> Tested-by: Hiroshi Doyu <hdoyu(a)nvidia.com> --- arch/arm/mm/dma-mapping.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..8c38378 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1255,11 +1255,11 @@ err_mapping: return NULL; } -static void __iommu_free_atomic(struct device *dev, struct page **pages, +static void __iommu_free_atomic(struct device *dev, void *cpu_addr, dma_addr_t handle, size_t size) { __iommu_remove_mapping(dev, handle, size); - __free_from_pool(page_address(pages[0]), size); + __free_from_pool(cpu_addr, size); } static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, @@ -1342,7 +1342,7 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, } if (__in_atomic_pool(cpu_addr, size)) { - __iommu_free_atomic(dev, pages, handle, size); + __iommu_free_atomic(dev, cpu_addr, handle, size); return; } -- 1.7.9.5

13 years

ashmem - mmap deadlock with mmap_sem & ashmem_mutex

by Shankar Brahadeeswaran

Hi, I'm working on Android Linux Kernel Vesion 3.0.15 and seeing a "deadlock" in the ashmem driver, while handling mmap request. I seek your support in finding the correct fix. The locks that involved in the dead lock are 1) mm->mmap_sem 2) ashmem_mutex The following is the sequence of events that leads to the deadlock. There are two threads A and B that belong to the same process (system_server) and hence share the mm struct. A1) In the A's context an mmap system call is made with an fd of ashmem A2) The system call sys_mmap_pgoff acquires the mmap_sem of the "mm" and sleeps before calling the .mmap of ashmem i.e before calling ashmem_mmap Now the thread B runs and proceeds to do the following B1) In the B's context ashmem ioctl with option ASHMEM_SET_NAME is called. B2) Now the code proceeds to acquire the ashmem_mutex and performs a "copy_from_user" B3) copy_from_user raises a valid exception to copy the data from user space and proceeds to handle it gracefully, do_DataAbort --> do_page_fault B4) In do_page_fault it finds that the mm->mmap_sem is not available (Note A & B share the mm) since A has it and sleeps Now the thread A runs again A3) It proceeds to call ashmem_mmap and tries to acquired ashmem_mutex, which is not available (is with B) and sleeps. Now A has acquired mmap_sem and waits for B to release ashmem_mutex B has acquired ashmem_mutex and waits for the mmap_sem to be available, which is held by A This creates a dead lock in the system. I'm not sure how to use these locks in such a way as to prevent this scenario. Any suggestions would be of great help. Workaround: One possible work around is to replace the mutex_lock call made in the ashmem_mmap with mutex_trylock and if it fails, wait for few milliseconds and try back for few iterations and finally give up after few iterations. This will bring the system out deadlock if this scneario happens. I myself feel that this suggestion is not clean. But I'm unable to think of anything. Is there any suggestion to avoid this scenario. Warm Regards, Shankar

13 years

[PATCH 0/2] ARM: dma-mapping: add highmem support for coherent allocation

by Marek Szyprowski

Hello, This is the last missing piece to let us efficiently use large DMA buffers on systems with lots of memory, which have support for himem enabled. The first patch adds support for CMA regions placed in high memory zones, the second one also enables allocations of individual pages from high memory zone for IOMMU-mapped devices. Those two changes let us to significantly save low memory for other tasks. Best regards Marek Szyprowski Samsung Poland R&D Center Patch summary: Marek Szyprowski (2): ARM: dma-mapping: add support for CMA regions placed in highmem zone ARM: dma-mapping: use himem for DMA buffers for IOMMU-mapped devices arch/arm/mm/dma-mapping.c | 70 +++++++++++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 18 deletions(-) -- 1.7.9.5

13 years

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig