[PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

[PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

[FUJITA Tomonori]

This patchset removes useless sync_single_range_for_cpu and
sync_single_range_for_device hooks in dma_map_ops. These hooks were
for dma_sync_single_range_* DMA API, however, the API became obsolete
(the description in DMA API docs was removed too). Architecutres
should use sync_single_for_cpu and sync_single_for_device hooks
instead (as DMA API docs say, they need to support a partial sync).

We could remove dma_sync_single_range_* DMA API completely (no user in
-mm) but the API had been until 2.6.34-rc so I guess that it might be
better to leave it alone for some time.

The first patch is a bug fix and might be 2.6.34-rc material however
the API is not so popular (only net/b44.c uses) and we are already in
-rc5. I guess that it would be fine to merge it in the next merge
window.

=
 arch/ia64/kernel/pci-swiotlb.c           |    2 -
 arch/powerpc/kernel/dma-swiotlb.c        |    4 +-
 arch/powerpc/kernel/dma.c                |   12 +++++-----
 arch/x86/kernel/pci-swiotlb.c            |    2 -
 include/asm-generic/dma-mapping-common.h |   20 +-----------------
 include/linux/dma-mapping.h              |   10 ---------
 include/linux/swiotlb.h                  |   10 ---------
 lib/swiotlb.c                            |   31 ------------------------------
 8 files changed, 10 insertions(+), 81 deletions(-)

[PATCH -mm 1/6] dma-mapping: fix dma_sync_single_range_*

[FUJITA Tomonori]

dma_sync_single_range_for_cpu and dma_sync_single_range_for_device
use a wrong address with a partial synchronization.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: stable@kernel.org
---
 include/asm-generic/dma-mapping-common.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
index e694263..6920695 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -131,7 +131,7 @@ static inline void dma_sync_single_range_for_cpu(struct device *dev,
 		debug_dma_sync_single_range_for_cpu(dev, addr, offset, size, dir);
 
 	} else
-		dma_sync_single_for_cpu(dev, addr, size, dir);
+		dma_sync_single_for_cpu(dev, addr + offset, size, dir);
 }
 
 static inline void dma_sync_single_range_for_device(struct device *dev,
@@ -148,7 +148,7 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
 		debug_dma_sync_single_range_for_device(dev, addr, offset, size, dir);
 
 	} else
-		dma_sync_single_for_device(dev, addr, size, dir);
+		dma_sync_single_for_device(dev, addr + offset, size, dir);
 }
 
 static inline void
-- 
1.6.5

[PATCH -mm 2/6] ia64: remove unnecessary sync_single_range_* in swiotlb_dma_ops

[FUJITA Tomonori]

