2010-04-27 12:26:20

by FUJITA Tomonori

[permalink] [raw]
Subject: [PATCH -mm 0/6] remove unnecessary sync_single_range_* in dma_map_ops

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


2010-04-27 12:26:24

by FUJITA Tomonori

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

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 <[email protected]>
Cc: [email protected]
---
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

2010-04-27 12:26:27

by FUJITA Tomonori

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

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 <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
---
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

2010-04-27 12:26:16

by FUJITA Tomonori

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

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 <[email protected]>
---
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

2010-04-27 12:27:10

by FUJITA Tomonori

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

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 <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Becky Bruce <[email protected]>
---
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

2010-04-27 12:26:19

by FUJITA Tomonori

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

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 <[email protected]>
---
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

2010-04-27 12:30:40

by FUJITA Tomonori

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

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 <[email protected]>
Cc: [email protected]
---
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

2010-04-27 17:08:21

by Konrad Rzeszutek Wilk

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

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 <[email protected]>