Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

16 participants
3178 discussions

[PATCH rdma-next 0/8] Introducing Multi-Path DMA Support for mlx5 RDMA Driver

by Leon Romanovsky

From: Leon Romanovsky <leonro(a)nvidia.com> From Yishai, Overview -------- This patch series aims to enable multi-path DMA support, allowing an mlx5 RDMA device to issue DMA commands through multiple paths. This feature is critical for improving performance and reaching line rate in certain environments where issuing PCI transactions over one path may be significantly faster than over another. These differences can arise from various PCI generations in the system or the specific system topology. To achieve this functionality, we introduced a data direct DMA device that can serve the RDMA device by issuing DMA transactions on its behalf. The main key features and changes are described below. Multi-Path Discovery -------------------- API Implementation: * Introduced an API to discover multiple paths for a given mlx5 RDMA device. IOCTL Command: * Added a new IOCTL command, MLX5_IB_METHOD_GET_DATA_DIRECT_SYSFS_PATH, to the DEVICE object. When an affiliated Data-Direct/DMA device is present, its sysfs path is returned. Feature Activation by mlx5 RDMA Application ------------------------------------------- UVERBS Extension: * Extended UVERBS_METHOD_REG_DMABUF_MR over UVERBS_OBJECT_MR to include mlx5 extended flags. Access Flag: * Introduced the MLX5_IB_UAPI_REG_DMABUF_ACCESS_DATA_DIRECT flag, allowing applications to request the use of the affiliated DMA device for DMABUF registration. Data-Direct/DMA Device ---------------------- New Driver: * Introduced a new driver to manage the new DMA PF device ID (0x2100). Its registration/un-registration is handled as part of the mlx5_ib init/exit flows, with mlx5 IB devices as its clients. Functionality: * The driver does not interface directly with the firmware (no command interface, no caps, etc.) but works over PCI to activate its DMA functionality. It serves as the DMA device for efficiently accessing other PCI devices (e.g., GPU PF) and reads its VUID over PCI to handle NICs registrations with the same VUID. mlx5 IB RDMA Device --------------------------- VUID Query: * Reads its affiliated DMA PF VUID via the QUERY_VUID command with the data_direct bit set. Driver Registration: * Registers with the DMA PF driver to be notified upon bind/unbind. Application Request Handling: * Uses the DMA PF device upon application request as described above. DMABUF over Umem ---------------- Introduced an option to obtain a DMABUF UMEM using a different DMA device instead of the IB device, allowing the device to register over IOMMU with the expected DMA device for a given buffer registration. Further details are provided in the commit logs of the patches in this series. Thanks Yishai Hadas (8): net/mlx5: Add IFC related stuff for data direct RDMA/mlx5: Introduce the 'data direct' driver RDMA/mlx5: Add the initialization flow to utilize the 'data direct' device RDMA/umem: Add support for creating pinned DMABUF umem with a given dma device RDMA/umem: Introduce an option to revoke DMABUF umem RDMA: Pass uverbs_attr_bundle as part of '.reg_user_mr_dmabuf' API RDMA/mlx5: Add support for DMABUF MR registrations with Data-direct RDMA/mlx5: Introduce GET_DATA_DIRECT_SYSFS_PATH ioctl drivers/infiniband/core/umem_dmabuf.c | 66 +++- drivers/infiniband/core/uverbs_std_types_mr.c | 2 +- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 3 +- drivers/infiniband/hw/bnxt_re/ib_verbs.h | 2 +- drivers/infiniband/hw/efa/efa.h | 2 +- drivers/infiniband/hw/efa/efa_verbs.c | 4 +- drivers/infiniband/hw/irdma/verbs.c | 2 +- drivers/infiniband/hw/mlx5/Makefile | 1 + drivers/infiniband/hw/mlx5/cmd.c | 21 ++ drivers/infiniband/hw/mlx5/cmd.h | 2 + drivers/infiniband/hw/mlx5/data_direct.c | 227 +++++++++++++ drivers/infiniband/hw/mlx5/data_direct.h | 23 ++ drivers/infiniband/hw/mlx5/main.c | 125 +++++++ drivers/infiniband/hw/mlx5/mlx5_ib.h | 22 +- drivers/infiniband/hw/mlx5/mr.c | 304 +++++++++++++++--- drivers/infiniband/hw/mlx5/odp.c | 5 +- drivers/infiniband/hw/mlx5/std_types.c | 55 +++- drivers/infiniband/hw/mlx5/umr.c | 93 ++++-- drivers/infiniband/hw/mlx5/umr.h | 1 + include/linux/mlx5/mlx5_ifc.h | 51 ++- include/rdma/ib_umem.h | 18 ++ include/rdma/ib_verbs.h | 2 +- include/uapi/rdma/mlx5_user_ioctl_cmds.h | 9 + include/uapi/rdma/mlx5_user_ioctl_verbs.h | 4 + 24 files changed, 944 insertions(+), 100 deletions(-) create mode 100644 drivers/infiniband/hw/mlx5/data_direct.c create mode 100644 drivers/infiniband/hw/mlx5/data_direct.h -- 2.45.2

1 year, 3 months

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

1 year, 3 months

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

1 year, 3 months

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; > } >

1 year, 3 months

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);

1 year, 3 months

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; > }

1 year, 3 months

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,

1 year, 3 months

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

1 year, 3 months

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

1 year, 3 months

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

1 year, 3 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig