Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

10 participants
2527 discussions

Re: [PATCH v2 0/5] Introduce DMA_HEAP_ALLOC_AND_READ_FILE heap flag

by Daniel Vetter

On Thu, Aug 01, 2024 at 10:53:45AM +0800, Huan Yang wrote: > > 在 2024/8/1 4:46, Daniel Vetter 写道: > > On Tue, Jul 30, 2024 at 08:04:04PM +0800, Huan Yang wrote: > > > 在 2024/7/30 17:05, Huan Yang 写道: > > > > 在 2024/7/30 16:56, Daniel Vetter 写道: > > > > > [????????? daniel.vetter(a)ffwll.ch ????????? > > > > > https://aka.ms/LearnAboutSenderIdentification?????????????] > > > > > > > > > > On Tue, Jul 30, 2024 at 03:57:44PM +0800, Huan Yang wrote: > > > > > > UDMA-BUF step: > > > > > > 1. memfd_create > > > > > > 2. open file(buffer/direct) > > > > > > 3. udmabuf create > > > > > > 4. mmap memfd > > > > > > 5. read file into memfd vaddr > > > > > Yeah this is really slow and the worst way to do it. You absolutely want > > > > > to start _all_ the io before you start creating the dma-buf, ideally > > > > > with > > > > > everything running in parallel. But just starting the direct I/O with > > > > > async and then creating the umdabuf should be a lot faster and avoid > > > > That's greate, Let me rephrase that, and please correct me if I'm wrong. > > > > > > > > UDMA-BUF step: > > > > 1. memfd_create > > > > 2. mmap memfd > > > > 3. open file(buffer/direct) > > > > 4. start thread to async read > > > > 3. udmabuf create > > > > > > > > With this, can improve > > > I just test with it. Step is: > > > > > > UDMA-BUF step: > > > 1. memfd_create > > > 2. mmap memfd > > > 3. open file(buffer/direct) > > > 4. start thread to async read > > > 5. udmabuf create > > > > > > 6 . join wait > > > > > > 3G file read all step cost 1,527,103,431ns, it's greate. > > Ok that's almost the throughput of your patch set, which I think is close > > enough. The remaining difference is probably just the mmap overhead, not > > sure whether/how we can do direct i/o to an fd directly ... in principle > > it's possible for any file that uses the standard pagecache. > > Yes, for mmap, IMO, now that we get all folios and pin it. That's mean all > pfn it's got when udmabuf created. > > So, I think mmap with page fault is helpless for save memory but increase > the mmap access cost.(maybe can save a little page table's memory) > > I want to offer a patchset to remove it and more suitable for folios > operate(And remove unpin list). And contains some fix patch. > > I'll send it when I test it's good. > > > About fd operation for direct I/O, maybe use sendfile or copy_file_range? > > sendfile base pipe buffer, it's low performance when I test is. > > copy_file_range can't work due to it's not the same file system. > > So, I can't find other way to do it. Can someone give some suggestions? Yeah direct I/O to pagecache without an mmap might be too niche to be supported. Maybe io_uring has something, but I guess as unlikely as anything else. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

4 months, 2 weeks

Re: [PATCH 0/5] udmbuf bug fix and some improvements

by Michel Dänzer

On 2024-08-01 20:32, Kasireddy, Vivek wrote: > Hi Huan, > >> This patchset attempts to fix some errors in udmabuf and remove the >> upin_list structure. >> >> Some of this fix just gather the patches which I upload before. >> >> Patch1 >> === >> Try to remove page fault mmap and direct map it. >> Due to current udmabuf has already obtained and pinned the folio >> upon completion of the creation.This means that the physical memory has >> already been acquired, rather than being accessed dynamically. The >> current page fault method only saves some page table memory. >> >> As a result, the page fault mechanism has lost its purpose as a demanding >> page. Due to the fact that page fault requires trapping into kernel mode >> and filling in when accessing the corresponding virtual address in mmap, >> this means that user mode access to virtual addresses needs to trap into >> kernel mode. >> >> Therefore, when creating a large size udmabuf, this represents a >> considerable overhead. > Just want to mention that for the main use-case the udmabuf driver is designed for, > (sharing Qemu Guest FB with Host for GPU DMA), udmabufs are not created very > frequently. And, I think providing CPU access via mmap is just a backup, mainly > intended for debugging purposes. FYI, Mesa now uses udmabuf for supporting dma-bufs with software rendering. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and Xwayland developer

4 months, 3 weeks

Re: [PATCH 4/5] udmabuf: add get_sg_table helper function

by Christian König

Am 01.08.24 um 12:45 schrieb Huan Yang: > Currently, there are three duplicate pieces of code that retrieve > sg_table and update uduf->sg. > > Since the sgt is used to populate the page in both mmap and vmap.It is > necessary to ensure that ubuf->sg is set correctly. That is a really bad idea. Why are sg tables used to populated the page tables? Regards, Christian. > > This patch add a helper function, if ubuf->sg exist, just return it. > Or else, try alloc a new sgt, and cmpxchg to set it. > > When the swap fails, it means that another process has set sg correctly. > Therefore, we reuse the new sg. If trigger by device, need invoke map to > sync it. > > Signed-off-by: Huan Yang <link(a)vivo.com> > --- > drivers/dma-buf/udmabuf.c | 60 ++++++++++++++++++++++++++++----------- > 1 file changed, 43 insertions(+), 17 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 7ed532342d7f..677ebb2d462f 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -38,8 +38,9 @@ struct udmabuf_folio { > struct list_head list; > }; > > -static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > - enum dma_data_direction direction); > +static struct sg_table *udmabuf_get_sg_table(struct device *dev, > + struct dma_buf *buf, > + enum dma_data_direction direction); > > static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) > { > @@ -52,12 +53,9 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) > if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) > return -EINVAL; > > - if (!table) { > - table = get_sg_table(NULL, buf, 0); > - if (IS_ERR(table)) > - return PTR_ERR(table); > - ubuf->sg = table; > - } > + table = udmabuf_get_sg_table(NULL, buf, 0); > + if (IS_ERR(table)) > + return PTR_ERR(table); > > for_each_sgtable_page(table, &piter, vma->vm_pgoff) { > struct page *page = sg_page_iter_page(&piter); > @@ -84,12 +82,9 @@ static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > > dma_resv_assert_held(buf->resv); > > - if (!sg) { > - sg = get_sg_table(NULL, buf, 0); > - if (IS_ERR(sg)) > - return PTR_ERR(sg); > - ubuf->sg = sg; > - } > + sg = udmabuf_get_sg_table(NULL, buf, 0); > + if (IS_ERR(sg)) > + return PTR_ERR(sg); > > pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); > if (!pages) > @@ -154,6 +149,39 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > return ERR_PTR(ret); > } > > +static struct sg_table *udmabuf_get_sg_table(struct device *dev, > + struct dma_buf *buf, > + enum dma_data_direction direction) > +{ > + struct udmabuf *ubuf = buf->priv; > + struct sg_table *sg = READ_ONCE(ubuf->sg); > + int ret = 0; > + > + if (sg) > + return sg; > + > + sg = get_sg_table(dev, buf, direction); > + if (IS_ERR(sg)) > + return sg; > + > + // Success update ubuf's sg, just return. > + if (!cmpxchg(&ubuf->sg, NULL, sg)) > + return sg; > + > + // use the new sg table. > + sg_free_table(sg); > + kfree(sg); > + sg = READ_ONCE(ubuf->sg); > + > + if (dev) > + ret = dma_map_sgtable(dev, sg, direction, 0); > + > + if (ret < 0) > + return ERR_PTR(ret); > + > + return sg; > +} > + > static void put_sg_table(struct device *dev, struct sg_table *sg, > enum dma_data_direction direction) > { > @@ -230,12 +258,10 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, > return 0; > } > > - sg = get_sg_table(dev, buf, direction); > + sg = udmabuf_get_sg_table(dev, buf, direction); > if (IS_ERR(sg)) > return PTR_ERR(sg); > > - ubuf->sg = sg; > - > return 0; > } >

4 months, 3 weeks

Re: [PATCH 3/5] udmabuf: fix vmap_udmabuf error page set

by Christian König

Am 01.08.24 um 12:45 schrieb Huan Yang: > Currently vmap_udmabuf set page's array by each folio. > But, ubuf->folios is only contain's the folio's head page. > > That mean we repeatedly mapped the folio head page to the vmalloc area. > > This patch fix it, set each folio's page correct, so that pages array > contains right page, and then map into vmalloc area > > Signed-off-by: Huan Yang <link(a)vivo.com> > --- > drivers/dma-buf/udmabuf.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index a915714c5dce..7ed532342d7f 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -77,18 +77,27 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) > static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > { > struct udmabuf *ubuf = buf->priv; > - struct page **pages; > + struct page **pages, **tmp; > + struct sg_table *sg = ubuf->sg; > + struct sg_page_iter piter; > void *vaddr; > - pgoff_t pg; > > dma_resv_assert_held(buf->resv); > > + if (!sg) { > + sg = get_sg_table(NULL, buf, 0); > + if (IS_ERR(sg)) > + return PTR_ERR(sg); > + ubuf->sg = sg; > + } > + > pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); > if (!pages) > return -ENOMEM; > + tmp = pages; > > - for (pg = 0; pg < ubuf->pagecount; pg++) > - pages[pg] = &ubuf->folios[pg]->page; > + for_each_sgtable_page(sg, &piter, 0) > + *tmp++ = sg_page_iter_page(&piter); Again don't abuse the sg table for that! Regards, Christian. > > vaddr = vm_map_ram(pages, ubuf->pagecount, -1); > kvfree(pages);

