[PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]

We have BUG_ON() right for this, don't we?

Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de>

---
commit e3f6da52c54970711fb90ffb0f6765f8fd0ee93e
tree 093ee6b4d8fb087a54c2c90e006b66783186aaff
parent e7156df18ec5f0ee8c9fe7ac0c0367ff9a1532e8
author Rolf Eike Beer <eike-kernel@sf-tec.de> Fri, 28 Jul 2006 09:27:31 +0200
committer Rolf Eike Beer <beer@siso-eb-i34d.silicon-software.de> Fri, 28 Jul 2006 09:27:31 +0200

 include/asm-i386/dma-mapping.h |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/asm-i386/dma-mapping.h b/include/asm-i386/dma-mapping.h
index 9cf20ca..576ae01 100644
--- a/include/asm-i386/dma-mapping.h
+++ b/include/asm-i386/dma-mapping.h
@@ -21,8 +21,7 @@ static inline dma_addr_t
 dma_map_single(struct device *dev, void *ptr, size_t size,
 	       enum dma_data_direction direction)
 {
-	if (direction == DMA_NONE)
-		BUG();
+	BUG_ON(direction == DMA_NONE);
 	WARN_ON(size == 0);
 	flush_write_buffers();
 	return virt_to_phys(ptr);
@@ -32,8 +31,7 @@ static inline void
 dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
 		 enum dma_data_direction direction)
 {
-	if (direction == DMA_NONE)
-		BUG();
+	BUG_ON(direction == DMA_NONE);
 }
 
 static inline int
