[PATCH] powerpc: simplify dma_ops bug conditions

[PATCH] powerpc: simplify dma_ops bug conditions

[Jeremy Kerr]

Use BUG_ON rather than BUG to simplify the dma_ops handing,
and remove the now-unnecessary return cases.

Booted on pseries.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---

 dma_64.c |   65 +++++++++++++++++++++++++++------------------------------------
 1 file changed, 28 insertions(+), 37 deletions(-)

Index: linux-2.6/arch/powerpc/kernel/dma_64.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/dma_64.c
+++ linux-2.6/arch/powerpc/kernel/dma_64.c
@@ -35,10 +35,9 @@ int dma_supported(struct device *dev, u6
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		return dma_ops->dma_supported(dev, mask);
-	BUG();
-	return 0;
+	BUG_ON(!dma_ops);
+
+	return dma_ops->dma_supported(dev, mask);
 }
 EXPORT_SYMBOL(dma_supported);
 
@@ -66,10 +65,9 @@ void *dma_alloc_coherent(struct device *
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		return dma_ops->alloc_coherent(dev, size, dma_handle, flag);
-	BUG();
-	return NULL;
+	BUG_ON(!dma_ops);
+
+	return dma_ops->alloc_coherent(dev, size, dma_handle, flag);
 }
 EXPORT_SYMBOL(dma_alloc_coherent);
 
@@ -78,10 +76,9 @@ void dma_free_coherent(struct device *de
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		dma_ops->free_coherent(dev, size, cpu_addr, dma_handle);
-	else
-		BUG();
+	BUG_ON(!dma_ops);
+
+	dma_ops->free_coherent(dev, size, cpu_addr, dma_handle);
 }
 EXPORT_SYMBOL(dma_free_coherent);
 
@@ -90,10 +87,9 @@ dma_addr_t dma_map_single(struct device 
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		return dma_ops->map_single(dev, cpu_addr, size, direction);
-	BUG();
-	return (dma_addr_t)0;
+	BUG_ON(!dma_ops);
+
+	return dma_ops->map_single(dev, cpu_addr, size, direction);
 }
 EXPORT_SYMBOL(dma_map_single);
 
@@ -102,10 +98,9 @@ void dma_unmap_single(struct device *dev
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		dma_ops->unmap_single(dev, dma_addr, size, direction);
-	else
-		BUG();
+	BUG_ON(!dma_ops);
+
+	dma_ops->unmap_single(dev, dma_addr, size, direction);
 }
 EXPORT_SYMBOL(dma_unmap_single);
 
@@ -115,11 +110,10 @@ dma_addr_t dma_map_page(struct device *d
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		return dma_ops->map_single(dev,
-				(page_address(page) + offset), size, direction);
-	BUG();
-	return (dma_addr_t)0;
+	BUG_ON(!dma_ops);
+
+	return dma_ops->map_single(dev, page_address(page) + offset, size,
+			direction);
 }
 EXPORT_SYMBOL(dma_map_page);
 
@@ -128,10 +122,9 @@ void dma_unmap_page(struct device *dev, 
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		dma_ops->unmap_single(dev, dma_address, size, direction);
-	else
-		BUG();
+	BUG_ON(!dma_ops);
+
+	dma_ops->unmap_single(dev, dma_address, size, direction);
 }
 EXPORT_SYMBOL(dma_unmap_page);
 
@@ -140,10 +133,9 @@ int dma_map_sg(struct device *dev, struc
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		return dma_ops->map_sg(dev, sg, nents, direction);
-	BUG();
-	return 0;
+	BUG_ON(!dma_ops);
+
+	return dma_ops->map_sg(dev, sg, nents, direction);
 }
 EXPORT_SYMBOL(dma_map_sg);
 
@@ -152,9 +144,8 @@ void dma_unmap_sg(struct device *dev, st
 {
 	struct dma_mapping_ops *dma_ops = get_dma_ops(dev);
 
-	if (dma_ops)
-		dma_ops->unmap_sg(dev, sg, nhwentries, direction);
-	else
-		BUG();
+	BUG_ON(!dma_ops);
+
+	dma_ops->unmap_sg(dev, sg, nhwentries, direction);
 }
 EXPORT_SYMBOL(dma_unmap_sg);

Re: [PATCH] powerpc: simplify dma_ops bug conditions

[Muli Ben-Yehuda]

On Thu, Jul 13, 2006 at 04:32:52PM +1000, Jeremy Kerr wrote:

> Use BUG_ON rather than BUG to simplify the dma_ops handing,
> and remove the now-unnecessary return cases.
> 
> Booted on pseries.

Is the BUG_ON() necessary? the next line just goes and deref's it,
which should lead to a shiny NULL pointer deref.

Cheers,
Muli

Re: [PATCH] powerpc: simplify dma_ops bug conditions

[Jeremy Kerr]

Muli,

> Is the BUG_ON() necessary? the next line just goes and deref's it,
> which should lead to a shiny NULL pointer deref.

No, it's not really necessary, more a style thing. The BUG_ON only adds 
an extra 3 instructions (subfic, addi and tdnei), which should be tiny 
compared to the indirect branch.

Also, it makes it a little easier for a user to understand: "there's a 
bug in the kernel" vs. "invalid memory access".


Jeremy

Re: [PATCH] powerpc: simplify dma_ops bug conditions

[Muli Ben-Yehuda]

On Mon, Jul 17, 2006 at 01:55:09PM +1000, Jeremy Kerr wrote:

> No, it's not really necessary, more a style thing. The BUG_ON only adds 
> an extra 3 instructions (subfic, addi and tdnei), which should be tiny 
> compared to the indirect branch.

When we were submitting a patch to add a
BUG_ON(!valid_dma_data_direction(dir)) to x86-64's DMA-API, Jeff
Garzik was vehemently opposed to adding a few instructions to "one of
the hottest paths in the kernel".

Cheers,
Muli