4 months, 3 weeks

Re: [PATCH 2/5] udmabuf: change folios array from kmalloc to kvmalloc

by Christian König

Am 01.08.24 um 12:45 schrieb Huan Yang: > When PAGE_SIZE 4096, MAX_PAGE_ORDER 10, 64bit machine, > page_alloc only support 4MB. > If above this, trigger this warn and return NULL. > > udmabuf can change size limit, if change it to 3072(3GB), and then alloc > 3GB udmabuf, will fail create. > > [ 4080.876581] ------------[ cut here ]------------ > [ 4080.876843] WARNING: CPU: 3 PID: 2015 at mm/page_alloc.c:4556 __alloc_pages+0x2c8/0x350 > [ 4080.878839] RIP: 0010:__alloc_pages+0x2c8/0x350 > [ 4080.879470] Call Trace: > [ 4080.879473] <TASK> > [ 4080.879473] ? __alloc_pages+0x2c8/0x350 > [ 4080.879475] ? __warn.cold+0x8e/0xe8 > [ 4080.880647] ? __alloc_pages+0x2c8/0x350 > [ 4080.880909] ? report_bug+0xff/0x140 > [ 4080.881175] ? handle_bug+0x3c/0x80 > [ 4080.881556] ? exc_invalid_op+0x17/0x70 > [ 4080.881559] ? asm_exc_invalid_op+0x1a/0x20 > [ 4080.882077] ? udmabuf_create+0x131/0x400 > > Because MAX_PAGE_ORDER, kmalloc can max alloc 4096 * (1 << 10), 4MB > memory, each array entry is pointer(8byte), so can save 524288 pages(2GB). > > Further more, costly order(order 3) may not be guaranteed that it can be > applied for, due to fragmentation. > > This patch change udmabuf array use kvmalloc_array, this can fallback > alloc into vmalloc, which can guarantee allocation for any size and does > not affect the performance of kmalloc allocations. > > Signed-off-by: Huan Yang <link(a)vivo.com> Acked-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/udmabuf.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index d69aeada7367..a915714c5dce 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -83,7 +83,7 @@ static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > > dma_resv_assert_held(buf->resv); > > - pages = kmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); > + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); > if (!pages) > return -ENOMEM; > > @@ -91,7 +91,7 @@ static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > pages[pg] = &ubuf->folios[pg]->page; > > vaddr = vm_map_ram(pages, ubuf->pagecount, -1); > - kfree(pages); > + kvfree(pages); > if (!vaddr) > return -EINVAL; > > @@ -203,8 +203,8 @@ static void release_udmabuf(struct dma_buf *buf) > put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); > > unpin_all_folios(&ubuf->unpin_list); > - kfree(ubuf->offsets); > - kfree(ubuf->folios); > + kvfree(ubuf->offsets); > + kvfree(ubuf->folios); > kfree(ubuf); > } > > @@ -330,14 +330,14 @@ static long udmabuf_create(struct miscdevice *device, > if (!ubuf->pagecount) > goto err; > > - ubuf->folios = kmalloc_array(ubuf->pagecount, sizeof(*ubuf->folios), > - GFP_KERNEL); > + ubuf->folios = kvmalloc_array(ubuf->pagecount, sizeof(*ubuf->folios), > + GFP_KERNEL); > if (!ubuf->folios) { > ret = -ENOMEM; > goto err; > } > - ubuf->offsets = kcalloc(ubuf->pagecount, sizeof(*ubuf->offsets), > - GFP_KERNEL); > + ubuf->offsets = > + kvcalloc(ubuf->pagecount, sizeof(*ubuf->offsets), GFP_KERNEL); > if (!ubuf->offsets) { > ret = -ENOMEM; > goto err; > @@ -351,7 +351,7 @@ static long udmabuf_create(struct miscdevice *device, > goto err; > > pgcnt = list[i].size >> PAGE_SHIFT; > - folios = kmalloc_array(pgcnt, sizeof(*folios), GFP_KERNEL); > + folios = kvmalloc_array(pgcnt, sizeof(*folios), GFP_KERNEL); > if (!folios) { > ret = -ENOMEM; > goto err; > @@ -361,7 +361,7 @@ static long udmabuf_create(struct miscdevice *device, > ret = memfd_pin_folios(memfd, list[i].offset, end, > folios, pgcnt, &pgoff); > if (ret <= 0) { > - kfree(folios); > + kvfree(folios); > if (!ret) > ret = -EINVAL; > goto err; > @@ -390,7 +390,7 @@ static long udmabuf_create(struct miscdevice *device, > } > } > > - kfree(folios); > + kvfree(folios); > fput(memfd); > memfd = NULL; > } > @@ -406,8 +406,8 @@ static long udmabuf_create(struct miscdevice *device, > if (memfd) > fput(memfd); > unpin_all_folios(&ubuf->unpin_list); > - kfree(ubuf->offsets); > - kfree(ubuf->folios); > + kvfree(ubuf->offsets); > + kvfree(ubuf->folios); > kfree(ubuf); > return ret; > }

4 months, 3 weeks

Re: [PATCH 1/5] udmabuf: cancel mmap page fault, direct map it

by Christian König

Am 01.08.24 um 12:45 schrieb Huan Yang: > The current udmabuf mmap uses a page fault mechanism to populate the vma. > > However, the current udmabuf has already obtained and pinned the folio > upon completion of the creation.This means that the physical memory has > already been acquired, rather than being accessed dynamically. The > current page fault method only saves some page table memory. > > As a result, the page fault mechanism has lost its purpose as a demanding > page. Due to the fact that page fault requires trapping into kernel mode > and filling in when accessing the corresponding virtual address in mmap, > this means that user mode access to virtual addresses needs to trap into > kernel mode. > > Therefore, when creating a large size udmabuf, this represents a > considerable overhead. > > Therefore, the current patch removes the page fault method of mmap and > instead fills it directly when mmap is triggered. > > Signed-off-by: Huan Yang <link(a)vivo.com> > --- > drivers/dma-buf/udmabuf.c | 70 ++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 31 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 047c3cd2ceff..d69aeada7367 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -38,36 +38,39 @@ struct udmabuf_folio { > struct list_head list; > }; > > -static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) > -{ > - struct vm_area_struct *vma = vmf->vma; > - struct udmabuf *ubuf = vma->vm_private_data; > - pgoff_t pgoff = vmf->pgoff; > - unsigned long pfn; > - > - if (pgoff >= ubuf->pagecount) > - return VM_FAULT_SIGBUS; > - > - pfn = folio_pfn(ubuf->folios[pgoff]); > - pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; > - > - return vmf_insert_pfn(vma, vmf->address, pfn); > -} > - > -static const struct vm_operations_struct udmabuf_vm_ops = { > - .fault = udmabuf_vm_fault, > -}; > +static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > + enum dma_data_direction direction); > > static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) > { > struct udmabuf *ubuf = buf->priv; > + struct sg_table *table = ubuf->sg; > + unsigned long addr = vma->vm_start; > + struct sg_page_iter piter; > + int ret; > > if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0) > return -EINVAL; > > - vma->vm_ops = &udmabuf_vm_ops; > - vma->vm_private_data = ubuf; > - vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP); > + if (!table) { > + table = get_sg_table(NULL, buf, 0); > + if (IS_ERR(table)) > + return PTR_ERR(table); > + ubuf->sg = table; > + } > + > + for_each_sgtable_page(table, &piter, vma->vm_pgoff) { That might not work correctly. We intentionally remove the pages from the sgtable when it is shared between devices. Additional to that the sgtable is *not* a page container, but rather a DMA address container. So that here is also a rather bad idea from the design side. Regards, Christian. > + struct page *page = sg_page_iter_page(&piter); > + > + ret = remap_pfn_range(vma, addr, page_to_pfn(page), PAGE_SIZE, > + vma->vm_page_prot); > + if (ret) > + return ret; > + addr += PAGE_SIZE; > + if (addr >= vma->vm_end) > + return 0; > + } > + > return 0; > } > > @@ -126,6 +129,10 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > sg_set_folio(sgl, ubuf->folios[i], PAGE_SIZE, > ubuf->offsets[i]); > > + // if dev is NULL, no need to sync. > + if (!dev) > + return sg; > + > ret = dma_map_sgtable(dev, sg, direction, 0); > if (ret < 0) > goto err_map; > @@ -206,20 +213,21 @@ static int begin_cpu_udmabuf(struct dma_buf *buf, > { > struct udmabuf *ubuf = buf->priv; > struct device *dev = ubuf->device->this_device; > - int ret = 0; > + struct sg_table *sg; > > - if (!ubuf->sg) { > - ubuf->sg = get_sg_table(dev, buf, direction); > - if (IS_ERR(ubuf->sg)) { > - ret = PTR_ERR(ubuf->sg); > - ubuf->sg = NULL; > - } > - } else { > + if (ubuf->sg) { > dma_sync_sg_for_cpu(dev, ubuf->sg->sgl, ubuf->sg->nents, > direction); > + return 0; > } > > - return ret; > + sg = get_sg_table(dev, buf, direction); > + if (IS_ERR(sg)) > + return PTR_ERR(sg); > + > + ubuf->sg = sg; > + > + return 0; > } > > static int end_cpu_udmabuf(struct dma_buf *buf,