sync_single_range_for_cpu and sync_single_range_for_device hooks in
swiotlb_dma_ops are unnecessary because sync_single_for_cpu and
sync_single_for_device are used there.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/ia64/kernel/pci-swiotlb.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/pci-swiotlb.c b/arch/ia64/kernel/pci-swiotlb.c
index 3095654..d9485d9 100644
--- a/arch/ia64/kernel/pci-swiotlb.c
+++ b/arch/ia64/kernel/pci-swiotlb.c
@@ -31,8 +31,6 @@ struct dma_map_ops swiotlb_dma_ops = {
 	.unmap_sg = swiotlb_unmap_sg_attrs,
 	.sync_single_for_cpu = swiotlb_sync_single_for_cpu,
 	.sync_single_for_device = swiotlb_sync_single_for_device,
-	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
-	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device,
 	.dma_supported = swiotlb_dma_supported,
-- 
1.6.5

[PATCH -mm 3/6] x86: remove unnecessary sync_single_range_* in swiotlb_dma_ops

[FUJITA Tomonori]

sync_single_range_for_cpu and sync_single_range_for_device hooks in
swiotlb_dma_ops are unnecessary because sync_single_for_cpu and
sync_single_for_device are used there.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: x86@kernel.org
---
 arch/x86/kernel/pci-swiotlb.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 7d2829d..a5bc528 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -31,8 +31,6 @@ static struct dma_map_ops swiotlb_dma_ops = {
 	.free_coherent = swiotlb_free_coherent,
 	.sync_single_for_cpu = swiotlb_sync_single_for_cpu,
 	.sync_single_for_device = swiotlb_sync_single_for_device,
-	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
-	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device,
 	.map_sg = swiotlb_map_sg_attrs,
-- 
1.6.5

[PATCH -mm 4/6] powerpc: remove unnecessary sync_single_range_* in swiotlb_dma_ops

[FUJITA Tomonori]

sync_single_range_for_cpu and sync_single_range_for_device hooks in
swiotlb_dma_ops are unnecessary because sync_single_for_cpu and
sync_single_for_device are used there.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Becky Bruce <beckyb@kernel.crashing.org>
---
 arch/powerpc/kernel/dma-swiotlb.c |    4 ++--
 arch/powerpc/kernel/dma.c         |   12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-swiotlb.c
index 59c9285..cc79e8d 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -38,8 +38,8 @@ struct dma_map_ops swiotlb_dma_ops = {
 	.dma_supported = swiotlb_dma_supported,
 	.map_page = swiotlb_map_page,
 	.unmap_page = swiotlb_unmap_page,
-	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
-	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
+	.sync_single_for_cpu = swiotlb_sync_single_for_cpu,
+	.sync_single_for_device = swiotlb_sync_single_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device,
 	.mapping_error = swiotlb_dma_mapping_error,
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index 6c1df57..8d1de6f 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -127,11 +127,11 @@ static inline void dma_direct_sync_sg(struct device *dev,
 		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
 }
 
-static inline void dma_direct_sync_single_range(struct device *dev,
-		dma_addr_t dma_handle, unsigned long offset, size_t size,
-		enum dma_data_direction direction)
+static inline void dma_direct_sync_single(struct device *dev,
+					  dma_addr_t dma_handle, size_t size,
+					  enum dma_data_direction direction)
 {
-	__dma_sync(bus_to_virt(dma_handle+offset), size, direction);
+	__dma_sync(bus_to_virt(dma_handle), size, direction);
 }
 #endif
 
@@ -144,8 +144,8 @@ struct dma_map_ops dma_direct_ops = {
 	.map_page	= dma_direct_map_page,
 	.unmap_page	= dma_direct_unmap_page,
 #ifdef CONFIG_NOT_COHERENT_CACHE
-	.sync_single_range_for_cpu 	= dma_direct_sync_single_range,
-	.sync_single_range_for_device 	= dma_direct_sync_single_range,
+	.sync_single_for_cpu 		= dma_direct_sync_single,
+	.sync_single_for_device 	= dma_direct_sync_single,
 	.sync_sg_for_cpu 		= dma_direct_sync_sg,
 	.sync_sg_for_device 		= dma_direct_sync_sg,
 #endif
-- 
1.6.5

[PATCH -mm 5/6] swiotlb: remove unnecessary swiotlb_sync_single_range_*

[FUJITA Tomonori]

swiotlb_sync_single_range_for_cpu and
swiotlb_sync_single_range_for_device are unnecessary because
swiotlb_sync_single_for_cpu and swiotlb_sync_single_for_device can be
used instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 include/linux/swiotlb.h |   10 ----------
 lib/swiotlb.c           |   31 -------------------------------
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index febedcf..81a4e21 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -73,16 +73,6 @@ extern void
 swiotlb_sync_sg_for_device(struct device *hwdev, struct scatterlist *sg,
 			   int nelems, enum dma_data_direction dir);
 
-extern void
-swiotlb_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dev_addr,
-				  unsigned long offset, size_t size,
-				  enum dma_data_direction dir);
-
-extern void
-swiotlb_sync_single_range_for_device(struct device *hwdev, dma_addr_t dev_addr,
-				     unsigned long offset, size_t size,
-				     enum dma_data_direction dir);
-
 extern int
 swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr);
 
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 5fddf72..a009055 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -757,37 +757,6 @@ swiotlb_sync_single_for_device(struct device *hwdev, dma_addr_t dev_addr,
 EXPORT_SYMBOL(swiotlb_sync_single_for_device);
 
 /*
- * Same as above, but for a sub-range of the mapping.
- */
-static void
-swiotlb_sync_single_range(struct device *hwdev, dma_addr_t dev_addr,
-			  unsigned long offset, size_t size,
-			  int dir, int target)
-{
-	swiotlb_sync_single(hwdev, dev_addr + offset, size, dir, target);
-}
-
-void
-swiotlb_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dev_addr,
-				  unsigned long offset, size_t size,
-				  enum dma_data_direction dir)
-{
-	swiotlb_sync_single_range(hwdev, dev_addr, offset, size, dir,
-				  SYNC_FOR_CPU);
-}
-EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_cpu);
-
-void
-swiotlb_sync_single_range_for_device(struct device *hwdev, dma_addr_t dev_addr,
-				     unsigned long offset, size_t size,
-				     enum dma_data_direction dir)
-{
-	swiotlb_sync_single_range(hwdev, dev_addr, offset, size, dir,
-				  SYNC_FOR_DEVICE);
-}
-EXPORT_SYMBOL_GPL(swiotlb_sync_single_range_for_device);
-
-/*
  * Map a set of buffers described by scatterlist in streaming mode for DMA.
  * This is the scatter-gather version of the above swiotlb_map_page
  * interface.  Here the scatter gather list elements are each tagged with the
-- 
1.6.5

[PATCH -mm 6/6] dma-mapping: remove unnecessary sync_single_range_* in dma_map_ops

[FUJITA Tomonori]

sync_single_range_for_cpu and sync_single_range_for_device hooks are
unnecessary because sync_single_for_cpu and sync_single_for_device can
be used instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 include/asm-generic/dma-mapping-common.h |   20 ++------------------
 include/linux/dma-mapping.h              |   10 ----------
 2 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h
index 6920695..0c80bb3 100644
--- a/include/asm-generic/dma-mapping-common.h
+++ b/include/asm-generic/dma-mapping-common.h
@@ -123,15 +123,7 @@ static inline void dma_sync_single_range_for_cpu(struct device *dev,
 						 size_t size,
 						 enum dma_data_direction dir)
 {
-	struct dma_map_ops *ops = get_dma_ops(dev);
-
-	BUG_ON(!valid_dma_direction(dir));
-	if (ops->sync_single_range_for_cpu) {
-		ops->sync_single_range_for_cpu(dev, addr, offset, size, dir);
-		debug_dma_sync_single_range_for_cpu(dev, addr, offset, size, dir);
-
-	} else
-		dma_sync_single_for_cpu(dev, addr + offset, size, dir);
+	dma_sync_single_for_cpu(dev, addr + offset, size, dir);
 }
 
 static inline void dma_sync_single_range_for_device(struct device *dev,
@@ -140,15 +132,7 @@ static inline void dma_sync_single_range_for_device(struct device *dev,
 						    size_t size,
 						    enum dma_data_direction dir)
 {
-	struct dma_map_ops *ops = get_dma_ops(dev);
-
-	BUG_ON(!valid_dma_direction(dir));
-	if (ops->sync_single_range_for_device) {
-		ops->sync_single_range_for_device(dev, addr, offset, size, dir);
-		debug_dma_sync_single_range_for_device(dev, addr, offset, size, dir);
-
-	} else
-		dma_sync_single_for_device(dev, addr + offset, size, dir);
+	dma_sync_single_for_device(dev, addr + offset, size, dir);
 }
 
 static inline void
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ca32ed7..2ea1494 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -40,16 +40,6 @@ struct dma_map_ops {
 	void (*sync_single_for_device)(struct device *dev,
 				       dma_addr_t dma_handle, size_t size,
 				       enum dma_data_direction dir);
-	void (*sync_single_range_for_cpu)(struct device *dev,
-					  dma_addr_t dma_handle,
-					  unsigned long offset,
-					  size_t size,
-					  enum dma_data_direction dir);
-	void (*sync_single_range_for_device)(struct device *dev,
-					     dma_addr_t dma_handle,
-					     unsigned long offset,
-					     size_t size,
-					     enum dma_data_direction dir);
 	void (*sync_sg_for_cpu)(struct device *dev,
 				struct scatterlist *sg, int nents,
 				enum dma_data_direction dir);
-- 
1.6.5

Re: [LKML] [PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

[Konrad Rzeszutek Wilk]

On Tue, Apr 27, 2010 at 09:25:34PM +0900, FUJITA Tomonori wrote:
> This patchset removes useless sync_single_range_for_cpu and
> sync_single_range_for_device hooks in dma_map_ops. These hooks were
> for dma_sync_single_range_* DMA API, however, the API became obsolete
> (the description in DMA API docs was removed too). Architecutres
> should use sync_single_for_cpu and sync_single_for_device hooks
> instead (as DMA API docs say, they need to support a partial sync).
> 
> We could remove dma_sync_single_range_* DMA API completely (no user in
> -mm) but the API had been until 2.6.34-rc so I guess that it might be
> better to leave it alone for some time.
> 
> The first patch is a bug fix and might be 2.6.34-rc material however
> the API is not so popular (only net/b44.c uses) and we are already in
> -rc5. I guess that it would be fine to merge it in the next merge
> window.

Not sure if this is needed since the patches aren't that complex, but I
did do a review of all of them. Hence:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Re: [PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

[Russell King]

On Tue, Apr 27, 2010 at 09:25:34PM +0900, FUJITA Tomonori wrote:
> This patchset removes useless sync_single_range_for_cpu and
> sync_single_range_for_device hooks in dma_map_ops. These hooks were
> for dma_sync_single_range_* DMA API, however, the API became obsolete
> (the description in DMA API docs was removed too). Architecutres
> should use sync_single_for_cpu and sync_single_for_device hooks
> instead (as DMA API docs say, they need to support a partial sync).

If you're going to do this, then please catch all instances - compare
the number of places which contain "dma_sync_single_range" with your
diffstat and you'll notice you've missed a _lot_ of places.

$ git grep -l '\<dma_sync_single_range'
arch/arm/include/asm/dma-mapping.h
arch/avr32/include/asm/dma-mapping.h
arch/blackfin/include/asm/dma-mapping.h
arch/cris/include/asm/dma-mapping.h
arch/frv/include/asm/dma-mapping.h
arch/m68k/include/asm/dma-mapping.h
arch/mips/include/asm/dma-mapping.h
arch/mips/mm/dma-default.c
arch/mn10300/include/asm/dma-mapping.h
arch/parisc/include/asm/dma-mapping.h
arch/xtensa/include/asm/dma-mapping.h
include/asm-generic/dma-mapping-broken.h
include/asm-generic/dma-mapping-common.h
include/linux/ssb/ssb.h

vs.

>  arch/ia64/kernel/pci-swiotlb.c           |    2 -
>  arch/powerpc/kernel/dma-swiotlb.c        |    4 +-
>  arch/powerpc/kernel/dma.c                |   12 +++++-----
>  arch/x86/kernel/pci-swiotlb.c            |    2 -
>  include/asm-generic/dma-mapping-common.h |   20 +-----------------
>  include/linux/dma-mapping.h              |   10 ---------
>  include/linux/swiotlb.h                  |   10 ---------
>  lib/swiotlb.c                            |   31 ------------------------------
>  8 files changed, 10 insertions(+), 81 deletions(-)

It seems there's users of this API in linux/ssb/ssb.h.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

[FUJITA Tomonori]

On Mon, 10 May 2010 10:29:09 +0100
Russell King <rmk@arm.linux.org.uk> wrote:

> On Tue, Apr 27, 2010 at 09:25:34PM +0900, FUJITA Tomonori wrote:
> > This patchset removes useless sync_single_range_for_cpu and
> > sync_single_range_for_device hooks in dma_map_ops. These hooks were
> > for dma_sync_single_range_* DMA API, however, the API became obsolete
> > (the description in DMA API docs was removed too). Architecutres
> > should use sync_single_for_cpu and sync_single_for_device hooks
> > instead (as DMA API docs say, they need to support a partial sync).
> 
> If you're going to do this, then please catch all instances - compare
> the number of places which contain "dma_sync_single_range" with your
> diffstat and you'll notice you've missed a _lot_ of places.
> 
> $ git grep -l '\<dma_sync_single_range'
> arch/arm/include/asm/dma-mapping.h
> arch/avr32/include/asm/dma-mapping.h
> arch/blackfin/include/asm/dma-mapping.h
> arch/cris/include/asm/dma-mapping.h
> arch/frv/include/asm/dma-mapping.h
> arch/m68k/include/asm/dma-mapping.h
> arch/mips/include/asm/dma-mapping.h
> arch/mips/mm/dma-default.c
> arch/mn10300/include/asm/dma-mapping.h
> arch/parisc/include/asm/dma-mapping.h
> arch/xtensa/include/asm/dma-mapping.h
> include/asm-generic/dma-mapping-broken.h
> include/asm-generic/dma-mapping-common.h
> include/linux/ssb/ssb.h

The patchset doesn't remove dma_sync_single_range_for_{cpu|device}
API. It removes sync_single_range_for_cpu and
sync_single_range_for_device hooks in struct dma_map_ops. The above
architectures doesn't use struct dma_map_ops so not related with this
patchset.

With this patchset, on the architectures using dma_map_ops,
dma_sync_single_range_for_cpu and dma_sync_single_range_for_device is
just the wrapper of dma_sync_single_for_cpu and
dma_sync_single_for_device.

static inline void dma_sync_single_range_for_cpu(struct device *dev,
						 dma_addr_t addr,
						 unsigned long offset,
						 size_t size,
						 enum dma_data_direction dir)
{
	dma_sync_single_for_cpu(dev, addr + offset, size, dir);
}

static inline void dma_sync_single_range_for_device(struct device *dev,
						    dma_addr_t addr,
						    unsigned long offset,
						    size_t size,
						    enum dma_data_direction dir)
{
	dma_sync_single_for_device(dev, addr + offset, size, dir);
}

dma_sync_single_range_for_cpu and dma_sync_single_range_for_device are
obsolete but the API had been for some time so I guess that we can't
just remove it; we need to add it to feature-removal-schedule, put
__deprecated, and wait for some time. But the API is rarely used so
I'm not against removing it now.


> vs.
> 
> >  arch/ia64/kernel/pci-swiotlb.c           |    2 -
> >  arch/powerpc/kernel/dma-swiotlb.c        |    4 +-
> >  arch/powerpc/kernel/dma.c                |   12 +++++-----
> >  arch/x86/kernel/pci-swiotlb.c            |    2 -
> >  include/asm-generic/dma-mapping-common.h |   20 +-----------------
> >  include/linux/dma-mapping.h              |   10 ---------
> >  include/linux/swiotlb.h                  |   10 ---------
> >  lib/swiotlb.c                            |   31 ------------------------------
> >  8 files changed, 10 insertions(+), 81 deletions(-)
> 
> It seems there's users of this API in linux/ssb/ssb.h.

As I wrote, the patchset doesn't remove the
dma_sync_single_range_for_{cpu|device} API so ssb.h is fine. However,
the -mm has patches to remove dma_sync_single_range_for_{cpu|device}
usage in ssb.h. There should be no user of this API in the -mm.