Re: 2.6.22-rc1-mm1 - s390 vs. md

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Cornelia Huck]

On Tue, 15 May 2007 20:19:14 -0700,
Andrew Morton <akpm@linux-foundation.org> wrote:

> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc1/2.6.22-rc1-mm1/

Doesn't build on s390 when selecting the md menu:

drivers/built-in.o(.text+0x438ae): In function `async_xor':
: undefined reference to `dma_map_page'
drivers/built-in.o(.text+0x43aac): In function `async_xor':
: undefined reference to `dma_map_page'
drivers/built-in.o(.text+0x43d2e): In function `async_xor_zero_sum':
: undefined reference to `dma_map_page'
drivers/built-in.o(.text+0x43f50): In function `async_memcpy':
: undefined reference to `dma_map_page'
drivers/built-in.o(.text+0x43f90): In function `async_memcpy':
: undefined reference to `dma_map_page'
drivers/built-in.o(.text+0x4423e): more undefined references to
`dma_map_page' follow

This is caused by the following in drivers/md/Kconfig:

menuconfig MD
        bool "Multiple devices driver support (RAID and LVM)"
        depends on BLOCK
        select ASYNC_TX_DMA
        help
          Support multiple physical spindles through a single logical device.
          Required for RAID and logical volume management.

ASYNC_TX_DMA is defined in drivers/dma/Kconfig, which has

menu "DMA Engine support"
        depends on !S390

but unfortunately ASYNC_TX_DMA depends neither on the menu nor
on !S390. (I think it was just an unknown symbol on s390 before
Martin's Kconfig rework, so I could build older -mm kernels.)

Currently, the only md stuff depending on ASYNC_TX_DMA is MD_RAID456
(which means it doesn't work on s390 anymore, which is bad enough).
With the select statement, no md stuff can be build on s390 at all (and
I really don't see why ASYNC_TX_DMA should be forced upon all md
users)...

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> 
> On Tue, 15 May 2007 20:19:14 -0700,
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> >
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-
> rc1/2.6.22-rc1-mm1/
> 
> Doesn't build on s390 when selecting the md menu:
> 
> drivers/built-in.o(.text+0x438ae): In function `async_xor':
> : undefined reference to `dma_map_page'
> drivers/built-in.o(.text+0x43aac): In function `async_xor':
> : undefined reference to `dma_map_page'
> drivers/built-in.o(.text+0x43d2e): In function `async_xor_zero_sum':
> : undefined reference to `dma_map_page'
> drivers/built-in.o(.text+0x43f50): In function `async_memcpy':
> : undefined reference to `dma_map_page'
> drivers/built-in.o(.text+0x43f90): In function `async_memcpy':
> : undefined reference to `dma_map_page'
> drivers/built-in.o(.text+0x4423e): more undefined references to
> `dma_map_page' follow
> 
> This is caused by the following in drivers/md/Kconfig:
> 
> menuconfig MD
>         bool "Multiple devices driver support (RAID and LVM)"
>         depends on BLOCK
>         select ASYNC_TX_DMA
>         help
>           Support multiple physical spindles through a single logical
device.
>           Required for RAID and logical volume management.

The rationale for the 'select' here was to attempt to prevent user
confusion since MD_RAID456 now depends on ASYNC_TX_DMA.