4 months, 3 weeks

Re: [PATCH] udmabuf: use kmem_cache to alloc udmabuf folio

by Daniel Vetter

On Wed, Jul 31, 2024 at 11:34:49AM +0800, Huan Yang wrote: > The current udmabuf_folio contains a list_head and the corresponding > folio pointer, with a size of 24 bytes. udmabuf_folio uses kmalloc to > allocate memory. > > However, kmalloc is a public pool, starting from 64 bytes. This means > that each udmabuf_folio allocation will waste 40 bytes. > > Considering that each udmabuf creates a folio corresponding to a > udmabuf_folio, the wasted memory can be significant in the case of > memory fragmentation. > > Furthermore, if udmabuf is frequently used, the allocation and > deallocation of udmabuf_folio will also be frequent. > > Therefore, this patch adds a kmem_cache dedicated to the allocation and > deallocation of udmabuf_folio.This is expected to improve the > performance of allocation and deallocation within the expected range, > while also avoiding memory waste. > > Signed-off-by: Huan Yang <link(a)vivo.com> > --- > drivers/dma-buf/udmabuf.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 047c3cd2ceff..db4de8c745ce 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -24,6 +24,8 @@ static int size_limit_mb = 64; > module_param(size_limit_mb, int, 0644); > MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is 64."); > > +static struct kmem_cache *udmabuf_folio_cachep; > + > struct udmabuf { > pgoff_t pagecount; > struct folio **folios; > @@ -169,7 +171,7 @@ static void unpin_all_folios(struct list_head *unpin_list) > unpin_folio(ubuf_folio->folio); > > list_del(&ubuf_folio->list); > - kfree(ubuf_folio); > + kmem_cache_free(udmabuf_folio_cachep, ubuf_folio); > } > } > > @@ -178,7 +180,7 @@ static int add_to_unpin_list(struct list_head *unpin_list, > { > struct udmabuf_folio *ubuf_folio; > > - ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL); > + ubuf_folio = kmem_cache_alloc(udmabuf_folio_cachep, GFP_KERNEL); > if (!ubuf_folio) > return -ENOMEM; > > @@ -492,10 +494,20 @@ static int __init udmabuf_dev_init(void) > if (ret < 0) { > pr_err("Could not setup DMA mask for udmabuf device\n"); > misc_deregister(&udmabuf_misc); misc_deregister() is now called twice in this error path, I think you've forgotten to delete this line too? Otherwise lgtm. -Sima > - return ret; > + goto err; > + } > + > + udmabuf_folio_cachep = KMEM_CACHE(udmabuf_folio, 0); > + if (unlikely(!udmabuf_folio_cachep)) { > + ret = -ENOMEM; > + goto err; > } > > return 0; > + > +err: > + misc_deregister(&udmabuf_misc); > + return ret; > } > > static void __exit udmabuf_dev_exit(void) > > base-commit: cd19ac2f903276b820f5d0d89de0c896c27036ed > -- > 2.45.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

