Re: PCI DMA to small buffers on cache-incoherent arch

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

> From: Roland Dreier <roland@topspin.com>
> Date: 10 Jun 2002 21:39:27 -0700
> 
>        David> How about allocating struct something using pci_pool?
>    
>    The problem is the driver can't safely touch field1 or field2 near the
>    DMA (it might pull the cache line back in too soon, or dirty the cache
>    line and have it written back on top of DMA'ed data)
> 
> Oh duh, I see, then go to making the thing a pointer to the
> dma area.

If it's going to be a pointer, it might as well be normal kmalloc data,
avoiding obfuscation of the memory model used by USB device drivers.
(They don't have to worry about DMA addresses, which IMO is a feature.)

Seems like there are two basic options for how to handle all this.

(a) Expose the dma/cache interaction to device drivers, so
     they can manage (and prevent) bad ones like the cacheline
     sharing scenarios.

(b) Require drivers to provide I/O buffers that are slab/page/...
     pointers from some API that prevents such problems.

I think (a) is more or less a variant of the existing alignment
requirements that get embedded in ABIs, but which still show up
in code that's very close to the hardware.  Many device driver
folk are familiar with them, or performance tuners, but not so
many folk doing USB device drivers would be.  (They are thankfully
far from host controller hardware and its quirks!)

Personally I'd avoid (b) since I like to allocate memory all at
once in most cases ... lots fewer error cases to cope with (or,
more typically _not_ cope with :).

One question is whether to support only one of them, or allow both.
In either case the DMA-mapping.txt will need to touch on the issue.

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Mon, 10 Jun 2002 22:31:45 -0700

   One question is whether to support only one of them, or allow both.
   In either case the DMA-mapping.txt will need to touch on the issue.

Another important issue is from where do these issues originate?

This stuff rarely happens most of the time because block buffers and
networking buffers are what are fed to the chip.

I still think it is crucial that we not put this alignment garbage
into the drivers themselves.  Driver writers are going to get it
wrong or be confused by it.

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

>    One question is whether to support only one of them, or allow both.
>    In either case the DMA-mapping.txt will need to touch on the issue.
> 
> Another important issue is from where do these issues originate?
> 
> This stuff rarely happens most of the time because block buffers and
> networking buffers are what are fed to the chip.

I think the examples Oliver found related mostly to device control
and status tracking.


> I still think it is crucial that we not put this alignment garbage
> into the drivers themselves.  Driver writers are going to get it
> wrong or be confused by it.

I guess I don't see a way around exposing some of it.  Do you?

Even the (b) option requires drivers to know they can't pass pointers
to the inside of a structure, AND that memory right after the end of
an I/O buffer is unavailable.  That exposes the issue.  Although it
doesn't provide help to manage it, as (a) does, or detect it.

Should the dma mapping APIs try to detect the "DMA buffer starts in
middle of non-coherent cacheline" case, and fail?  That might be
worth checking, catching some of these errors, even if it ignores
the corresponding "ends in middle of non-coherent cacheline" case.
And it'd handle that "it's a runtime issue on some HW" concern.

Or then there's David Woodhouse's option (disable caching on those
pages while the DMA mapping is active) which seems good, except for
the fact that this issue is most common for buffers that are a lot
smaller than one page ... so lots of otherwise cacheable data would
suddenly get very slow. :)

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[Oliver Neukum]

Am Dienstag, 11. Juni 2002 17:12 schrieb David Brownell:
> >    One question is whether to support only one of them, or allow both.
> >    In either case the DMA-mapping.txt will need to touch on the issue.
> >
> > Another important issue is from where do these issues originate?
> >
> > This stuff rarely happens most of the time because block buffers and
> > networking buffers are what are fed to the chip.
>
> I think the examples Oliver found related mostly to device control
> and status tracking.

I did not look for others. It would seem that SCSI and networking are
affected as well. SCSI due to the sense buffer.

> Or then there's David Woodhouse's option (disable caching on those
> pages while the DMA mapping is active) which seems good, except for
> the fact that this issue is most common for buffers that are a lot
> smaller than one page ... so lots of otherwise cacheable data would
> suddenly get very slow. :)

There might be several buffers in one page. You'd have to count
DMA users in struct page.
Secondly are you sure that a physical page won't be accessed
by several different virtual addresses ? On some architectures
that would kill us.

	Regards
		Oliver

Re: PCI DMA to small buffers on cache-incoherent arch

[Benjamin Herrenschmidt]

>Remember please that specifically the DMA mapping APIs encourage use
>of consistent memory for small data objects.  It is specifically
>because non-consistent DMA accesses to small bits are going to be very
>slow (ie. the PCI controller is going to prefetch further cache lines
>for no reason, for example).  The non-consistent end of the APIs is
>meant for long contiguous buffers, not small chunks.
>
>This is one of the reasons I want to fix this by making people use
>either consistent memory or PCI pools (which is consistent memory
>too).

Please don't limit the API design to PCI ;)

There are more and more embedded CPUs out there with their own
bunch of on-chip devices that are neither consistent nor PCI based,
their drivers will have the exact same problem to deal with though.

Ben.

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Tue, 11 Jun 2002 08:12:35 -0700
   
   Should the dma mapping APIs try to detect the "DMA buffer starts in
   middle of non-coherent cacheline" case, and fail?  That might be
   worth checking, catching some of these errors, even if it ignores
   the corresponding "ends in middle of non-coherent cacheline" case.
   And it'd handle that "it's a runtime issue on some HW" concern.
   
This brings back another issue, returning failure from pci_map_*()
and friends which currently cannot happen.

   Or then there's David Woodhouse's option (disable caching on those
   pages while the DMA mapping is active) which seems good, except for
   the fact that this issue is most common for buffers that are a lot
   smaller than one page ... so lots of otherwise cacheable data would
   suddenly get very slow. :)
   
Remember please that specifically the DMA mapping APIs encourage use
of consistent memory for small data objects.  It is specifically
because non-consistent DMA accesses to small bits are going to be very
slow (ie. the PCI controller is going to prefetch further cache lines
for no reason, for example).  The non-consistent end of the APIs is
meant for long contiguous buffers, not small chunks.

This is one of the reasons I want to fix this by making people use
either consistent memory or PCI pools (which is consistent memory
too).

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Tue, 11 Jun 2002 23:25:09 -0700

   I'd suspect ((dma_addr_t)0) would be a reasonable error return.
   At least some hardware treats such values like software would
   treat null pointers.  No call syntax change necessary, which
   might be good or bad depending on how you feel tomorrow.
   
0 is a valid PCI dma address on many platforms.  This is part
of the problem.

   > Remember please that specifically the DMA mapping APIs encourage use
   > of consistent memory for small data objects.  ...
   > ...  The non-consistent end of the APIs is
   > meant for long contiguous buffers, not small chunks.
   
   And in between, a nice huge grey area to play with and argue about!
   
Not gray area, fully intentional!  From the beginning that was meant
to be one of the distinctions between consistent and streaming DMA
memory.

   For that model, I would prefer tools more like a kmalloc than the
   pci_pool, which is most like a kmem_cache_t.  The particular objects
   in question are a bit small to use page-or-bigger allocators, too.
   
Huh?  The whole idea is that it is memory for PCI dma, it has to be
PCI in nature.  If you want a kmalloc'ish thing, simple use
pci_alloc_consistent and carve up the pages you get internally.

   The problem for APIs like USB is that they haven't yet exposed DMA
   addresses.  Doing that, giving folk a choice from the "non-consistent
   end of the APIs", would be a big change.
   
But that is the direction I'd like things to go in.  A lot of problems
have arisen because the USB layer likes to internally let drivers do
DMA on any gob of memory whatsoever.  That has to stop, it really does.
   
   Oh no -- I just had an evil thought.  Now that we have the device model
   code partially in place, shouldn't we have DMA memory calls talk in
   terms of "struct device" not "struct pci_device"?  That'd be the way
   to have _one_ API for dma mapping (and consistent memory allocation),
   working for PCI, USB, and any other bus framework that comes along.
   
Sure, that's the idea.  Just change pci_alloc_consistent to
dev_alloc_consistent whatever.  It's all still the same problem
though.  The USB drivers have to stop DMA'ing to arbitrary little gobs
of memory.

   

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

David S. Miller wrote:
>    From: David Brownell <david-b@pacbell.net>
>    
>    Should the dma mapping APIs try to detect the "DMA buffer starts in
>    middle of non-coherent cacheline" case, and fail?
>    
> This brings back another issue, returning failure from pci_map_*()
> and friends which currently cannot happen.

An issue that'll get fixed, yes? :)

I'd suspect ((dma_addr_t)0) would be a reasonable error return.
At least some hardware treats such values like software would
treat null pointers.  No call syntax change necessary, which
might be good or bad depending on how you feel tomorrow.


> Remember please that specifically the DMA mapping APIs encourage use
> of consistent memory for small data objects.  ...
> ...  The non-consistent end of the APIs is
> meant for long contiguous buffers, not small chunks.

And in between, a nice huge grey area to play with and argue about!


> This is one of the reasons I want to fix this by making people use
> either consistent memory or PCI pools (which is consistent memory
> too).

For that model, I would prefer tools more like a kmalloc than the
pci_pool, which is most like a kmem_cache_t.  The particular objects
in question are a bit small to use page-or-bigger allocators, too.

The problem for APIs like USB is that they haven't yet exposed DMA
addresses.  Doing that, giving folk a choice from the "non-consistent
end of the APIs", would be a big change.


Oh no -- I just had an evil thought.  Now that we have the device model
code partially in place, shouldn't we have DMA memory calls talk in
terms of "struct device" not "struct pci_device"?  That'd be the way
to have _one_ API for dma mapping (and consistent memory allocation),
working for PCI, USB, and any other bus framework that comes along.

Nah ... forget I said that.  Too logical, it could never happen!  ;)

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

David S. Miller wrote:
>    From: David Brownell <david-b@pacbell.net>
>    Date: Tue, 11 Jun 2002 23:25:09 -0700
> 
>    I'd suspect ((dma_addr_t)0) would be a reasonable error return.
>    At least some hardware treats such values like software would
>    treat null pointers.  No call syntax change necessary, which
>    might be good or bad depending on how you feel tomorrow.
>    
> 0 is a valid PCI dma address on many platforms.  This is part
> of the problem.

OK, so a new call syntax would be desirable.  Or reserving page zero
even on such platforms.


>    > Remember please that specifically the DMA mapping APIs encourage use
>    > of consistent memory for small data objects.  ...
>    > ...  The non-consistent end of the APIs is
>    > meant for long contiguous buffers, not small chunks.
>    
>    And in between, a nice huge grey area to play with and argue about!
>    
> Not gray area, fully intentional!  From the beginning that was meant
> to be one of the distinctions between consistent and streaming DMA
> memory.

I realize that, but it's still a grey area ... meaning only that the
specific choice, in a set of systems, is one of the design tradeoffs.
Costs can vary a lot, but near either end the tradeoffs get clearer.


>    For that model, I would prefer tools more like a kmalloc than the
>    pci_pool, which is most like a kmem_cache_t.  The particular objects
>    in question are a bit small to use page-or-bigger allocators, too.
>    
> Huh?  The whole idea is that it is memory for PCI dma, it has to be
> PCI in nature.  If you want a kmalloc'ish thing, simple use
> pci_alloc_consistent and carve up the pages you get internally.

And the reason that logic doesn't lead us to rip kmalloc() out of
the kernel (in favor of the page allocator) is ... what?  Every driver
shouldn't _need_ to write yet another malloc() clone, that's all I'm
saying; that'd be a waste.


>    The problem for APIs like USB is that they haven't yet exposed DMA
>    addresses.  Doing that, giving folk a choice from the "non-consistent
>    end of the APIs", would be a big change.
>    
> But that is the direction I'd like things to go in.  A lot of problems
> have arisen because the USB layer likes to internally let drivers do
> DMA on any gob of memory whatsoever.  That has to stop, it really does.

I'll accept that for argument -- though I'll ask what other problems
(than this shared-cacheline scenario) you're thinking of.  This is the
only one that I recall seeming like it might argue for an API change.

(And USB is just one example, too.  Oliver turned up similar cases
in the SCSI layer, as I recall.  Likely a lot of it can be traced
down to widely-available hardware that wouldn't recognize a NUMA if
it tripped on one.)


>    Oh no -- I just had an evil thought.  Now that we have the device model
>    code partially in place, shouldn't we have DMA memory calls talk in
>    terms of "struct device" not "struct pci_device"?  That'd be the way
>    to have _one_ API for dma mapping (and consistent memory allocation),
>    working for PCI, USB, and any other bus framework that comes along.
>    
> Sure, that's the idea.  Just change pci_alloc_consistent to
> dev_alloc_consistent whatever. 

Change, and fix up that lack of error codes on the mapping calls, and
maybe a few other things ... :)

Certainly hanging some sort of memory allocator object off struct device
should be pretty cheap.  At least for USB, all devices on the same bus
could share the same allocator.  Not that I know PCI that well, but I
suspect that could be true there too ... more sharing, less internal
fragmentation, and more efficient use of various resources.

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Wed, 12 Jun 2002 00:06:39 -0700

   David S. Miller wrote:
   > 0 is a valid PCI dma address on many platforms.  This is part
   > of the problem.
   
   OK, so a new call syntax would be desirable.  Or reserving page zero
   even on such platforms.
   
Or, better yet, define a PCI_BAD_ADDR per-arch.
   
   > Huh?  The whole idea is that it is memory for PCI dma, it has to be
   > PCI in nature.  If you want a kmalloc'ish thing, simple use
   > pci_alloc_consistent and carve up the pages you get internally.
   
   And the reason that logic doesn't lead us to rip kmalloc() out of
   the kernel (in favor of the page allocator) is ... what?  Every driver
   shouldn't _need_ to write yet another malloc() clone, that's all I'm
   saying; that'd be a waste.
   
That is why we have the PCI pool thing, it's PCI alloc consistent +
SLAB carving.
   
   Certainly hanging some sort of memory allocator object off struct device
   should be pretty cheap.  At least for USB, all devices on the same bus
   could share the same allocator.  Not that I know PCI that well, but I
   suspect that could be true there too ... more sharing, less internal
   fragmentation, and more efficient use of various resources.
   
These instances we are talking about want to allocate multiple
instance of the same object (thus the same size).  They are not
allocating things like variable length strings and whatnot.

That is why I keep suggesting PCI pools, it's the perfect kind of
setup (and the easiest to implement).  A generic kmalloc() out of
pages is more to implement, certainly.

If you want to hide this behind a usb_dma_pool_create,
usb_dma_pool_alloc etc. kind of interface that calls back into the
host controller layer to get the pci_dev etc., that's fine.

Now enough talking and someone implement this, talk is cheap.

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
   Date: Tue, 11 Jun 2002 19:33:47 +0200

   >This is one of the reasons I want to fix this by making people use
   >either consistent memory or PCI pools (which is consistent memory
   >too).
   
   Please don't limit the API design to PCI ;)
   
When I say "PCI pools" think "struct device pools" because that
will what it will be in the end.

That is Linus's long range plan, everything you see as pci_*
will become dev_*.

So I'm not really limiting the design in any way.

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

>    Please don't limit the API design to PCI ;)
>    
> When I say "PCI pools" think "struct device pools" because that
> will what it will be in the end.
> 
> That is Linus's long range plan, everything you see as pci_*
> will become dev_*.

I think the guts of it could happen in the 2.5 series soonish
though, so it'd only be "long range" in that quaint Wall Street
sense of "finishes sometime after next quarter".  :)

Just to put some flesh on the discussion ... here's the rough
shape of what I was thinking, as more wood for the fire:

- Associated with a "struct device" would be some kind of
   memory allocator/mapper object that would expose every (?)
   primitive in DMA-mapping.txt, but not PCI-specific.  That
   would basically be a vtable, using the device for state.

	struct dev_dma_whatsit {
	    /* page-level "consistent" allocators
	     * should they continue to be nonblocking?
	     */
	    void *alloc_consistent(struct device *dev,
		size_t *size, dma_addr_t *dma);
	    void free_consistent(struct device *dev,
		size_t size, void *addr, dma_addr_t dma);

	    /* "streaming mapping" calls
	     * return BAD_DMA_ADDR on error
	     */
	    dma_addr_t map_single(struct device *dev,
		void *addr, size_t size, int dir);
	    void unmap_single(struct device *dev,
		dma_addr_t, size_t size, int dir);
	    void sync_single(struct device *dev,
		dma_addr_t, size_t size, int dir);
		
	    /* Also: dma masks, page and scatterlist
	     * map/unmap/sync, dma64_addr_t, ...
	     */
	};

   Those would be designed so they could be shared between
   devices, invisibly to code talking to a struct device.

   Example:  all USB devices connected to a given bus would
   normally delegate to the PCI device underlying that bus.
   (Except for non-PCI host controllers, of course!)

   And the PCI versions of those methods have rather obvious
   implementations ... :)

- There'd be dev_*() routines that in many cases just call
   directly to those methods, or failed cleanly if the whatsit
   wasn't set up ("used the default allocator"), or omitted
   key methods.  Many could be inlinable functions:

	dev_alloc_consistent(...)
	dev_free_consistent(...)
	dev_map_single(...)
	... etc

   In some cases they'd mostly be layered over those primitives:

	/* kmalloc analogue */
   	void *dev_kmalloc(struct device *dev,
		size_t size, int mem_flags,
		dma_addr_t *dma);
	void dev_kfree (struct device *dev, void *mem,
		dma_addr_t dma);

	/* pci_pool/kmem_cache_t analogues */
	struct dev_pool *dev_pool_create(const char *name,
		struct device *dev, size_t size,
		size_t allocation, int mem_flags);
	void dev_pool_destroy(struct dev_pool *pool);

	void *dev_pool_alloc(struct dev_pool *pool,
		int mem_flags, dma_addr_t *dma);
	void dev_pool_free(struct dev_pool *pool,
		void *mem, dma_addr_t dma);
		
- Some of the other driver APIs would want to change to
   leverage these primitives.  They might want to provide
   wrappers to make the dev_*() calls given the subsystem
   device API.

   Again using USB for an, one _possible_ change would be
   to make urb->transfer_buffer be a dma_addr_t ... where
   today it's a void*.  (Discussions of such details belong
   on the linux-usb-devel list, for the most part.)   That'd
   be a pretty major change to the USB API, since it'd force
   device drivers to manage DMA mappings.  While usb-storage
   might benefit from sglist support, other drivers might
   not see much of a win.

OK, that's enough trouble for this morning... ;)

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[Benjamin Herrenschmidt]

>   Those would be designed so they could be shared between
>   devices, invisibly to code talking to a struct device.
>
>   Example:  all USB devices connected to a given bus would
>   normally delegate to the PCI device underlying that bus.
>   (Except for non-PCI host controllers, of course!)
>
>   And the PCI versions of those methods have rather obvious
>   implementations ... :)

Yup. The simplest way I see here is to have a device automatically
inherit, by default, his parent device ones. The arch will provide
default "root" allocators, so a fully coherent or fully non-coherent
arch may not have to do anything but these.

Now, if a given bus in the arch need specific quirks, it's up to
the bus device node to put it's own versions to be used by it's
childs.

Ben.

Re: PCI DMA to small buffers on cache-incoherent arch

[Roland Dreier]

>>>>> "David" == David Brownell <david-b@pacbell.net> writes:

    David> Again using USB for an, one _possible_ change would be to
    David> make urb->transfer_buffer be a dma_addr_t ... where today
    David> it's a void*.  (Discussions of such details belong on the
    David> linux-usb-devel list, for the most part.)  That'd be a
    David> pretty major change to the USB API, since it'd force device
    David> drivers to manage DMA mappings.  While usb-storage might
    David> benefit from sglist support, other drivers might not see
    David> much of a win.

Let's go back to the beginning of this discussion since I think we're
losing sight of the original problem.  In general I certainly support
the idea of making the DMA mapping stuff device generic instead of
tied to PCI.  For example, this would really clean up the handling of
on-chip peripherals for some of the PowerPC 4xx CPUs I work with (in
fact on the linuxppc-embedded list there has been discussion about the
abuse of constants like PCI_DMA_FROMDEVICE in contexts that have
nothing to do with PCI).

However, this discussion started when I fixed some problems I was
having with USB on my IBM PowerPC 440GP system (which is not cache
coherent).  I posted a patch to linux-usb-devel that got rid of the
use of unaligned buffers for DMA.  Most of the changes got rid of DMA
into variables on the stack, which everyone agreed were clear bugs.  I
also changed some code that did DMA into the middle of a struct in
kmalloc()'ed memory.  As a concrete example, in drivers/usb/hub.h I
made the change:

 struct usb_hub {
 	struct usb_device *dev;
 
 	struct urb *urb;		/* Interrupt polling pipe */
 
-	char buffer[(USB_MAXCHILDREN + 1 + 7) / 8]; /* add 1 bit for hub status change */
-					/* and add 7 bits to round up to byte boundary */
+        char *buffer;
 	int error;
 	int nerrors;

and then after the kmalloc of struct usb_hub *hub, I added a kmalloc
of hub->buffer.

There were two objections to this change.  First, there were questions
about whether the change was really needed.  I think we have all
agreed that DMA into buffer is not necessarily safe.  Second, people
felt that splitting the allocation of hub in two introduced needless
complication and that it would be better to align buffer within the
structure.  To this end I proposed a __dma_buffer macro; cache
coherent architectures would just do

        #define __dma_buffer

while non-cache-coherent architectures would define

        #define __dma_buffer __dma_buffer_line(__LINE__)
        #define __dma_buffer_line(line) __dma_buffer_expand_line(line)
        #define __dma_buffer_expand_line(line) \
        	__attribute__ ((aligned(L1_CACHE_BYTES))); \
        	char __dma_pad_ ## line [0] __attribute__ ((aligned(L1_CACHE_BYTES)))

Then the USB hub fix would just amount to changing the declaration of
buffer to

	char buffer[(USB_MAXCHILDREN + 1 + 7) / 8] __dma_buffer; /* add 1 bit for hub status change */

and no other code would change.  This seems to me to be pretty close
to the minimal change to make things correct.  You would get zero
bloat on cache coherent architectures, and only one line of code needs
to be touched.  We can debate about whether this puts too much
knowledge about DMA into the driver; I would argue that
device-specific allocators are worse.

The other objection to __dma_buffer seems to be that alignment
requirements can't be calculated at compile time; however this problem
seems to exist for kmalloc() as well (since slab caches have to be
initialized well before every bus is discovered, not to mention the
fact that we might have new alignment requirements coming from
hot-plugged devices).  It seems to me that both kmalloc() and
__dma_buffer both need to align for the worst possible case, and no
one objects to kmalloc().  (Well, you might say that device drivers
should use device specific allocators, but skbuffs and other generic
buffers don't know what device they'll end up at so they need to come
from a generic allocator)

The current USB driver design seems pretty reasonable: only the HCD
drivers need to know about DMA mappings, and other USB drivers just
pass buffer addresses.  I don't think you would get much support for
forcing every driver to handle its own DMA mapping.

I would like to see both dev_map_xxx etc. and something like
__dma_buffer go into the kernel.  I think they both have their uses.

Best,
  Roland

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

> Let's go back to the beginning of this discussion since I think we're
> losing sight of the original problem.

I think actually a couple were unearthed, but you're right to focus.


>  In general I certainly support
> the idea of making the DMA mapping stuff device generic instead of
> tied to PCI.  

PCI was a good place to start (focus ... :) but clearly it shouldn't
be the only bus architecture with such support.  Note that there are
actually two related approaches in the DMA-mapping.txt APIs ... one is
DMA mapping, the other is "consistent memory".  Both should be made
generic rather than PCI-specific, not just mapping APIs.


> However, this discussion started when I fixed some problems I was
> having with USB on my IBM PowerPC 440GP system (which is not cache
> coherent).  

... which basically led to discussion of options I summarized a while
back as (a) expose the issues to drivers via macros, or (b) expose
them through some other API.

And DaveM wanted to focus on (b) options that involve exposing
consistent memory to drivers as buffers (sized in multiples of
cachelines, where that matters) rather than DMA-mapping them.

Based on the discussion, I think the answer for now is to go with
the (b) variant you had originally started with, using kmalloc for
the buffers.  The __dma_buffer style macro didn't seem popular;
though I agree that it's not clear kmalloc() really solves it
today.  (Given DaveM's SPARC example, the minimum size value it
returns would need to be 128 bytes ... which clearly isn't so.)


> The current USB driver design seems pretty reasonable: only the HCD
> drivers need to know about DMA mappings, and other USB drivers just
> pass buffer addresses.  I don't think you would get much support for
> forcing every driver to handle its own DMA mapping.

Me either.  But I suspect that it'd be good to have that as an option;
maybe just add a transfer_dma field to the URB, and have the HCD use
that, instead of creating a mapping, when transfer_buffer is null.
That'd certainly be a better approach for supporting sglist in the
usb-storage code than the alternatives I've heard so far.


> I would like to see both dev_map_xxx etc. and something like
> __dma_buffer go into the kernel.  I think they both have their uses.

Got Patch?  Actually, I know you do, I shouldn't ask.  :)

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[Oliver Neukum]

> Based on the discussion, I think the answer for now is to go with
> the (b) variant you had originally started with, using kmalloc for
> the buffers.  The __dma_buffer style macro didn't seem popular;
> though I agree that it's not clear kmalloc() really solves it
> today.  (Given DaveM's SPARC example, the minimum size value it
> returns would need to be 128 bytes ... which clearly isn't so.)

Perhaps I might point out that we need a solution for 2.4.
We certainly could go for kmalloc. But given the sparc64 example,
it won't work. If you do it with alignment in your own buffer, you can
control which fields are accessed.
We'd need to export a worst case minimum size for kmalloc.
Ugly, very ugly. Tons of kmalloc( size>MIN_DMA ? size : MIN_DMA,...

But it seems that this would mean quite a painful change in many drivers.
Which alignment macros wouldn't mean. In fact in the common case
there'd be no code change at all.
So if we take stability as a guideline, the alignment macros win without
question.

[..]
> That'd certainly be a better approach for supporting sglist in the
> usb-storage code than the alternatives I've heard so far.

Could you elaborate ? This sounds interesting.

> > I would like to see both dev_map_xxx etc. and something like
> > __dma_buffer go into the kernel.  I think they both have their uses.
>
> Got Patch?  Actually, I know you do, I shouldn't ask.  :)

So perhaps something could be done in time for 2.4.20 ?

	Regards
		Oliver

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Wed, 12 Jun 2002 12:13:08 -0700

   > The current USB driver design seems pretty reasonable: only the HCD
   > drivers need to know about DMA mappings, and other USB drivers just
   > pass buffer addresses.  I don't think you would get much support for
   > forcing every driver to handle its own DMA mapping.
   
   Me either.  But I suspect that it'd be good to have that as an option;
   maybe just add a transfer_dma field to the URB, and have the HCD use
   that, instead of creating a mapping, when transfer_buffer is null.
   That'd certainly be a better approach for supporting sglist in the
   usb-storage code than the alternatives I've heard so far.

Another solution for the "small chunks" issue is to in fact keep all
of the real DMA buffers in the host controller driver.  Ie. a pool
of tiny consitent DMA memory bounce buffers.  That is an idea for 
discussion, I have no particular preference.

This could actually improve performance for things like the input
layer USB drivers.  On several systems multiple cache lines are
fetched when a keyboard key is typed, and this is because we use
streaming buffers instead of consistent ones for these transfers.

We have two problems we want to solve, the DMA alignment stuff and
using consistent memory for these small buffers.  Therefore moving to
consistent memory (by whatever mechanism the USB desires to implement
this) is the way to go.

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Wed, 12 Jun 2002 12:13:08 -0700

   >  In general I certainly support
   > the idea of making the DMA mapping stuff device generic instead of
   > tied to PCI.  
   
   PCI was a good place to start (focus ... :) but clearly it shouldn't
   be the only bus architecture with such support.  Note that there are
   actually two related approaches in the DMA-mapping.txt APIs ... one is
   DMA mapping, the other is "consistent memory".  Both should be made
   generic rather than PCI-specific, not just mapping APIs.
   
I tried to imply this, if I didn't it is my error.

The intention is that every interface in DMA-mapping.txt will have
a dev_* counterpart when we move away from pci_dev to the generic
device struct.

One idea I want people to avoid is that we end up trying to do DMA
operations from a USB generic device struct.  This will lead to
multiple levels of indirection to do the PCI dma operation (USB device
--> USB bus --> PCI bus --> PCI bus DMA ops) and that will be ugly as
hell.
   
   Based on the discussion, I think the answer for now is to go with
   the (b) variant you had originally started with, using kmalloc for
   the buffers.  The __dma_buffer style macro didn't seem popular;
   though I agree that it's not clear kmalloc() really solves it
   today.  (Given DaveM's SPARC example, the minimum size value it
   returns would need to be 128 bytes ... which clearly isn't so.)
   
Let's ignore the sparc64 issue for now.  It's really a red herring
because as Pavel mentioned I could just use the largest size.

In actuality the "sparc64 issue" was partially a straw-man.  Sparc64
has none of the DMA alignment issues as the PCI controller provides
coherency of partial cacheline transfers, we just need to flush
pending writes and prefetched reads out of the PCI controllers around
non-consistent DMA transfers.
   
Franks a lot,
David S. Miller
davem@redhat.com

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: Oliver Neukum <oliver@neukum.name>
   Date: Wed, 12 Jun 2002 21:58:55 +0200

   Perhaps I might point out that we need a solution for 2.4.

For 2.4.x just do the DMA alignment macro idea.  That would
be the easiest change to verify.

Re: PCI DMA to small buffers on cache-incoherent arch

[Oliver Neukum]

Am Donnerstag, 13. Juni 2002 00:51 schrieb David S. Miller:
>    From: Oliver Neukum <oliver@neukum.name>
>    Date: Wed, 12 Jun 2002 21:58:55 +0200
>
>    Perhaps I might point out that we need a solution for 2.4.
>
> For 2.4.x just do the DMA alignment macro idea.  That would
> be the easiest change to verify.

OK, there we have something to work on.
Apart from the drivers I've already mailed, struct scsi_cmnd
is affected. kmalloc might be a problem.

Could we agree on a name for the macro ?

	Regards
		Oliver

[PATCH] 2.4 add __dma_buffer alignment macro

[Roland Dreier]

Following up on the "PCI DMA to small buffers on cache-incoherent
arch" discussion, here is a patch that adds <asm/dma_buffer.h>, which
contains a __dma_buffer alignment macro for DMA into buffers in the
middle of structures.

I only implemented for i386 and ppc, I don't know enough about other
architectures to get them right.

Thanks,
  Roland

 Documentation/DMA-mapping.txt    |   27 +++++++++++++++++++++++++++
 include/asm-alpha/dma_buffer.h   |   22 ++++++++++++++++++++++
 include/asm-arm/dma_buffer.h     |   22 ++++++++++++++++++++++
 include/asm-cris/dma_buffer.h    |   22 ++++++++++++++++++++++
 include/asm-i386/dma_buffer.h    |   23 +++++++++++++++++++++++
 include/asm-ia64/dma_buffer.h    |   22 ++++++++++++++++++++++
 include/asm-m68k/dma_buffer.h    |   22 ++++++++++++++++++++++
 include/asm-mips/dma_buffer.h    |   22 ++++++++++++++++++++++
 include/asm-mips64/dma_buffer.h  |   22 ++++++++++++++++++++++
 include/asm-parisc/dma_buffer.h  |   22 ++++++++++++++++++++++
 include/asm-ppc/dma_buffer.h     |   38 ++++++++++++++++++++++++++++++++++++++
 include/asm-s390/dma_buffer.h    |   22 ++++++++++++++++++++++
 include/asm-s390x/dma_buffer.h   |   22 ++++++++++++++++++++++
 include/asm-sh/dma_buffer.h      |   22 ++++++++++++++++++++++
 include/asm-sparc/dma_buffer.h   |   22 ++++++++++++++++++++++
 include/asm-sparc64/dma_buffer.h |   22 ++++++++++++++++++++++
 16 files changed, 374 insertions(+)

diff -Naur linux-2.4.18.orig/Documentation/DMA-mapping.txt linux-2.4.18/Documentation/DMA-mapping.txt
--- linux-2.4.18.orig/Documentation/DMA-mapping.txt	Mon Feb 25 11:37:51 2002
+++ linux-2.4.18/Documentation/DMA-mapping.txt	Wed Jun 12 13:38:30 2002
@@ -67,6 +67,33 @@
 networking subsystems make sure that the buffers they use are valid
 for you to DMA from/to.
 
+Note that on non-cache-coherent architectures, having a DMA buffer
+that shares a cache line with other data can lead to memory
+corruption.  The __dma_buffer macro defined in <asm/dma_buffer.h>
+exists to allow safe DMA buffers to be declared easily and portably as
+part of larger structures without causing bloat on cache-coherent
+architectures.  Of course these structures must be contained in memory
+that can be used for DMA as described above.
+
+To use __dma_buffer, just declare a struct like:
+
+	struct mydevice {
+		    int field1;
+		    char buffer[BUFFER_SIZE] __dma_buffer;
+		    int field2;
+	};
+	
+If this is used in code like:
+
+	struct mydevice *dev;
+	dev = kmalloc(sizeof *dev, GFP_KERNEL);
+
+then dev->buffer will be safe for DMA on all architectures.  On a
+cache-coherent architecture the members of dev will be aligned exactly
+as they would have been without __dma_buffer; on a non-cache-coherent
+architecture buffer and field2 will be aligned so that buffer does not
+share a cache line with any other data.
+
 			DMA addressing limitations
 
 Does your device have any DMA addressing limitations?  For example, is
diff -Naur linux-2.4.18.orig/include/asm-alpha/dma_buffer.h linux-2.4.18/include/asm-alpha/dma_buffer.h
--- linux-2.4.18.orig/include/asm-alpha/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-alpha/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_ALPHA_DMA_BUFFER_H
+#define _ASM_ALPHA_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for alpha
+
+#endif /* _ASM_ALPHA_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-arm/dma_buffer.h linux-2.4.18/include/asm-arm/dma_buffer.h
--- linux-2.4.18.orig/include/asm-arm/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-arm/dma_buffer.h	Wed Jun 12 13:45:45 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_ARM_DMA_BUFFER_H
+#define _ASM_ARM_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for arm
+
+#endif /* _ASM_ARM_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-cris/dma_buffer.h linux-2.4.18/include/asm-cris/dma_buffer.h
--- linux-2.4.18.orig/include/asm-cris/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-cris/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_CRIS_DMA_BUFFER_H
+#define _ASM_CRIS_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for cris
+
+#endif /* _ASM_CRIS_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-i386/dma_buffer.h linux-2.4.18/include/asm-i386/dma_buffer.h
--- linux-2.4.18.orig/include/asm-i386/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-i386/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,23 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_I386_DMA_BUFFER_H
+#define _ASM_I386_DMA_BUFFER_H
+
+/* We have cache-coherent DMA so __dma_buffer is defined to be a NOP */
+#define __dma_buffer
+
+#endif /* _ASM_I386_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-ia64/dma_buffer.h linux-2.4.18/include/asm-ia64/dma_buffer.h
--- linux-2.4.18.orig/include/asm-ia64/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-ia64/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_IA64_DMA_BUFFER_H
+#define _ASM_IA64_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for ia64
+
+#endif /* _ASM_IA64_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-m68k/dma_buffer.h linux-2.4.18/include/asm-m68k/dma_buffer.h
--- linux-2.4.18.orig/include/asm-m68k/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-m68k/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_M68K_DMA_BUFFER_H
+#define _ASM_M68K_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for m68k
+
+#endif /* _ASM_M68K_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-mips/dma_buffer.h linux-2.4.18/include/asm-mips/dma_buffer.h
--- linux-2.4.18.orig/include/asm-mips/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-mips/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_MIPS_DMA_BUFFER_H
+#define _ASM_MIPS_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for mips
+
+#endif /* _ASM_MIPS_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-mips64/dma_buffer.h linux-2.4.18/include/asm-mips64/dma_buffer.h
--- linux-2.4.18.orig/include/asm-mips64/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-mips64/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_MIPS64_DMA_BUFFER_H
+#define _ASM_MIPS64_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for mips64
+
+#endif /* _ASM_MIPS64_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-parisc/dma_buffer.h linux-2.4.18/include/asm-parisc/dma_buffer.h
--- linux-2.4.18.orig/include/asm-parisc/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-parisc/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_PARISC_DMA_BUFFER_H
+#define _ASM_PARISC_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for parisc
+
+#endif /* _ASM_PARISC_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-ppc/dma_buffer.h linux-2.4.18/include/asm-ppc/dma_buffer.h
--- linux-2.4.18.orig/include/asm-ppc/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-ppc/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,38 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.  */
+
+#ifdef __KERNEL__
+#ifndef _ASM_PPC_DMA_BUFFER_H
+#define _ASM_PPC_DMA_BUFFER_H
+
+#include <linux/config.h>
+
+#ifdef CONFIG_NOT_COHERENT_CACHE
+
+#include <asm/cache.h>
+
+#define __dma_buffer __dma_buffer_line(__LINE__)
+#define __dma_buffer_line(line) __dma_buffer_expand_line(line)
+#define __dma_buffer_expand_line(line) \
+	__attribute__ ((aligned(L1_CACHE_BYTES))); \
+	char __dma_pad_ ## line [0] __attribute__ ((aligned(L1_CACHE_BYTES)))
+
+#else  /* CONFIG_NOT_COHERENT_CACHE */
+
+/* We have cache-coherent DMA so __dma_buffer is defined to be a NOP */
+#define __dma_buffer
+
+#endif /* CONFIG_NOT_COHERENT_CACHE */
+
+#endif /* _ASM_PPC_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-s390/dma_buffer.h linux-2.4.18/include/asm-s390/dma_buffer.h
--- linux-2.4.18.orig/include/asm-s390/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-s390/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_S390_DMA_BUFFER_H
+#define _ASM_S390_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for s390
+
+#endif /* _ASM_S390_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-s390x/dma_buffer.h linux-2.4.18/include/asm-s390x/dma_buffer.h
--- linux-2.4.18.orig/include/asm-s390x/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-s390x/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_S390X_DMA_BUFFER_H
+#define _ASM_S390X_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for s390x
+
+#endif /* _ASM_S390X_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-sh/dma_buffer.h linux-2.4.18/include/asm-sh/dma_buffer.h
--- linux-2.4.18.orig/include/asm-sh/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-sh/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_SH_DMA_BUFFER_H
+#define _ASM_SH_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for sh
+
+#endif /* _ASM_SH_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-sparc/dma_buffer.h linux-2.4.18/include/asm-sparc/dma_buffer.h
--- linux-2.4.18.orig/include/asm-sparc/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-sparc/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_SPARC_DMA_BUFFER_H
+#define _ASM_SPARC_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for sparc
+
+#endif /* _ASM_SPARC_DMA_BUFFER_H */
+#endif /* __KERNEL__ */
diff -Naur linux-2.4.18.orig/include/asm-sparc64/dma_buffer.h linux-2.4.18/include/asm-sparc64/dma_buffer.h
--- linux-2.4.18.orig/include/asm-sparc64/dma_buffer.h	Wed Dec 31 16:00:00 1969
+++ linux-2.4.18/include/asm-sparc64/dma_buffer.h	Wed Jun 12 13:38:30 2002
@@ -0,0 +1,22 @@
+/*
+ *  __dma_buffer macro for safe DMA into struct members
+ *
+ *  See "What memory is DMA'able?" in Documentation/DMA-mapping.txt
+ *  for more information.
+ *
+ *  Copyright (c) 2002  Roland Dreier <roland@digitalvampire.org>
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  as published by the Free Software Foundation; either version
+ *  2 of the License, or (at your option) any later version.
+ */
+
+#ifdef __KERNEL__
+#ifndef _ASM_SPARC64_DMA_BUFFER_H
+#define _ASM_SPARC64_DMA_BUFFER_H
+
+#warning <asm/dma_buffer.h> is not implemented yet for sparc64
+
+#endif /* _ASM_SPARC64_DMA_BUFFER_H */
+#endif /* __KERNEL__ */

Re: [PATCH] 2.4 add __dma_buffer alignment macro

[David S. Miller]

Just use asm/dma.h, no need to make a new file.

Re: [PATCH] 2.4 add __dma_buffer alignment macro

[Roland Dreier]

>>>>> "David" == David S Miller <davem@redhat.com> writes:

    David> Just use asm/dma.h, no need to make a new file.

I could do that, however my thinking was that if I added it to an
existing file then it would add to the existing nested include bloat.
For example <asm/dma.h> for i386 has 10 inline functions and includes
<linux/config.h>, <linux/spinlock.h>, <asm/io.h> and <linux/delay.h>.
Seems kind of excessive just to get one macro that ends up being the
empty string, and I would prefer not to force people to include all
that just to declare a structure.

Best,
  Roland

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

>>That'd certainly be a better approach for supporting sglist in the
>>usb-storage code than the alternatives I've heard so far.
>
> Could you elaborate ? This sounds interesting.

It's been discussed on linux-usb-devel more than once.

It's really a secondary or tertiary consideration.  The top
performance issue for usb-storage in USB 2.0 is that it doesn't
yet use bulk queueing for its requests ... which means that it
wastes at least half the bandwidth available to it (staying in
the low tens of MByte/sec vs twenties or even higher). [*]

The sglist issue is a "how to create fewer DMA mappings" issue.
There are specific sglist DMA mapping calls, but they can't
be used without adding to the usbcore API ... either sglist,
usable only by usb-storage AFAICT, or something more general
like dma_addr_t.

In any case it's an optimization issue that doesn't seem like it
should be at the front of anyone's list for some time.

- Dave

[*] Yes, patches have been available to fix it.  Not updated
     after the block I/O changes in 2.5, but not hard to do so.

     Worth updating and integrating after the HCD stuff settles
     down so we can make sure that all three major HCDs do the
     bulk queuing correctly in both success and failure paths.

Re: PCI DMA to small buffers on cache-incoherent arch

[David Brownell]

David S. Miller wrote:
>    From: David Brownell <david-b@pacbell.net>
>    Date: Wed, 12 Jun 2002 12:13:08 -0700
> 
>    > The current USB driver design seems pretty reasonable: only the HCD
>    > drivers need to know about DMA mappings, and other USB drivers just
>    > pass buffer addresses.  I don't think you would get much support for
>    > forcing every driver to handle its own DMA mapping.
>    
>    Me either.  But I suspect that it'd be good to have that as an option;
>    maybe just add a transfer_dma field to the URB, and have the HCD use
>    that, instead of creating a mapping, when transfer_buffer is null.
>    That'd certainly be a better approach for supporting sglist in the
>    usb-storage code than the alternatives I've heard so far.
> 
> Another solution for the "small chunks" issue is to in fact keep all
> of the real DMA buffers in the host controller driver.  Ie. a pool
> of tiny consitent DMA memory bounce buffers.  That is an idea for 
> discussion, I have no particular preference.

Interesting idea.  I'd rather have the "hcd framework" layer
manage such stuff than have each of N host controller drivers
trying to do the same thing though ... less likely to turn
into bug heaven.


> This could actually improve performance for things like the input
> layer USB drivers.  On several systems multiple cache lines are
> fetched when a keyboard key is typed, and this is because we use
> streaming buffers instead of consistent ones for these transfers.

And that's exactly the problem scenario, since it uses the "automagic
resubmit" model with interrupt transfers.  Each of the HCDs needs to
handle that differently -- different behaviors, different bugs, yeech.
I won't go into it now, but a queued transfer model would work a lot
better.


> We have two problems we want to solve, the DMA alignment stuff and
> using consistent memory for these small buffers.  Therefore moving to
> consistent memory (by whatever mechanism the USB desires to implement
> this) is the way to go.

Right, the alignment stuff is a correctness issue, the consistent
memory issue is a performance concern.  I like to think that 2.5
will have a lot less correctness issues in the USB stack, so it
can start to pay more attention to performance concerns.

- Dave

Re: PCI DMA to small buffers on cache-incoherent arch

[David S. Miller]

   From: David Brownell <david-b@pacbell.net>
   Date: Wed, 12 Jun 2002 22:13:17 -0700

   David S. Miller wrote:
   > We have two problems we want to solve, the DMA alignment stuff and
   > using consistent memory for these small buffers.  Therefore moving to
   > consistent memory (by whatever mechanism the USB desires to implement
   > this) is the way to go.
   
   Right, the alignment stuff is a correctness issue, the consistent
   memory issue is a performance concern.  I like to think that 2.5
   will have a lot less correctness issues in the USB stack, so it
   can start to pay more attention to performance concerns.

I want to reemphasize that by going to consistent memory we solve
both problems, in particular the alignment stuff becomes a non-issue.