> 
> ASYNC_TX_DMA is defined in drivers/dma/Kconfig, which has
> 
> menu "DMA Engine support"
>         depends on !S390
> 
> but unfortunately ASYNC_TX_DMA depends neither on the menu nor
> on !S390. (I think it was just an unknown symbol on s390 before
> Martin's Kconfig rework, so I could build older -mm kernels.)
> 
> Currently, the only md stuff depending on ASYNC_TX_DMA is MD_RAID456
> (which means it doesn't work on s390 anymore, which is bad enough).
> With the select statement, no md stuff can be build on s390 at all
(and
> I really don't see why ASYNC_TX_DMA should be forced upon all md
> users)...

I agree it should not be forced on all users, I will push the following
change:

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 4a1b77e..fd29a54 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -8,7 +8,6 @@ menu "Multi-device support (RAID and LVM)"

 config MD
        bool "Multiple devices driver support (RAID and LVM)"
-       select ASYNC_TX_DMA
        help
          Support multiple physical spindles through a single logical
device.
          Required for RAID and logical volume management.
@@ -109,7 +108,8 @@ config MD_RAID10

 config MD_RAID456
        tristate "RAID-4/RAID-5/RAID-6 mode"
-       depends on BLK_DEV_MD && ASYNC_TX_DMA
+       depends on BLK_DEV_MD
+       select ASYNC_TX_DMA
        ---help---
          A RAID-5 set of N drives with a capacity of C MB per drive
provides
          the capacity of C * (N - 1) MB, and protects against a failure

However this still will not allow s390 to build MD_RAID456.  This
dependency is in place because the xor.o object has moved from
drivers/md to drivers/dma.  The goal of the interface is to support
using offload engines when they are present, and use software routines
(like xor_block) when engines are not available.  In other words, the
intent is that DMA_ENGINE=n && ASYNC_TX_DMA=y is a valid configuration.

Can we rework the !S390 change to the DMA_ENGINE menu?  It seems to me
that S390 should follow the ARM example and only enable the driver menus
they want in arch/s390/Kconfig, no?

...

On a closer look, it seems async_tx should be its own directory like
crypto...  I'll post the incremental changes to the md-accel git tree
for review.

Dan

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Williams, Dan J
> On a closer look, it seems async_tx should be its own directory like
crypto...
> I'll post the incremental changes to the md-accel git tree for review.
> 
Here is a copy of the patch added to the md-accel series
(git://lost.foo-projects.org/~dwillia2/git/iop md-accel-linus).  This
along with some other code re-factoring makes async_tx mimic the
organization of crypto, and should resolve the S390 clash.

---

async_tx: add the Kconfig infrastructure for async_tx

From: Dan Williams <dan.j.williams@intel.com>

async_tx is similar to crypto in that there is an api component and a
drivers component.

* Add 'source "async_tx/Kconfig"' to the per architecture Kconfig files,
  for each architecture that sources drivers/md/Kconfig (which appears
to be
  all of them).
* Add 'select' statements for the subsystems that use async_tx
(md-raid4,5)

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

 arch/alpha/Kconfig     |    1 +
 arch/arm/Kconfig       |    2 ++
 arch/arm26/Kconfig     |    2 ++
 arch/avr32/Kconfig     |    2 ++
 arch/blackfin/Kconfig  |    2 ++
 arch/cris/Kconfig      |    2 ++
 arch/frv/Kconfig       |    2 ++
 arch/h8300/Kconfig     |    2 ++
 arch/i386/Kconfig      |    2 ++
 arch/ia64/Kconfig      |    2 ++
 arch/m32r/Kconfig      |    2 ++
 arch/m68k/Kconfig      |    2 ++
 arch/m68knommu/Kconfig |    2 ++
 arch/mips/Kconfig      |    2 ++
 arch/parisc/Kconfig    |    2 ++
 arch/powerpc/Kconfig   |    2 ++
 arch/ppc/Kconfig       |    2 ++
 arch/s390/Kconfig      |    2 ++
 arch/sh/Kconfig        |    2 ++
 arch/sh64/Kconfig      |    2 ++
 arch/sparc/Kconfig     |    2 ++
 arch/sparc64/Kconfig   |    2 ++
 arch/um/Kconfig        |    2 ++
 arch/v850/Kconfig      |    2 ++
 arch/x86_64/Kconfig    |    2 ++
 arch/xtensa/Kconfig    |    2 ++
 async_tx/Kconfig       |   27 +++++++++++++++++++++++++++
 drivers/md/Kconfig     |    3 +++
 28 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 770f717..1fb0075 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -650,3 +650,4 @@ source "crypto/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1f2809c..dd49b2a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1048,3 +1048,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/arm26/Kconfig b/arch/arm26/Kconfig
index 20688bc..bc8b99d 100644
--- a/arch/arm26/Kconfig
+++ b/arch/arm26/Kconfig
@@ -248,3 +248,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/avr32/Kconfig b/arch/avr32/Kconfig
index 3ec7658..5f16cc6 100644
--- a/arch/avr32/Kconfig
+++ b/arch/avr32/Kconfig
@@ -213,3 +213,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index 1a49305..648043a 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -987,3 +987,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/cris/Kconfig b/arch/cris/Kconfig
index 4b41248..f902242 100644
--- a/arch/cris/Kconfig
+++ b/arch/cris/Kconfig
@@ -205,3 +205,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index 114738a..6aa888b 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -390,3 +390,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 618dbad..95e33df 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -227,3 +227,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index c2d54b8..2776164 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -1236,6 +1236,8 @@ source "crypto/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 #
 # Use the generic interrupt handling code in kernel/irq/:
 #
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index de1bff6..929e59b 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -552,6 +552,8 @@ source "fs/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 #
 # Use the generic interrupt handling code in kernel/irq/:
 #
diff --git a/arch/m32r/Kconfig b/arch/m32r/Kconfig
index c3bb8a7..35c6c08 100644
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -412,3 +412,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index b8536c7..1421427 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -677,3 +677,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/m68knommu/Kconfig b/arch/m68knommu/Kconfig
index 823f737..9cf83cf 100644
--- a/arch/m68knommu/Kconfig
+++ b/arch/m68knommu/Kconfig
@@ -683,3 +683,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 0f09412..581040c 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1963,3 +1963,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 3d73545..afc4bad 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -276,3 +276,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 56d3c0d..65f6c55 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -887,6 +887,8 @@ source "arch/powerpc/sysdev/qe_lib/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 menu "Instrumentation Support"
 	depends on EXPERIMENTAL
 
diff --git a/arch/ppc/Kconfig b/arch/ppc/Kconfig
index ccce2a4..e249d9a 100644
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -1458,3 +1458,5 @@ source "arch/ppc/Kconfig.debug"
 source "security/Kconfig"
 
 source "crypto/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 098c62c..25d86af 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -556,3 +556,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 038179e..6f99b0e 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -728,3 +728,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/sh64/Kconfig b/arch/sh64/Kconfig
index ff65420..32f032a 100644
--- a/arch/sh64/Kconfig
+++ b/arch/sh64/Kconfig
@@ -292,6 +292,8 @@ source "crypto/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 #
 # Use the generic interrupt handling code in kernel/irq/:
 #
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index bd992c0..a782692 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -315,3 +315,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/sparc64/Kconfig b/arch/sparc64/Kconfig
index 831781c..16a9175 100644
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -447,3 +447,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index c504312..220d7de 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -325,6 +325,8 @@ source "drivers/scsi/Kconfig"
 
 source "drivers/md/Kconfig"
 
+source "async_tx/Kconfig"
+
 if BROKEN
 	source "drivers/mtd/Kconfig"
 endif
diff --git a/arch/v850/Kconfig b/arch/v850/Kconfig
index 5f54c12..bdee041 100644
--- a/arch/v850/Kconfig
+++ b/arch/v850/Kconfig
@@ -347,4 +347,6 @@ source "crypto/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 
########################################################################
#####
diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index 145bb82..2b8b637 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -784,3 +784,5 @@ source "security/Kconfig"
 source "crypto/Kconfig"
 
 source "lib/Kconfig"
+
+source "async_tx/Kconfig"
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 7fbb44b..b18d15f 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -259,4 +259,6 @@ source "crypto/Kconfig"
 
 source "lib/Kconfig"
 
+source "async_tx/Kconfig"
+
 
diff --git a/async_tx/Kconfig b/async_tx/Kconfig
new file mode 100644
index 0000000..4a6801e
--- /dev/null
+++ b/async_tx/Kconfig
@@ -0,0 +1,27 @@
+menuconfig ASYNC_CORE
+	tristate "Asynchronous Bulk Memory Transfers/Transforms"
+	default n
+	---help---
+	  This enables the async_tx interface layer for dma (offload)
engines.
+	  Subsystems coded to this api will use offload engines for bulk
memory
+	  operations (e.g. memcpy, memset, xor...).  When an offload
engine is not
+	  available the interface will implicitly fall back to a
software
+	  implementation of the operation.
+
+	  If unsure, say N
+
+if ASYNC_CORE
+
+config ASYNC_MEMCPY
+	default m
+	tristate "async_memcpy support"
+
+config ASYNC_XOR
+	default m
+	tristate "async_xor support"
+
+config ASYNC_MEMSET
+	default m
+	tristate "async_memset support"
+
+endif
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 7df934d..9fa1555 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -109,6 +109,9 @@ config MD_RAID10
 config MD_RAID456
 	tristate "RAID-4/RAID-5/RAID-6 mode"
 	depends on BLK_DEV_MD
+	select ASYNC_CORE
+	select ASYNC_MEMCPY
+	select ASYNC_XOR
 	---help---
 	  A RAID-5 set of N drives with a capacity of C MB per drive
provides
 	  the capacity of C * (N - 1) MB, and protects against a failure

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Cornelia Huck]

On Wed, 16 May 2007 23:36:56 -0700,
"Williams, Dan J" <dan.j.williams@intel.com> wrote:

> async_tx: add the Kconfig infrastructure for async_tx
> 
> From: Dan Williams <dan.j.williams@intel.com>
> 
> async_tx is similar to crypto in that there is an api component and a
> drivers component.
> 
> * Add 'source "async_tx/Kconfig"' to the per architecture Kconfig files,
>   for each architecture that sources drivers/md/Kconfig (which appears
> to be
>   all of them).
> * Add 'select' statements for the subsystems that use async_tx
> (md-raid4,5)

Finer granularity is certainly better here, but I'm not quite sure if
this solves our s390 problem (we don't have dma support). All those
backends should also have a non-dma version...

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> Finer granularity is certainly better here, but I'm not quite sure if
> this solves our s390 problem (we don't have dma support). All those
> backends should also have a non-dma version...

In fact that is already there.  Here is the form of async_memcpy for
example:
... async_memcpy( ... )
{
	struct dma_chan *chan = async_tx_find_channel(depend_tx,
DMA_MEMCPY);
	struct dma_device *device = chan ? chan->device : NULL;
	int int_en = callback ? 1 : 0;
	struct dma_async_tx_descriptor *tx = device ?
		device->device_prep_dma_memcpy(chan, len,
		int_en) : NULL;

	if (tx) { /* run the memcpy asynchronously */

		...

	} else { /* run the memcpy synchronously */

		...
	}
}

When CONFIG_DMA_ENGINE=n async_tx_find_channel takes the form:
... async_tx_find_channel( ... )
{
	return NULL;
}

So in the S390 case the entire asynchronous path will be compiled away.

--
Dan

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Cornelia Huck]

On Fri, 18 May 2007 09:30:09 -0700,
"Williams, Dan J" <dan.j.williams@intel.com> wrote:

> When CONFIG_DMA_ENGINE=n async_tx_find_channel takes the form:
> ... async_tx_find_channel( ... )
> {
> 	return NULL;
> }
> 
> So in the S390 case the entire asynchronous path will be compiled away.

Unfortunately, do_async_xor() (and others) is not ifdef'ed and contains
dma_map_page(), which led to the compile failure...

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> On Fri, 18 May 2007 09:30:09 -0700,
> "Williams, Dan J" <dan.j.williams@intel.com> wrote:
> 
> > When CONFIG_DMA_ENGINE=n async_tx_find_channel takes the form:
> > ... async_tx_find_channel( ... )
> > {
> > 	return NULL;
> > }
> >
> > So in the S390 case the entire asynchronous path will be compiled
away.
> 
> Unfortunately, do_async_xor() (and others) is not ifdef'ed and
contains
> dma_map_page(), which led to the compile failure...

Sorry, I did not realize dma_map_page did not exist on s390.  I am
building an s390 cross compiler so I can clean up these errors.

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> On Fri, 18 May 2007 09:30:09 -0700,
> "Williams, Dan J" <dan.j.williams@intel.com> wrote:
> 
> > When CONFIG_DMA_ENGINE=n async_tx_find_channel takes the form:
> > ... async_tx_find_channel( ... )
> > {
> > 	return NULL;
> > }
> >
> > So in the S390 case the entire asynchronous path will be compiled
away.
> 
> Unfortunately, do_async_xor() (and others) is not ifdef'ed and
contains
> dma_map_page(), which led to the compile failure...

The approach I have taken is to add the missing definitions to
include/asm-s390/dma-mapping.h [ a non-outlook-mangled version of the
patch is pushed out in my rebased git tree ].  I was not able to fully
compile-test this change as the three s390-cross-toolchains I tried each
died early in the kernel build process.  The most common error was:
"s390-unknown-linux-gnu-ld: unrecognised emulation mode: elf64_s390"

---

s390: add dma mapping api stub definitions for async_tx

From: Dan Williams <dan.j.williams@intel.com>

The asynchronous path in async_tx is meant to be compiled away on
platforms
like s390 with CONFIG_DMA_ENGINE=n.  However, it is difficult to compile
something away if it does not compile in the first place.  This patch
adds
the missing dma api definitions as BUG() stubs.

Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---

 include/asm-s390/dma-mapping.h |   78
++++++++++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/include/asm-s390/dma-mapping.h
b/include/asm-s390/dma-mapping.h
index 3f8c12f..33a3c82 100644
--- a/include/asm-s390/dma-mapping.h
+++ b/include/asm-s390/dma-mapping.h
@@ -4,9 +4,87 @@
  *  S390 version
  *
  *  This file exists so that #include <dma-mapping.h> doesn't break
anything.
+ *  It also includes stub definitions of the API so common code like
async_tx
+ *  can compile.
  */
 
 #ifndef _ASM_DMA_MAPPING_H
 #define _ASM_DMA_MAPPING_H
 
+#include <linux/mm.h>
+#include <asm/scatterlist.h>
+
+static inline dma_addr_t
+dma_map_single(struct device *dev, void *cpu_addr, size_t size,
+	enum dma_data_direction dir)
+{
+	BUG();
+	return 0;
+}
+
+static inline dma_addr_t
+dma_map_page(struct device *dev, struct page *page,
+	     unsigned long offset, size_t size,
+	     enum dma_data_direction dir)
+{
+	BUG();
+	return 0;
+}
+
+static inline void
+dma_unmap_single(struct device *dev, dma_addr_t handle, size_t size,
+	enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline void
+dma_unmap_page(struct device *dev, dma_addr_t handle, size_t size,
+	       enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline int
+dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+	enum dma_data_direction dir)
+{
+	BUG();
+	return 0;
+}
+
+static inline void
+dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
+	     enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_single_for_cpu(struct device *dev, dma_addr_t handle, size_t
size,
+			enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_single_for_device(struct device *dev, dma_addr_t handle,
size_t size,
+			   enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg, int
nents,
+		    enum dma_data_direction dir)
+{
+	BUG();
+}
+
+static inline void
+dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, int
nents,
+		       enum dma_data_direction dir)
+{
+	BUG();
+}
 #endif /* _ASM_DMA_MAPPING_H */

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Martin Schwidefsky]

On Tue, 2007-05-22 at 17:25 -0700, Williams, Dan J wrote:
> The approach I have taken is to add the missing definitions to
> include/asm-s390/dma-mapping.h [ a non-outlook-mangled version of the
> patch is pushed out in my rebased git tree ].  I was not able to fully
> compile-test this change as the three s390-cross-toolchains I tried
> each

We are trying to get rid of dma-mapping.h, see the last change to the
file with commit 411f0f3edc141a582190d3605cadd1d993abb6df. I don't think
we should reintroduce dma related definition but split the async_tx in a
way that allows to compile it on an architecture with CONFIG_NO_DMA=y
(yes I know that is harder that to just add the dma stubs).
You've said that there is a software implementation if there is no dma
engine present. This software implementation should be independent of
dma-mapping.h. Without having looked at the code, isn't it possible to
isolate that software implementation into its own C file? That would be
the only one that gets compiled for s390.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Cornelia Huck]

On Wed, 23 May 2007 10:05:39 +0200,
Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> We are trying to get rid of dma-mapping.h, see the last change to the
> file with commit 411f0f3edc141a582190d3605cadd1d993abb6df. I don't think
> we should reintroduce dma related definition but split the async_tx in a
> way that allows to compile it on an architecture with CONFIG_NO_DMA=y
> (yes I know that is harder that to just add the dma stubs).
> You've said that there is a software implementation if there is no dma
> engine present. This software implementation should be independent of
> dma-mapping.h. Without having looked at the code, isn't it possible to
> isolate that software implementation into its own C file? That would be
> the only one that gets compiled for s390.

Taking a quick look at the async_*.c stuff, the functions in question
basically seem to be of the form

check_if_we_can_do_it_async();
if (async_ok) {
	/* do async stuff */
	/* that's where the dma mapping creeps in */
} else {
	/* do it sync */
	/* seems fine for us */
}

So you should be able to factor out (say) async_memset_{sync,async}()
and put it into async_memset_{sync,async}.c. async_memset() would then
be

async_memset()
{
#if CONFIG_HAS_DMA
	if (check_if_we_can_do_at_async())
		async_memset_async();
#endif
	return async_memset_sync();
}

Kconfig could then do

config ASYNC_MEMSET
	default m
	tristate "async_memset support"
	select ASYNC_MEMSET_ASYNC if HAS_DMA

config ASYNC_MEMSET_ASYNC
	depends on HAS_DMA
	tristate "async_memset async via dma support"

Thoughts?

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Martin Schwidefsky]

On Wed, 2007-05-23 at 10:46 +0200, Cornelia Huck wrote:
> Taking a quick look at the async_*.c stuff, the functions in question
> basically seem to be of the form
> 
> check_if_we_can_do_it_async();
> if (async_ok) {
>         /* do async stuff */
>         /* that's where the dma mapping creeps in */
> } else {
>         /* do it sync */
>         /* seems fine for us */
> }

Hmm, on what does the async_ok depend? Is that a runtime check that is
done once or is it something more complicated like the availability of a
dma slot? If it is a simple runtime check then there should be a
operations structure that has indirect function pointers for the
different async_memset_{sync,async}() functions. Instead of doing the
async_ok check just call the function. That would save an if as well.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.

RE: 2.6.22-rc1-mm1 - s390 vs. md

[Williams, Dan J]

> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> On Wed, 23 May 2007 10:05:39 +0200,
> Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > We are trying to get rid of dma-mapping.h, see the last change to
the
> > file with commit 411f0f3edc141a582190d3605cadd1d993abb6df. I don't
think
> > we should reintroduce dma related definition but split the async_tx
in a
> > way that allows to compile it on an architecture with
CONFIG_NO_DMA=y
> > (yes I know that is harder that to just add the dma stubs).

Not harder, it just a question of which is "uglier", but given the
direction taken with CONFIG_HAS_DMA it now appears more appropriate to
change async_tx.

> > You've said that there is a software implementation if there is no
dma
> > engine present. This software implementation should be independent
of
> > dma-mapping.h. Without having looked at the code, isn't it possible
to
> > isolate that software implementation into its own C file? That would
be
> > the only one that gets compiled for s390.
> 
> Taking a quick look at the async_*.c stuff, the functions in question
> basically seem to be of the form
> 
> check_if_we_can_do_it_async();
> if (async_ok) {
> 	/* do async stuff */
> 	/* that's where the dma mapping creeps in */
> } else {
> 	/* do it sync */
> 	/* seems fine for us */
> }
> 
> So you should be able to factor out (say) async_memset_{sync,async}()
> and put it into async_memset_{sync,async}.c. async_memset() would then
> be
> 
> async_memset()
> {
> #if CONFIG_HAS_DMA
> 	if (check_if_we_can_do_at_async())
> 		async_memset_async();
> #endif
> 	return async_memset_sync();
> }
> 
> Kconfig could then do
> 
> config ASYNC_MEMSET
> 	default m
> 	tristate "async_memset support"
> 	select ASYNC_MEMSET_ASYNC if HAS_DMA
> 
> config ASYNC_MEMSET_ASYNC
> 	depends on HAS_DMA
> 	tristate "async_memset async via dma support"
> 
> Thoughts?

I took your approach of encasing the HAS_DMA dependent portion of the
code in #ifdef CONFIG_HAS_DMA, and I dropped the dma-mapping-stub patch.
I let the compiler do the factoring out for me by making
async_tx_find_channel become the following when CONFIG_DMA_ENGINE=n:

static inline struct dma_chan *
async_tx_find_channel(struct dma_async_tx_descriptor *depend_tx,
	enum dma_transaction_type tx_type)
{
	return NULL;
}

---

diff --git a/async_tx/async_memcpy.c b/async_tx/async_memcpy.c
index 7896ba8..547976e 100644
--- a/async_tx/async_memcpy.c
+++ b/async_tx/async_memcpy.c
@@ -56,6 +56,7 @@ async_memcpy(struct page *dest, struct page *src,
unsigned int dest_offset,
 		int_en) : NULL;
 
 	if (tx) { /* run the memcpy asynchronously */
+		#ifdef CONFIG_HAS_DMA
 		dma_addr_t dma_addr;
 		enum dma_data_direction dir;
 
@@ -75,6 +76,9 @@ async_memcpy(struct page *dest, struct page *src,
unsigned int dest_offset,
 
 		async_tx_submit(chan, tx, flags, depend_tx, callback,
 			callback_param);
+		#else
+		BUG();
+		#endif /* CONFIG_HAS_DMA */
 	} else { /* run the memcpy synchronously */
 		void *dest_buf, *src_buf;
 		pr_debug("%s: (sync) len: %zu\n", __FUNCTION__, len);
diff --git a/async_tx/async_memset.c b/async_tx/async_memset.c
index 736c7c2..9166a27 100644
--- a/async_tx/async_memset.c
+++ b/async_tx/async_memset.c
@@ -55,6 +55,7 @@ async_memset(struct page *dest, int val, unsigned int
offset,
 			int_en) : NULL;
 
 	if (tx) { /* run the memset asynchronously */
+		#ifdef CONFIG_HAS_DMA
 		dma_addr_t dma_addr;
 		enum dma_data_direction dir;
 
@@ -67,6 +68,9 @@ async_memset(struct page *dest, int val, unsigned int
offset,
 
 		async_tx_submit(chan, tx, flags, depend_tx, callback,
 			callback_param);
+		#else
+		BUG();
+		#endif /* CONFIG_HAS_DMA */
 	} else { /* run the memset synchronously */
 		void *dest_buf;
 		pr_debug("%s: (sync) len: %zu\n", __FUNCTION__, len);
diff --git a/async_tx/async_xor.c b/async_tx/async_xor.c
index 37ae5fc..5e4bc29 100644
--- a/async_tx/async_xor.c
+++ b/async_tx/async_xor.c
@@ -31,6 +31,7 @@
 #include <linux/raid/xor.h>
 #include <linux/async_tx.h>
 
+#ifdef CONFIG_HAS_DMA
 static void
 do_async_xor(struct dma_async_tx_descriptor *tx, struct dma_device
*device,
 	struct dma_chan *chan, struct page *dest, struct page
**src_list,
@@ -62,6 +63,17 @@ do_async_xor(struct dma_async_tx_descriptor *tx,
struct dma_device *device,
 	async_tx_submit(chan, tx, flags, depend_tx, callback,
 		callback_param);
 }
+#else
+static void
+do_async_xor(struct dma_async_tx_descriptor *tx, struct dma_device
*device,
+	struct dma_chan *chan, struct page *dest, struct page
**src_list,
+	unsigned int offset, unsigned int src_cnt, size_t len,
+	enum async_tx_flags flags, struct dma_async_tx_descriptor
*depend_tx,
+	dma_async_tx_callback callback, void *callback_param)
+{
+	BUG();
+}
+#endif /* CONFIG_HAS_DMA */
 
 static void
 do_sync_xor(struct page *dest, struct page **src_list, unsigned int
offset,
@@ -265,6 +277,7 @@ async_xor_zero_sum(struct page *dest, struct page
**src_list,
 	BUG_ON(src_cnt <= 1);
 
 	if (tx) {
+		#ifdef CONFIG_HAS_DMA
 		dma_addr_t dma_addr;
 		enum dma_data_direction dir;
 
@@ -281,6 +294,9 @@ async_xor_zero_sum(struct page *dest, struct page
**src_list,
 
 		async_tx_submit(chan, tx, flags, depend_tx, callback,
 			callback_param);
+		#else
+		BUG();
+		#endif /* CONFIG_HAS_DMA */
 	} else {
 		unsigned long xor_flags = flags;
 

Re: 2.6.22-rc1-mm1 - s390 vs. md

[Cornelia Huck]

On Thu, 24 May 2007 15:11:08 -0700,
"Williams, Dan J" <dan.j.williams@intel.com> wrote:


> --- a/async_tx/async_memcpy.c
> +++ b/async_tx/async_memcpy.c
> @@ -56,6 +56,7 @@ async_memcpy(struct page *dest, struct page *src,
> unsigned int dest_offset,
>  		int_en) : NULL;
>  
>  	if (tx) { /* run the memcpy asynchronously */
> +		#ifdef CONFIG_HAS_DMA
>  		dma_addr_t dma_addr;
>  		enum dma_data_direction dir;

Can you factor out the async stuff into a function so you can use the
#ifdefs to define different functions rather than put them in the middle
of a complex function?

(Maybe you should rather use #ifdef CONFIG_DMA_ENGINE, since the async
part is not needed for !DMA_ENGINE either.)