| Message ID | 20210202095110.1215346-7-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] dma-mapping: remove the {alloc,free}_noncoherent methods | expand |
Hi Christoph On Tue, Feb 2, 2021 at 6:51 PM Christoph Hellwig <hch@lst.de> wrote: > > Implement support for allocating a non-contiguous DMA region. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/iommu/dma-iommu.c | 35 +++++++++++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 85cb004d7a44c6..4e0b170d38d57a 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -718,6 +718,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > goto out_free_sg; > > sgt->sgl->dma_address = iova; > + sgt->sgl->dma_length = size; > return pages; > > out_free_sg: > @@ -755,6 +756,36 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, > return NULL; > } > > +#ifdef CONFIG_DMA_REMAP > +static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, > + size_t size, enum dma_data_direction dir, gfp_t gfp) > +{ > + struct dma_sgt_handle *sh; > + > + sh = kmalloc(sizeof(*sh), gfp); > + if (!sh) > + return NULL; > + > + sh->pages = __iommu_dma_alloc_noncontiguous(dev, size, &sh->sgt, gfp, > + PAGE_KERNEL, 0); When working on the videobuf2 integration with Sergey I noticed that we always pass 0 as DMA attrs here, which removes the ability for drivers to use DMA_ATTR_ALLOC_SINGLE_PAGES. It's quite important from a system stability point of view, because by default the iommu_dma allocator would prefer big order allocations for TLB locality reasons. For many devices, though, it doesn't really affect the performance, because of random access patterns, so single pages are good enough and reduce the risk of allocation failures or latency due to fragmentation. Do you think we could add the attrs parameter to the dma_alloc_noncontiguous() API? Best regards, Tomasz
On Tue, Feb 16, 2021 at 05:14:55PM +0900, Tomasz Figa wrote: > When working on the videobuf2 integration with Sergey I noticed that > we always pass 0 as DMA attrs here, which removes the ability for > drivers to use DMA_ATTR_ALLOC_SINGLE_PAGES. > > It's quite important from a system stability point of view, because by > default the iommu_dma allocator would prefer big order allocations for > TLB locality reasons. For many devices, though, it doesn't really > affect the performance, because of random access patterns, so single > pages are good enough and reduce the risk of allocation failures or > latency due to fragmentation. > > Do you think we could add the attrs parameter to the > dma_alloc_noncontiguous() API? Yes, we could probably do that.
On (21/02/16 09:49), Christoph Hellwig wrote: > > When working on the videobuf2 integration with Sergey I noticed that > > we always pass 0 as DMA attrs here, which removes the ability for > > drivers to use DMA_ATTR_ALLOC_SINGLE_PAGES. > > > > It's quite important from a system stability point of view, because by > > default the iommu_dma allocator would prefer big order allocations for > > TLB locality reasons. For many devices, though, it doesn't really > > affect the performance, because of random access patterns, so single > > pages are good enough and reduce the risk of allocation failures or > > latency due to fragmentation. > > > > Do you think we could add the attrs parameter to the > > dma_alloc_noncontiguous() API? > > Yes, we could probably do that. I can cook a patch, unless somebody is already looking into it. -ss
On Mon, Mar 01, 2021 at 04:17:42PM +0900, Sergey Senozhatsky wrote: > > > Do you think we could add the attrs parameter to the > > > dma_alloc_noncontiguous() API? > > > > Yes, we could probably do that. > > I can cook a patch, unless somebody is already looking into it. I plan to resend the whole series with the comments very soon.
On (21/03/01 08:21), Christoph Hellwig wrote: > On Mon, Mar 01, 2021 at 04:17:42PM +0900, Sergey Senozhatsky wrote: > > > > Do you think we could add the attrs parameter to the > > > > dma_alloc_noncontiguous() API? > > > > > > Yes, we could probably do that. > > > > I can cook a patch, unless somebody is already looking into it. > > I plan to resend the whole series with the comments very soon. Oh, OK. I thought the series was in linux-next already so a single patch would do. -ss
On Mon, Mar 01, 2021 at 05:02:43PM +0900, Sergey Senozhatsky wrote: > > I plan to resend the whole series with the comments very soon. > > Oh, OK. > > I thought the series was in linux-next already so a single patch > would do. It was, with an emphasys on was. I hadn't realized I need an ack from Laurent for uvcvideo, and he didn't have time to review it by the time we noticed. So I'll repost it with him in the receipients list and the small fixups accumulated now that -rc1 is out.
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 85cb004d7a44c6..4e0b170d38d57a 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -718,6 +718,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, goto out_free_sg; sgt->sgl->dma_address = iova; + sgt->sgl->dma_length = size; return pages; out_free_sg: @@ -755,6 +756,36 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, return NULL; } +#ifdef CONFIG_DMA_REMAP +static struct sg_table *iommu_dma_alloc_noncontiguous(struct device *dev, + size_t size, enum dma_data_direction dir, gfp_t gfp) +{ + struct dma_sgt_handle *sh; + + sh = kmalloc(sizeof(*sh), gfp); + if (!sh) + return NULL; + + sh->pages = __iommu_dma_alloc_noncontiguous(dev, size, &sh->sgt, gfp, + PAGE_KERNEL, 0); + if (!sh->pages) { + kfree(sh); + return NULL; + } + return &sh->sgt; +} + +static void iommu_dma_free_noncontiguous(struct device *dev, size_t size, + struct sg_table *sgt, enum dma_data_direction dir) +{ + struct dma_sgt_handle *sh = sgt_handle(sgt); + + __iommu_dma_unmap(dev, sgt->sgl->dma_address, size); + __iommu_dma_free_pages(sh->pages, PAGE_ALIGN(size) >> PAGE_SHIFT); + sg_free_table(&sh->sgt); +} +#endif /* CONFIG_DMA_REMAP */ + static void iommu_dma_sync_single_for_cpu(struct device *dev, dma_addr_t dma_handle, size_t size, enum dma_data_direction dir) { @@ -1270,6 +1301,10 @@ static const struct dma_map_ops iommu_dma_ops = { .free = iommu_dma_free, .alloc_pages = dma_common_alloc_pages, .free_pages = dma_common_free_pages, +#ifdef CONFIG_DMA_REMAP + .alloc_noncontiguous = iommu_dma_alloc_noncontiguous, + .free_noncontiguous = iommu_dma_free_noncontiguous, +#endif .mmap = iommu_dma_mmap, .get_sgtable = iommu_dma_get_sgtable, .map_page = iommu_dma_map_page,
Implement support for allocating a non-contiguous DMA region. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/iommu/dma-iommu.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)