@@ -42,8 +40,7 @@ dma_map_sg(struct device *dev, struct sc
 {
 	int i;
 
-	if (direction == DMA_NONE)
-		BUG();
+	BUG_ON(direction == DMA_NONE);
 	WARN_ON(nents == 0 || sg[0].length == 0);
 
 	for (i = 0; i < nents; i++ ) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[Andrew Morton]

On Fri, 28 Jul 2006 09:28:49 +0200
Rolf Eike Beer <eike-kernel@sf-tec.de> wrote:

> We have BUG_ON() right for this, don't we?

Well yes, but there are over a thousand BUG->BUG_ON conversion
possibilities in the tree.  If people start sending them three-at-a-time
we'll all go mad.

So.  If we're going to do this, bigger patches, please.

Re: [PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[Pavel Machek]

Hi!

> > We have BUG_ON() right for this, don't we?
> 
> Well yes, but there are over a thousand BUG->BUG_ON conversion
> possibilities in the tree.  If people start sending them three-at-a-time
> we'll all go mad.
> 
> So.  If we're going to do this, bigger patches, please.

If we are going that way... I guess we should specify if BUG_ON() has
to evaluate its arguments even if it is compiled out...

Or probably better pecify that BUG_ON() must not have side effects?

							Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: [PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[Adrian Bunk]

On Sat, Aug 05, 2006 at 11:37:04AM +0000, Pavel Machek wrote:
> Hi!
> 
> > > We have BUG_ON() right for this, don't we?
> > 
> > Well yes, but there are over a thousand BUG->BUG_ON conversion
> > possibilities in the tree.  If people start sending them three-at-a-time
> > we'll all go mad.
> > 
> > So.  If we're going to do this, bigger patches, please.
> 
> If we are going that way... I guess we should specify if BUG_ON() has
> to evaluate its arguments even if it is compiled out...
>...

This is already implemented for ages.

> 							Pavel

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] Use BUG_ON(foo) instead of "if (foo) BUG()" in include/asm-i386/dma-mapping.h

[Muli Ben-Yehuda]

On Fri, Jul 28, 2006 at 09:28:49AM +0200, Rolf Eike Beer wrote:

> We have BUG_ON() right for this, don't we?

Even better, we have valid_dma_direction() in x86-64. Care to move it
to generic code and update i386 to use it?

Cheers,
Muli

[PATCH] Move valid_dma_direction() from x86_64 to generic code

[Rolf Eike Beer]

As suggested by Muli Ben-Yehuda this function is moved to generic code as
may be useful for all archs.

Signed-off-by: Rolf Eike Beer <eike-kernel@sf-tec.de>

---
commit 9f990495512e3f106ce56f885a675636b47ff421
tree ad2864a5204c57d9836ff2360d6bf80c0d2246e3
parent 3ac898b16fb75cd996ec98fa578ea06b6ffb2760
author Rolf Eike Beer <eike-kernel@sf-tec.de> Wed, 02 Aug 2006 17:07:56 +0200
committer Rolf Eike Beer <beer@siso-eb-i34d.silicon-software.de> Wed, 02 Aug 2006 17:07:56 +0200

 include/asm-x86_64/dma-mapping.h |    7 -------
 include/linux/dma-mapping.h      |    7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/asm-x86_64/dma-mapping.h b/include/asm-x86_64/dma-mapping.h
index b6da83d..10174b1 100644
--- a/include/asm-x86_64/dma-mapping.h
+++ b/include/asm-x86_64/dma-mapping.h
@@ -55,13 +55,6 @@ extern dma_addr_t bad_dma_address;
 extern struct dma_mapping_ops* dma_ops;
 extern int iommu_merge;
 
-static inline int valid_dma_direction(int dma_direction)
-{
-	return ((dma_direction == DMA_BIDIRECTIONAL) ||
-		(dma_direction == DMA_TO_DEVICE) ||
-		(dma_direction == DMA_FROM_DEVICE));
-}
-
 static inline int dma_mapping_error(dma_addr_t dma_addr)
 {
 	if (dma_ops->mapping_error)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 635690c..ff203c4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -24,6 +24,13 @@ #define DMA_29BIT_MASK	0x000000001ffffff
 #define DMA_28BIT_MASK	0x000000000fffffffULL
 #define DMA_24BIT_MASK	0x0000000000ffffffULL
 
+static inline int valid_dma_direction(int dma_direction)
+{
+	return ((dma_direction == DMA_BIDIRECTIONAL) ||
+		(dma_direction == DMA_TO_DEVICE) ||
+		(dma_direction == DMA_FROM_DEVICE));
+}
+
 #include <asm/dma-mapping.h>
 
 /* Backwards compat, remove in 2.7.x */

Re: [PATCH] Move valid_dma_direction() from x86_64 to generic code

[Muli Ben-Yehuda]

On Wed, Aug 02, 2006 at 05:20:40PM +0200, Rolf Eike Beer wrote:

> As suggested by Muli Ben-Yehuda this function is moved to generic code as
> may be useful for all archs.

I like it, but ...

> diff --git a/include/asm-x86_64/dma-mapping.h b/include/asm-x86_64/dma-mapping.h
> index b6da83d..10174b1 100644
> --- a/include/asm-x86_64/dma-mapping.h
> +++ b/include/asm-x86_64/dma-mapping.h
> @@ -55,13 +55,6 @@ extern dma_addr_t bad_dma_address;
>  extern struct dma_mapping_ops* dma_ops;
>  extern int iommu_merge;
>  
> -static inline int valid_dma_direction(int dma_direction)
> -{
> -	return ((dma_direction == DMA_BIDIRECTIONAL) ||
> -		(dma_direction == DMA_TO_DEVICE) ||
> -		(dma_direction == DMA_FROM_DEVICE));
> -}
> -

Several files include asm/dma-mapping.h directly, which will now cause
them to fail to compile on x86-64 due to the missing definition for
valid_dma_direction, unless by chance another header already brought
it in indirectly. I guess the right thing to do is convert them all to
using linux/dma-mapping.h instead.

./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>
./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>

Cheers,
Muli

Re: [PATCH] Move valid_dma_direction() from x86_64 to generic code

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

Muli Ben-Yehuda wrote:
> On Wed, Aug 02, 2006 at 05:20:40PM +0200, Rolf Eike Beer wrote:
> > As suggested by Muli Ben-Yehuda this function is moved to generic code as
> > may be useful for all archs.
>
> I like it, but ...

> Several files include asm/dma-mapping.h directly, which will now cause
> them to fail to compile on x86-64 due to the missing definition for
> valid_dma_direction, unless by chance another header already brought
> it in indirectly. I guess the right thing to do is convert them all to
> using linux/dma-mapping.h instead.

Yes, akpm already fixed it up. Sorry guys, currently no x86 kernel around 
here. Need to fix that.

> ./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
> ./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
> ./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
> ./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>

I suspect it to be a bug anyway that every of this files ever included 
asm/dma-mapping.h.

> ./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>

This is perfectly valid, isn't it :)

Eike

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Move valid_dma_direction() from x86_64 to generic code

[Muli Ben-Yehuda]

On Thu, Aug 03, 2006 at 08:25:19AM +0200, Rolf Eike Beer wrote:

> > ./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
> > ./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
> > ./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
> > ./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>
> 
> I suspect it to be a bug anyway that every of this files ever included 
> asm/dma-mapping.h.

Agreed wrt the fs_enet and fec_8xx; the swiotlb stuff I dimly recall I
had a reason for. I'll take a look in bit to verify akpm's fix works.

> > ./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>
> 
> This is perfectly valid, isn't it :)

:-)

Cheers,
Muli

IOMMU (Calgary) patches

[Duran, Leo]

Andi, Jon, Muli,

I'm trying to build with the latest IOMMU patches for x86_64, so, I've
pulled down 2.6.17, and the 2.6.18-rc3 patches... But, that's obviously
lagging behind a bit.

Is there a source tree that you guys work from?
(I'm hoping that there's a better mechanism than keeping track of
patches as they are submitted here).

Leo Duran.

-----Original Message-----
From: Muli Ben-Yehuda [mailto:muli@il.ibm.com] 
Sent: Thursday, August 03, 2006 2:24 AM
To: Rolf Eike Beer
Cc: Andrew Morton; linux-kernel@vger.kernel.org; Andi Kleen;
discuss@x86-64.org
Subject: [discuss] Re: [PATCH] Move valid_dma_direction() from x86_64 to
generic code

On Thu, Aug 03, 2006 at 08:25:19AM +0200, Rolf Eike Beer wrote:

> > ./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
> > ./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
> > ./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
> > ./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>
> 
> I suspect it to be a bug anyway that every of this files ever included

> asm/dma-mapping.h.

Agreed wrt the fs_enet and fec_8xx; the swiotlb stuff I dimly recall I
had a reason for. I'll take a look in bit to verify akpm's fix works.

> > ./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>
> 
> This is perfectly valid, isn't it :)

:-)

Cheers,
Muli

Re: IOMMU (Calgary) patches

[Jon Mason]

On Thu, Aug 03, 2006 at 04:10:55PM -0500, Duran, Leo wrote:
> Andi, Jon, Muli,
> 
> I'm trying to build with the latest IOMMU patches for x86_64, so, I've
> pulled down 2.6.17, and the 2.6.18-rc3 patches... But, that's obviously
> lagging behind a bit.
> 
> Is there a source tree that you guys work from?
Hey Leo,

I personally use the mercurial tree hosted on kernel.org
(http://www.kernel.org/hg/linux-2.6).  Alternatively, you can use git to
access the latest tree.

> (I'm hoping that there's a better mechanism than keeping track of
> patches as they are submitted here).

Andrew Morton is good about pulling in the IOMMU patches as soon as we
push them.  So the -mm kernel is an option.  Andi also queues up the
patches in his firstfloor repository.  So you can pull the patches from
there.

So, if you can't wait for the latest kernel and don't want to
get the latest kernel and patch it with Andi's patches on firstfloor,
then the mm kernel will be the best option.  But, the mercurial tree I
mentioned above should be sufficient.

Thanks,
Jon

> Leo Duran.
> 
> -----Original Message-----
> From: Muli Ben-Yehuda [mailto:muli@il.ibm.com] 
> Sent: Thursday, August 03, 2006 2:24 AM
> To: Rolf Eike Beer
> Cc: Andrew Morton; linux-kernel@vger.kernel.org; Andi Kleen;
> discuss@x86-64.org
> Subject: [discuss] Re: [PATCH] Move valid_dma_direction() from x86_64 to
> generic code
> 
> On Thu, Aug 03, 2006 at 08:25:19AM +0200, Rolf Eike Beer wrote:
> 
> > > ./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
> > > ./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
> > > ./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
> > > ./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>
> > 
> > I suspect it to be a bug anyway that every of this files ever included
> 
> > asm/dma-mapping.h.
> 
> Agreed wrt the fs_enet and fec_8xx; the swiotlb stuff I dimly recall I
> had a reason for. I'll take a look in bit to verify akpm's fix works.
> 
> > > ./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>
> > 
> > This is perfectly valid, isn't it :)
> 
> :-)
> 
> Cheers,
> Muli
> 
> 
> 
> 

RE: IOMMU (Calgary) patches

[Duran, Leo]

Jon,
Many thanks.  That helps.
Leo.

-----Original Message-----
From: Jon Mason [mailto:jdmason@us.ibm.com] 
Sent: Thursday, August 03, 2006 4:26 PM
To: Duran, Leo
Cc: Muli Ben-Yehuda; Andi Kleen; linux-kernel@vger.kernel.org;
discuss@x86-64.org
Subject: Re: IOMMU (Calgary) patches

On Thu, Aug 03, 2006 at 04:10:55PM -0500, Duran, Leo wrote:
> Andi, Jon, Muli,
> 
> I'm trying to build with the latest IOMMU patches for x86_64, so, I've
> pulled down 2.6.17, and the 2.6.18-rc3 patches... But, that's
obviously
> lagging behind a bit.
> 
> Is there a source tree that you guys work from?
Hey Leo,

I personally use the mercurial tree hosted on kernel.org
(http://www.kernel.org/hg/linux-2.6).  Alternatively, you can use git to
access the latest tree.

> (I'm hoping that there's a better mechanism than keeping track of
> patches as they are submitted here).

Andrew Morton is good about pulling in the IOMMU patches as soon as we
push them.  So the -mm kernel is an option.  Andi also queues up the
patches in his firstfloor repository.  So you can pull the patches from
there.

So, if you can't wait for the latest kernel and don't want to
get the latest kernel and patch it with Andi's patches on firstfloor,
then the mm kernel will be the best option.  But, the mercurial tree I
mentioned above should be sufficient.

Thanks,
Jon

> Leo Duran.
> 
> -----Original Message-----
> From: Muli Ben-Yehuda [mailto:muli@il.ibm.com] 
> Sent: Thursday, August 03, 2006 2:24 AM
> To: Rolf Eike Beer
> Cc: Andrew Morton; linux-kernel@vger.kernel.org; Andi Kleen;
> discuss@x86-64.org
> Subject: [discuss] Re: [PATCH] Move valid_dma_direction() from x86_64
to
> generic code
> 
> On Thu, Aug 03, 2006 at 08:25:19AM +0200, Rolf Eike Beer wrote:
> 
> > > ./arch/x86_64/kernel/pci-swiotlb.c:6:#include <asm/dma-mapping.h>
> > > ./drivers/net/fec_8xx/fec_main.c:40:#include <asm/dma-mapping.h>
> > > ./drivers/net/fs_enet/fs_enet.h:11:#include <asm/dma-mapping.h>
> > > ./include/asm-x86_64/swiotlb.h:5:#include <asm/dma-mapping.h>
> > 
> > I suspect it to be a bug anyway that every of this files ever
included
> 
> > asm/dma-mapping.h.
> 
> Agreed wrt the fs_enet and fec_8xx; the swiotlb stuff I dimly recall I
> had a reason for. I'll take a look in bit to verify akpm's fix works.
> 
> > > ./include/linux/dma-mapping.h:27:#include <asm/dma-mapping.h>
> > 
> > This is perfectly valid, isn't it :)
> 
> :-)
> 
> Cheers,
> Muli
> 
> 
> 
> 

Re: [discuss] IOMMU (Calgary) patches

[Andi Kleen]

On Thursday 03 August 2006 23:10, Duran, Leo wrote:
> Andi, Jon, Muli,
> 
> I'm trying to build with the latest IOMMU patches for x86_64, so, I've
> pulled down 2.6.17, and the 2.6.18-rc3 patches... But, that's obviously
> lagging behind a bit.
> 
> Is there a source tree that you guys work from?

My latest source tree which contains all the submitted Calgary updates
is in ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt/patches/  (in quilt format)

-Andi

[PATCH] Use valid_dma_direction() in include/asm-i386/dma-mapping.h

[Rolf Eike Beer]

Now that the generic DMA code has a function to decide if a given DMA
mapping is valid use it. This will catch cases where direction is not any
of the defined enum values but some random number outside the valid range.
The current implementation will only catch the defined but invalid case
DMA_NONE.

Signed-off-by: Rolf Eike Beer

---
commit 61f76f37d18da432ea1b55b92c98dd39388077f0
tree e8c5d79bdb2e2b786b4d5c719d64f28dfc54c4fb
parent 9f990495512e3f106ce56f885a675636b47ff421
author Rolf Eike Beer <eike-kernel@sf-tec.de> Wed, 02 Aug 2006 17:11:04 +0200
committer Rolf Eike Beer <beer@siso-eb-i34d.silicon-software.de> Wed, 02 Aug 2006 17:11:04 +0200

 include/asm-i386/dma-mapping.h |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/asm-i386/dma-mapping.h b/include/asm-i386/dma-mapping.h
index 576ae01..81999a3 100644
--- a/include/asm-i386/dma-mapping.h
+++ b/include/asm-i386/dma-mapping.h
@@ -21,7 +21,7 @@ static inline dma_addr_t
 dma_map_single(struct device *dev, void *ptr, size_t size,
 	       enum dma_data_direction direction)
 {
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 	WARN_ON(size == 0);
 	flush_write_buffers();
 	return virt_to_phys(ptr);
@@ -31,7 +31,7 @@ static inline void
 dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
 		 enum dma_data_direction direction)
 {
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 }
 
 static inline int
@@ -40,7 +40,7 @@ dma_map_sg(struct device *dev, struct sc
 {
 	int i;
 
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 	WARN_ON(nents == 0 || sg[0].length == 0);
 
 	for (i = 0; i < nents; i++ ) {
@@ -57,7 +57,7 @@ static inline dma_addr_t
 dma_map_page(struct device *dev, struct page *page, unsigned long offset,
 	     size_t size, enum dma_data_direction direction)
 {
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 	return page_to_phys(page) + offset;
 }
 
@@ -65,7 +65,7 @@ static inline void
 dma_unmap_page(struct device *dev, dma_addr_t dma_address, size_t size,
 	       enum dma_data_direction direction)
 {
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 }
 
 
@@ -73,7 +73,7 @@ static inline void
 dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nhwentries,
 	     enum dma_data_direction direction)
 {
-	BUG_ON(direction == DMA_NONE);
+	BUG_ON(!valid_dma_direction(direction));
 }
 
 static inline void

Re: [PATCH] Use valid_dma_direction() in include/asm-i386/dma-mapping.h

[Muli Ben-Yehuda]

On Wed, Aug 02, 2006 at 05:22:35PM +0200, Rolf Eike Beer wrote:

> Now that the generic DMA code has a function to decide if a given DMA
> mapping is valid use it. This will catch cases where direction is not any
> of the defined enum values but some random number outside the valid range.
> The current implementation will only catch the defined but invalid case
> DMA_NONE.
> 
> Signed-off-by: Rolf Eike Beer

Acked-by: Muli Ben-Yehuda <muli@il.ibm.com>

... but see my comments to your other patch this one depends on.

Cheers,
Muli