* [PATCH v5 29/38] drm: rcar-du: fix common struct sg_table related issues [not found] ` <CGME20200513133317eucas1p27aead4025db2da13e5b7c3e14a7cd79d@eucas1p2.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 2020-05-13 14:23 ` Laurent Pinchart 0 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Mauro Carvalho Chehab, Laurent Pinchart, Kieran Bingham, linux-renesas-soc, linux-media The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg(). struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry). It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function. To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe. dma_map_sgtable() function returns zero or an error code, so adjust the return value check for the vsp1_du_map_sg() function. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-- drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 5e4faf2..2fc1816 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -197,9 +197,8 @@ int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb, goto fail; ret = vsp1_du_map_sg(vsp->vsp, sgt); - if (!ret) { + if (ret) { sg_free_table(sgt); - ret = -ENOMEM; goto fail; } } diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index a4a45d6..86d5e3f 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -912,8 +912,8 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt) * skip cache sync. This will need to be revisited when support for * non-coherent buffers will be added to the DU driver. */ - return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); + return dma_map_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, + DMA_ATTR_SKIP_CPU_SYNC); } EXPORT_SYMBOL_GPL(vsp1_du_map_sg); @@ -921,8 +921,8 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt) { struct vsp1_device *vsp1 = dev_get_drvdata(dev); - dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, + DMA_ATTR_SKIP_CPU_SYNC); } EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 29/38] drm: rcar-du: fix common struct sg_table related issues 2020-05-13 13:32 ` [PATCH v5 29/38] drm: rcar-du: fix common struct sg_table related issues Marek Szyprowski @ 2020-05-13 14:23 ` Laurent Pinchart 0 siblings, 0 replies; 8+ messages in thread From: Laurent Pinchart @ 2020-05-13 14:23 UTC (permalink / raw) To: Marek Szyprowski Cc: dri-devel, iommu, linaro-mm-sig, linux-kernel, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Mauro Carvalho Chehab, Kieran Bingham, linux-renesas-soc, linux-media Hi Marek, Thank you for the patch. On Wed, May 13, 2020 at 03:32:36PM +0200, Marek Szyprowski wrote: > The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function > returns the number of the created entries in the DMA address space. > However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and > dma_unmap_sg must be called with the original number of the entries > passed to the dma_map_sg(). > > struct sg_table is a common structure used for describing a non-contiguous > memory buffer, used commonly in the DRM and graphics subsystems. It > consists of a scatterlist with memory pages and DMA addresses (sgl entry), > as well as the number of scatterlist entries: CPU pages (orig_nents entry) > and DMA mapped pages (nents entry). > > It turned out that it was a common mistake to misuse nents and orig_nents > entries, calling DMA-mapping functions with a wrong number of entries or > ignoring the number of mapped entries returned by the dma_map_sg() > function. > > To avoid such issues, lets use a common dma-mapping wrappers operating > directly on the struct sg_table objects and use scatterlist page > iterators where possible. This, almost always, hides references to the > nents and orig_nents entries, making the code robust, easier to follow > and copy/paste safe. > > dma_map_sgtable() function returns zero or an error code, so adjust the > return value check for the vsp1_du_map_sg() function. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> This is a very nice simplification, I've always foudn the dma_map_sg API cumbersome to use. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I assume you will get the whole series merged in one go. If I need to pick the patch up at any point, please let me know. Otherwise I'll wait until I see it upstream, no reply needed :-) > --- > For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents > vs. orig_nents misuse' thread: > https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ > --- > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 3 +-- > drivers/media/platform/vsp1/vsp1_drm.c | 8 ++++---- > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > index 5e4faf2..2fc1816 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > @@ -197,9 +197,8 @@ int rcar_du_vsp_map_fb(struct rcar_du_vsp *vsp, struct drm_framebuffer *fb, > goto fail; > > ret = vsp1_du_map_sg(vsp->vsp, sgt); > - if (!ret) { > + if (ret) { > sg_free_table(sgt); > - ret = -ENOMEM; > goto fail; > } > } > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c > index a4a45d6..86d5e3f 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -912,8 +912,8 @@ int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt) > * skip cache sync. This will need to be revisited when support for > * non-coherent buffers will be added to the DU driver. > */ > - return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, > - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > + return dma_map_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, > + DMA_ATTR_SKIP_CPU_SYNC); > } > EXPORT_SYMBOL_GPL(vsp1_du_map_sg); > > @@ -921,8 +921,8 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt) > { > struct vsp1_device *vsp1 = dev_get_drvdata(dev); > > - dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents, > - DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > + dma_unmap_sgtable(vsp1->bus_master, sgt, DMA_TO_DEVICE, > + DMA_ATTR_SKIP_CPU_SYNC); > } > EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg); > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CGME20200513133318eucas1p1dd6c1ac6a777874c115070d8b5197f34@eucas1p1.samsung.com>]
* [PATCH v5 30/38] dmabuf: fix common struct sg_table related issues [not found] ` <CGME20200513133318eucas1p1dd6c1ac6a777874c115070d8b5197f34@eucas1p1.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 2020-05-15 10:13 ` Gerd Hoffmann 0 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Sumit Semwal, Gerd Hoffmann, linux-media The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg(). struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry). It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function. To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- drivers/dma-buf/heaps/heap-helpers.c | 13 ++++++------- drivers/dma-buf/udmabuf.c | 7 +++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c index 9f964ca..d0696cf 100644 --- a/drivers/dma-buf/heaps/heap-helpers.c +++ b/drivers/dma-buf/heaps/heap-helpers.c @@ -140,13 +140,12 @@ struct sg_table *dma_heap_map_dma_buf(struct dma_buf_attachment *attachment, enum dma_data_direction direction) { struct dma_heaps_attachment *a = attachment->priv; - struct sg_table *table; - - table = &a->table; + struct sg_table *table = &a->table; + int ret; - if (!dma_map_sg(attachment->dev, table->sgl, table->nents, - direction)) - table = ERR_PTR(-ENOMEM); + ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (ret) + table = ERR_PTR(ret); return table; } @@ -154,7 +153,7 @@ static void dma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { - dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); + dma_unmap_sgtable(attachment->dev, table, direction, 0); } static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index acb26c6..89e293b 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -63,10 +63,9 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, GFP_KERNEL); if (ret < 0) goto err; - if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) { - ret = -EINVAL; + ret = dma_map_sgtable(dev, sg, direction, 0); + if (ret < 0) goto err; - } return sg; err: @@ -78,7 +77,7 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, static void put_sg_table(struct device *dev, struct sg_table *sg, enum dma_data_direction direction) { - dma_unmap_sg(dev, sg->sgl, sg->nents, direction); + dma_unmap_sgtable(dev, sg, direction, 0); sg_free_table(sg); kfree(sg); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 30/38] dmabuf: fix common struct sg_table related issues 2020-05-13 13:32 ` [PATCH v5 30/38] dmabuf: " Marek Szyprowski @ 2020-05-15 10:13 ` Gerd Hoffmann 0 siblings, 0 replies; 8+ messages in thread From: Gerd Hoffmann @ 2020-05-15 10:13 UTC (permalink / raw) To: Marek Szyprowski Cc: dri-devel, iommu, linaro-mm-sig, linux-kernel, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Sumit Semwal, linux-media > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index acb26c6..89e293b 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -63,10 +63,9 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > GFP_KERNEL); > if (ret < 0) > goto err; > - if (!dma_map_sg(dev, sg->sgl, sg->nents, direction)) { > - ret = -EINVAL; > + ret = dma_map_sgtable(dev, sg, direction, 0); > + if (ret < 0) > goto err; > - } > return sg; > > err: > @@ -78,7 +77,7 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, > static void put_sg_table(struct device *dev, struct sg_table *sg, > enum dma_data_direction direction) > { > - dma_unmap_sg(dev, sg->sgl, sg->nents, direction); > + dma_unmap_sgtable(dev, sg, direction, 0); > sg_free_table(sg); > kfree(sg); > } Easy straightforward conversation. Acked-by: Gerd Hoffmann <kraxel@redhat.com> take care, Gerd ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CGME20200513133321eucas1p13acea3aa6219ce5f7076c7677ef9eae3@eucas1p1.samsung.com>]
* [PATCH v5 33/38] staging: tegra-vde: fix common struct sg_table related issues [not found] ` <CGME20200513133321eucas1p13acea3aa6219ce5f7076c7677ef9eae3@eucas1p1.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 2020-05-14 20:07 ` Dmitry Osipenko 0 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman, Thierry Reding, Jonathan Hunter, linux-media, linux-tegra, devel The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function returns the number of the created entries in the DMA address space. However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and dma_unmap_sg must be called with the original number of the entries passed to the dma_map_sg(). struct sg_table is a common structure used for describing a non-contiguous memory buffer, used commonly in the DRM and graphics subsystems. It consists of a scatterlist with memory pages and DMA addresses (sgl entry), as well as the number of scatterlist entries: CPU pages (orig_nents entry) and DMA mapped pages (nents entry). It turned out that it was a common mistake to misuse nents and orig_nents entries, calling DMA-mapping functions with a wrong number of entries or ignoring the number of mapped entries returned by the dma_map_sg() function. To avoid such issues, lets use a common dma-mapping wrappers operating directly on the struct sg_table objects and use scatterlist page iterators where possible. This, almost always, hides references to the nents and orig_nents entries, making the code robust, easier to follow and copy/paste safe. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- drivers/staging/media/tegra-vde/iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c index 6af863d..adf8dc7 100644 --- a/drivers/staging/media/tegra-vde/iommu.c +++ b/drivers/staging/media/tegra-vde/iommu.c @@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde, addr = iova_dma_addr(&vde->iova, iova); - size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents, - IOMMU_READ | IOMMU_WRITE); + size = iommu_map_sgtable(vde->domain, addr, sgt, + IOMMU_READ | IOMMU_WRITE); if (!size) { __free_iova(&vde->iova, iova); return -ENXIO; -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 33/38] staging: tegra-vde: fix common struct sg_table related issues 2020-05-13 13:32 ` [PATCH v5 33/38] staging: tegra-vde: " Marek Szyprowski @ 2020-05-14 20:07 ` Dmitry Osipenko 0 siblings, 0 replies; 8+ messages in thread From: Dmitry Osipenko @ 2020-05-14 20:07 UTC (permalink / raw) To: Marek Szyprowski, dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Mauro Carvalho Chehab, Greg Kroah-Hartman, Thierry Reding, Jonathan Hunter, linux-media, linux-tegra, devel 13.05.2020 16:32, Marek Szyprowski пишет: > The Documentation/DMA-API-HOWTO.txt states that the dma_map_sg() function > returns the number of the created entries in the DMA address space. > However the subsequent calls to the dma_sync_sg_for_{device,cpu}() and > dma_unmap_sg must be called with the original number of the entries > passed to the dma_map_sg(). > > struct sg_table is a common structure used for describing a non-contiguous > memory buffer, used commonly in the DRM and graphics subsystems. It > consists of a scatterlist with memory pages and DMA addresses (sgl entry), > as well as the number of scatterlist entries: CPU pages (orig_nents entry) > and DMA mapped pages (nents entry). > > It turned out that it was a common mistake to misuse nents and orig_nents > entries, calling DMA-mapping functions with a wrong number of entries or > ignoring the number of mapped entries returned by the dma_map_sg() > function. > > To avoid such issues, lets use a common dma-mapping wrappers operating > directly on the struct sg_table objects and use scatterlist page > iterators where possible. This, almost always, hides references to the > nents and orig_nents entries, making the code robust, easier to follow > and copy/paste safe. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents > vs. orig_nents misuse' thread: > https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ > --- > drivers/staging/media/tegra-vde/iommu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c > index 6af863d..adf8dc7 100644 > --- a/drivers/staging/media/tegra-vde/iommu.c > +++ b/drivers/staging/media/tegra-vde/iommu.c > @@ -36,8 +36,8 @@ int tegra_vde_iommu_map(struct tegra_vde *vde, > > addr = iova_dma_addr(&vde->iova, iova); > > - size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents, > - IOMMU_READ | IOMMU_WRITE); > + size = iommu_map_sgtable(vde->domain, addr, sgt, > + IOMMU_READ | IOMMU_WRITE); > if (!size) { > __free_iova(&vde->iova, iova); > return -ENXIO; > Reviewed-by: Dmitry Osipenko <digetx@gmail.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CGME20200513133324eucas1p25dc380046c516ca8f07677ed14f93e63@eucas1p2.samsung.com>]
* [PATCH v5 37/38] media: pci: fix common ALSA DMA-mapping related codes [not found] ` <CGME20200513133324eucas1p25dc380046c516ca8f07677ed14f93e63@eucas1p2.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 0 siblings, 0 replies; 8+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Mauro Carvalho Chehab, linux-media The Documentation/DMA-API-HOWTO.txt states that dma_map_sg returns the numer of the created entries in the DMA address space. However the subsequent calls to dma_sync_sg_for_{device,cpu} and dma_unmap_sg must be called with the original number of entries passed to dma_map_sg. The sg_table->nents in turn holds the result of the dma_map_sg call as stated in include/linux/scatterlist.h. Adapt the code to obey those rules. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- drivers/media/pci/cx23885/cx23885-alsa.c | 2 +- drivers/media/pci/cx25821/cx25821-alsa.c | 2 +- drivers/media/pci/cx88/cx88-alsa.c | 2 +- drivers/media/pci/saa7134/saa7134-alsa.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/pci/cx23885/cx23885-alsa.c b/drivers/media/pci/cx23885/cx23885-alsa.c index df44ed7..3f366e4 100644 --- a/drivers/media/pci/cx23885/cx23885-alsa.c +++ b/drivers/media/pci/cx23885/cx23885-alsa.c @@ -129,7 +129,7 @@ static int cx23885_alsa_dma_unmap(struct cx23885_audio_dev *dev) if (!buf->sglen) return 0; - dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; } diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c b/drivers/media/pci/cx25821/cx25821-alsa.c index 3016164..c40304d 100644 --- a/drivers/media/pci/cx25821/cx25821-alsa.c +++ b/drivers/media/pci/cx25821/cx25821-alsa.c @@ -193,7 +193,7 @@ static int cx25821_alsa_dma_unmap(struct cx25821_audio_dev *dev) if (!buf->sglen) return 0; - dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; } diff --git a/drivers/media/pci/cx88/cx88-alsa.c b/drivers/media/pci/cx88/cx88-alsa.c index 7d7acee..3c6fe6c 100644 --- a/drivers/media/pci/cx88/cx88-alsa.c +++ b/drivers/media/pci/cx88/cx88-alsa.c @@ -332,7 +332,7 @@ static int cx88_alsa_dma_unmap(struct cx88_audio_dev *dev) if (!buf->sglen) return 0; - dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->sglen, + dma_unmap_sg(&dev->pci->dev, buf->sglist, buf->nr_pages, PCI_DMA_FROMDEVICE); buf->sglen = 0; return 0; diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c b/drivers/media/pci/saa7134/saa7134-alsa.c index 544ca57..398c47f 100644 --- a/drivers/media/pci/saa7134/saa7134-alsa.c +++ b/drivers/media/pci/saa7134/saa7134-alsa.c @@ -313,7 +313,7 @@ static int saa7134_alsa_dma_unmap(struct saa7134_dev *dev) if (!dma->sglen) return 0; - dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->sglen, PCI_DMA_FROMDEVICE); + dma_unmap_sg(&dev->pci->dev, dma->sglist, dma->nr_pages, PCI_DMA_FROMDEVICE); dma->sglen = 0; return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <CGME20200513133325eucas1p1c8faf557b23b5d171b7c328eca5f6482@eucas1p1.samsung.com>]
* [PATCH v5 38/38] videobuf2: use sgtable-based scatterlist wrappers [not found] ` <CGME20200513133325eucas1p1c8faf557b23b5d171b7c328eca5f6482@eucas1p1.samsung.com> @ 2020-05-13 13:32 ` Marek Szyprowski 0 siblings, 0 replies; 8+ messages in thread From: Marek Szyprowski @ 2020-05-13 13:32 UTC (permalink / raw) To: dri-devel, iommu, linaro-mm-sig, linux-kernel Cc: Marek Szyprowski, Christoph Hellwig, Robin Murphy, Bartlomiej Zolnierkiewicz, linux-arm-kernel, David Airlie, Daniel Vetter, Pawel Osciak, Kyungmin Park, Mauro Carvalho Chehab, linux-media Use recently introduced common wrappers operating directly on the struct sg_table objects and scatterlist page iterators to make the code a bit more compact, robust, easier to follow and copy/paste safe. No functional change, because the code already properly did all the scaterlist related calls. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents vs. orig_nents misuse' thread: https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprowski@samsung.com/T/ --- .../media/common/videobuf2/videobuf2-dma-contig.c | 41 ++++++++++------------ drivers/media/common/videobuf2/videobuf2-dma-sg.c | 32 +++++++---------- drivers/media/common/videobuf2/videobuf2-vmalloc.c | 12 +++---- 3 files changed, 34 insertions(+), 51 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c index d3a3ee5..bf31a9d 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c @@ -48,16 +48,15 @@ struct vb2_dc_buf { static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt) { - struct scatterlist *s; dma_addr_t expected = sg_dma_address(sgt->sgl); - unsigned int i; + struct sg_dma_page_iter dma_iter; unsigned long size = 0; - for_each_sg(sgt->sgl, s, sgt->nents, i) { - if (sg_dma_address(s) != expected) + for_each_sgtable_dma_page(sgt, &dma_iter, 0) { + if (sg_page_iter_dma_address(&dma_iter) != expected) break; - expected = sg_dma_address(s) + sg_dma_len(s); - size += sg_dma_len(s); + expected += PAGE_SIZE; + size += PAGE_SIZE; } return size; } @@ -99,8 +98,7 @@ static void vb2_dc_prepare(void *buf_priv) if (!sgt || buf->db_attach) return; - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); } static void vb2_dc_finish(void *buf_priv) @@ -112,7 +110,7 @@ static void vb2_dc_finish(void *buf_priv) if (!sgt || buf->db_attach) return; - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); } /*********************************************/ @@ -273,8 +271,8 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf, * memory locations do not require any explicit cache * maintenance prior or after being used by the device. */ - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -299,8 +297,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( /* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); attach->dma_dir = DMA_NONE; } @@ -308,9 +306,8 @@ static struct sg_table *vb2_dc_dmabuf_ops_map( * mapping to the client with new direction, no cache sync * required see comment in vb2_dc_dmabuf_ops_detach() */ - sgt->nents = dma_map_sg_attrs(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO); @@ -423,8 +420,8 @@ static void vb2_dc_put_userptr(void *buf_priv) * No need to sync to CPU, it's already synced to the CPU * since the finish() memop will have been called before this. */ - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); pages = frame_vector_pages(buf->vec); /* sgt should exist only if vector contains pages... */ BUG_ON(IS_ERR(pages)); @@ -521,9 +518,8 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, * No need to sync to the device, this will happen later when the * prepare() memop is called. */ - sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (sgt->nents <= 0) { + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { pr_err("failed to map scatterlist\n"); ret = -EIO; goto fail_sgt_init; @@ -545,8 +541,7 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, return buf; fail_map_sg: - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); fail_sgt_init: sg_free_table(sgt); diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 92072a0..6ddf953 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -142,9 +142,8 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, * No need to sync to the device, this will happen later when the * prepare() memop is called. */ - sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { goto fail_map; buf->handler.refcount = &buf->refcount; @@ -180,8 +179,8 @@ static void vb2_dma_sg_put(void *buf_priv) if (refcount_dec_and_test(&buf->refcount)) { dprintk(1, "%s: Freeing buffer of %d pages\n", __func__, buf->num_pages); - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC); if (buf->vaddr) vm_unmap_ram(buf->vaddr, buf->num_pages); sg_free_table(buf->dma_sgt); @@ -202,8 +201,7 @@ static void vb2_dma_sg_prepare(void *buf_priv) if (buf->db_attach) return; - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir); + dma_sync_sgtable_for_device(buf->dev, sgt, buf->dma_dir); } static void vb2_dma_sg_finish(void *buf_priv) @@ -215,7 +213,7 @@ static void vb2_dma_sg_finish(void *buf_priv) if (buf->db_attach) return; - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); + dma_sync_sgtable_for_cpu(buf->dev, sgt, buf->dma_dir); } static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, @@ -258,9 +256,8 @@ static void *vb2_dma_sg_get_userptr(struct device *dev, unsigned long vaddr, * No need to sync to the device, this will happen later when the * prepare() memop is called. */ - sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, - buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); - if (!sgt->nents) + if (dma_map_sgtable(buf->dev, sgt, buf->dma_dir, + DMA_ATTR_SKIP_CPU_SYNC)) { goto userptr_fail_map; return buf; @@ -286,8 +283,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv) dprintk(1, "%s: Releasing userspace buffer of %d pages\n", __func__, buf->num_pages); - dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir, - DMA_ATTR_SKIP_CPU_SYNC); + dma_unmap_sgtable(buf->dev, sgt, buf->dma_dir, DMA_ATTR_SKIP_CPU_SYNC); if (buf->vaddr) vm_unmap_ram(buf->vaddr, buf->num_pages); sg_free_table(buf->dma_sgt); @@ -410,8 +406,7 @@ static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf, /* release the scatterlist cache */ if (attach->dma_dir != DMA_NONE) - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -436,15 +431,12 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map( /* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir); attach->dma_dir = DMA_NONE; } /* mapping to the client with new direction */ - sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO); diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c index c66fda4..bf5ac63 100644 --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c @@ -229,7 +229,7 @@ static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf, kfree(attach); return ret; } - for_each_sg(sgt->sgl, sg, sgt->nents, i) { + for_each_sgtable_sg(sgt, sg, i) { struct page *page = vmalloc_to_page(vaddr); if (!page) { @@ -259,8 +259,7 @@ static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf, /* release the scatterlist cache */ if (attach->dma_dir != DMA_NONE) - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0); sg_free_table(sgt); kfree(attach); db_attach->priv = NULL; @@ -285,15 +284,12 @@ static struct sg_table *vb2_vmalloc_dmabuf_ops_map( /* release any previous cache */ if (attach->dma_dir != DMA_NONE) { - dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - attach->dma_dir); + dma_unmap_sgtable(db_attach->dev, sgt, attach->dma_dir, 0); attach->dma_dir = DMA_NONE; } /* mapping to the client with new direction */ - sgt->nents = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, - dma_dir); - if (!sgt->nents) { + if (dma_map_sgtable(db_attach->dev, sgt, dma_dir, 0)) { pr_err("failed to map scatterlist\n"); mutex_unlock(lock); return ERR_PTR(-EIO); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-15 10:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200513132114.6046-1-m.szyprowski@samsung.com> [not found] ` <20200513133245.6408-1-m.szyprowski@samsung.com> [not found] ` <CGME20200513133317eucas1p27aead4025db2da13e5b7c3e14a7cd79d@eucas1p2.samsung.com> 2020-05-13 13:32 ` [PATCH v5 29/38] drm: rcar-du: fix common struct sg_table related issues Marek Szyprowski 2020-05-13 14:23 ` Laurent Pinchart [not found] ` <CGME20200513133318eucas1p1dd6c1ac6a777874c115070d8b5197f34@eucas1p1.samsung.com> 2020-05-13 13:32 ` [PATCH v5 30/38] dmabuf: " Marek Szyprowski 2020-05-15 10:13 ` Gerd Hoffmann [not found] ` <CGME20200513133321eucas1p13acea3aa6219ce5f7076c7677ef9eae3@eucas1p1.samsung.com> 2020-05-13 13:32 ` [PATCH v5 33/38] staging: tegra-vde: " Marek Szyprowski 2020-05-14 20:07 ` Dmitry Osipenko [not found] ` <CGME20200513133324eucas1p25dc380046c516ca8f07677ed14f93e63@eucas1p2.samsung.com> 2020-05-13 13:32 ` [PATCH v5 37/38] media: pci: fix common ALSA DMA-mapping related codes Marek Szyprowski [not found] ` <CGME20200513133325eucas1p1c8faf557b23b5d171b7c328eca5f6482@eucas1p1.samsung.com> 2020-05-13 13:32 ` [PATCH v5 38/38] videobuf2: use sgtable-based scatterlist wrappers Marek Szyprowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).