4 months, 3 weeks

Re: [PATCH v2 0/5] Introduce DMA_HEAP_ALLOC_AND_READ_FILE heap flag

by Daniel Vetter

On Tue, Jul 30, 2024 at 08:04:04PM +0800, Huan Yang wrote: > > 在 2024/7/30 17:05, Huan Yang 写道: > > > > 在 2024/7/30 16:56, Daniel Vetter 写道: > > > [????????? daniel.vetter(a)ffwll.ch ????????? > > > https://aka.ms/LearnAboutSenderIdentification?????????????] > > > > > > On Tue, Jul 30, 2024 at 03:57:44PM +0800, Huan Yang wrote: > > > > UDMA-BUF step: > > > > 1. memfd_create > > > > 2. open file(buffer/direct) > > > > 3. udmabuf create > > > > 4. mmap memfd > > > > 5. read file into memfd vaddr > > > Yeah this is really slow and the worst way to do it. You absolutely want > > > to start _all_ the io before you start creating the dma-buf, ideally > > > with > > > everything running in parallel. But just starting the direct I/O with > > > async and then creating the umdabuf should be a lot faster and avoid > > That's greate, Let me rephrase that, and please correct me if I'm wrong. > > > > UDMA-BUF step: > > 1. memfd_create > > 2. mmap memfd > > 3. open file(buffer/direct) > > 4. start thread to async read > > 3. udmabuf create > > > > With this, can improve > > I just test with it. Step is: > > UDMA-BUF step: > 1. memfd_create > 2. mmap memfd > 3. open file(buffer/direct) > 4. start thread to async read > 5. udmabuf create > > 6 . join wait > > 3G file read all step cost 1,527,103,431ns, it's greate. Ok that's almost the throughput of your patch set, which I think is close enough. The remaining difference is probably just the mmap overhead, not sure whether/how we can do direct i/o to an fd directly ... in principle it's possible for any file that uses the standard pagecache. -Sima -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

4 months, 3 weeks

Re: [PATCH v3] udmabuf: use kmem_cache to alloc udmabuf folio

by Christophe JAILLET

Le 31/07/2024 à 09:37, Huan Yang a écrit : > The current udmabuf_folio contains a list_head and the corresponding > folio pointer, with a size of 24 bytes. udmabuf_folio uses kmalloc to > allocate memory. > > However, kmalloc is a public pool, starting from 8,16,32 bytes. > Additionally, if the size is not aligned with the kmalloc size, it will > be rounded up to the corresponding size. > This means that each udmabuf_folio allocation will get 32 bytes, and > waste 8 bytes. > > Considering that each udmabuf creates a folio corresponding to a > udmabuf_folio, the wasted memory can be significant in the case of > memory fragmentation. > > Furthermore, if udmabuf is frequently used, the allocation and > deallocation of udmabuf_folio will also be frequent. > > Therefore, this patch adds a kmem_cache dedicated to the allocation and > deallocation of udmabuf_folio.This is expected to improve the > performance of allocation and deallocation within the expected range, > while also avoiding memory waste. > > Signed-off-by: Huan Yang <link(a)vivo.com> > --- > v3 -> v2: fix error description. > v2 -> v1: fix double unregister, remove unlikely. > drivers/dma-buf/udmabuf.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 047c3cd2ceff..c112c58ef09a 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -24,6 +24,8 @@ static int size_limit_mb = 64; > module_param(size_limit_mb, int, 0644); > MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is 64."); > > +static struct kmem_cache *udmabuf_folio_cachep; > + > struct udmabuf { > pgoff_t pagecount; > struct folio **folios; > @@ -169,7 +171,7 @@ static void unpin_all_folios(struct list_head *unpin_list) > unpin_folio(ubuf_folio->folio); > > list_del(&ubuf_folio->list); > - kfree(ubuf_folio); > + kmem_cache_free(udmabuf_folio_cachep, ubuf_folio); > } > } > > @@ -178,7 +180,7 @@ static int add_to_unpin_list(struct list_head *unpin_list, > { > struct udmabuf_folio *ubuf_folio; > > - ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL); > + ubuf_folio = kmem_cache_alloc(udmabuf_folio_cachep, GFP_KERNEL); > if (!ubuf_folio) > return -ENOMEM; > > @@ -491,11 +493,20 @@ static int __init udmabuf_dev_init(void) > DMA_BIT_MASK(64)); > if (ret < 0) { > pr_err("Could not setup DMA mask for udmabuf device\n"); > - misc_deregister(&udmabuf_misc); > - return ret; > + goto err; > + } > + > + udmabuf_folio_cachep = KMEM_CACHE(udmabuf_folio, 0); > + if (!udmabuf_folio_cachep) { > + ret = -ENOMEM; > + goto err; > } > > return 0; > + > +err: > + misc_deregister(&udmabuf_misc); > + return ret; > } > > static void __exit udmabuf_dev_exit(void) Hi, should a kmem_cache_destroy() be also added in udmabuf_dev_exit()? CJ > > base-commit: cd19ac2f903276b820f5d0d89de0c896c27036ed

4 months, 3 weeks

Re: [PATCH v2 3/5] dma-buf: heaps: support alloc async read file

by kernel test robot

Hi Huan, kernel test robot noticed the following build warnings: [auto build test WARNING on 931a3b3bccc96e7708c82b30b2b5fa82dfd04890] url: https://github.com/intel-lab-lkp/linux/commits/Huan-Yang/dma-buf-heaps-Intr… base: 931a3b3bccc96e7708c82b30b2b5fa82dfd04890 patch link: https://lore.kernel.org/r/20240730075755.10941-4-link%40vivo.com patch subject: [PATCH v2 3/5] dma-buf: heaps: support alloc async read file config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20240731/202407312202.LhLTLEhX-lkp@…) compiler: xtensa-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240731/202407312202.LhLTLEhX-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202407312202.LhLTLEhX-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/dma-buf/dma-heap.c:45: warning: Function parameter or struct member 'priv' not described in 'dma_heap' drivers/dma-buf/dma-heap.c:45: warning: Function parameter or struct member 'heap_devt' not described in 'dma_heap' drivers/dma-buf/dma-heap.c:45: warning: Function parameter or struct member 'list' not described in 'dma_heap' drivers/dma-buf/dma-heap.c:45: warning: Function parameter or struct member 'heap_cdev' not described in 'dma_heap' >> drivers/dma-buf/dma-heap.c:158: warning: Function parameter or struct member 'lock' not described in 'dma_heap_file_control' drivers/dma-buf/dma-heap.c:482: warning: expecting prototype for Trigger sync file read, read into dma(). Prototype was for dma_heap_read_file_sync() instead vim +158 drivers/dma-buf/dma-heap.c 133 134 /** 135 * struct dma_heap_file_control - global control of dma_heap file read. 136 * @works: @dma_heap_file_work's list head. 137 * 138 * @threadwq: wait queue for @work_thread, if commit work, @work_thread 139 * wakeup and read this work's file contains. 140 * 141 * @workwq: used for main thread wait for file read end, if allocation 142 * end before file read. @dma_heap_file_task ref effect this. 143 * 144 * @work_thread: file read kthread. the dma_heap_file_task work's consumer. 145 * 146 * @heap_fwork_cachep: @dma_heap_file_work's cachep, it's alloc/free frequently. 147 * 148 * @nr_work: global number of how many work committed. 149 */ 150 struct dma_heap_file_control { 151 struct list_head works; 152 spinlock_t lock; // only lock for @works. 153 wait_queue_head_t threadwq; 154 wait_queue_head_t workwq; 155 struct task_struct *work_thread; 156 struct kmem_cache *heap_fwork_cachep; 157 atomic_t nr_work; > 158 }; 159 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

4 months, 3 weeks

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig