* Re: PCI DMA to small buffers on cache-incoherent arch
@ 2002-06-11 5:31 David Brownell
2002-06-11 5:44 ` David S. Miller
0 siblings, 1 reply; 26+ messages in thread
From: David Brownell @ 2002-06-11 5:31 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-kernel
> 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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-11 5:31 PCI DMA to small buffers on cache-incoherent arch David Brownell @ 2002-06-11 5:44 ` David S. Miller 2002-06-11 15:12 ` David Brownell 0 siblings, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-11 5:44 UTC (permalink / raw) To: david-b; +Cc: linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-11 5:44 ` David S. Miller @ 2002-06-11 15:12 ` David Brownell 2002-06-11 15:44 ` Oliver Neukum 2002-06-12 3:25 ` David S. Miller 0 siblings, 2 replies; 26+ messages in thread From: David Brownell @ 2002-06-11 15:12 UTC (permalink / raw) To: David S. Miller; +Cc: linux-kernel > 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-11 15:12 ` David Brownell @ 2002-06-11 15:44 ` Oliver Neukum 2002-06-12 3:25 ` David S. Miller 1 sibling, 0 replies; 26+ messages in thread From: Oliver Neukum @ 2002-06-11 15:44 UTC (permalink / raw) To: David Brownell, David S. Miller; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-11 15:12 ` David Brownell 2002-06-11 15:44 ` Oliver Neukum @ 2002-06-12 3:25 ` David S. Miller 2002-06-11 17:33 ` Benjamin Herrenschmidt 2002-06-12 6:25 ` PCI DMA to small buffers on cache-incoherent arch David Brownell 1 sibling, 2 replies; 26+ messages in thread From: David S. Miller @ 2002-06-12 3:25 UTC (permalink / raw) To: david-b; +Cc: linux-kernel 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). ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 3:25 ` David S. Miller @ 2002-06-11 17:33 ` Benjamin Herrenschmidt 2002-06-12 9:42 ` David S. Miller 2002-06-12 6:25 ` PCI DMA to small buffers on cache-incoherent arch David Brownell 1 sibling, 1 reply; 26+ messages in thread From: Benjamin Herrenschmidt @ 2002-06-11 17:33 UTC (permalink / raw) To: David S. Miller, david-b; +Cc: linux-kernel >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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-11 17:33 ` Benjamin Herrenschmidt @ 2002-06-12 9:42 ` David S. Miller 2002-06-12 14:14 ` David Brownell 0 siblings, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-12 9:42 UTC (permalink / raw) To: benh; +Cc: david-b, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 9:42 ` David S. Miller @ 2002-06-12 14:14 ` David Brownell 2002-06-12 15:00 ` Benjamin Herrenschmidt 2002-06-12 18:44 ` Roland Dreier 0 siblings, 2 replies; 26+ messages in thread From: David Brownell @ 2002-06-12 14:14 UTC (permalink / raw) To: David S. Miller; +Cc: benh, linux-kernel > 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 14:14 ` David Brownell @ 2002-06-12 15:00 ` Benjamin Herrenschmidt 2002-06-12 18:44 ` Roland Dreier 1 sibling, 0 replies; 26+ messages in thread From: Benjamin Herrenschmidt @ 2002-06-12 15:00 UTC (permalink / raw) To: David Brownell, David S. Miller; +Cc: linux-kernel > 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 14:14 ` David Brownell 2002-06-12 15:00 ` Benjamin Herrenschmidt @ 2002-06-12 18:44 ` Roland Dreier 2002-06-12 19:13 ` David Brownell 1 sibling, 1 reply; 26+ messages in thread From: Roland Dreier @ 2002-06-12 18:44 UTC (permalink / raw) To: David Brownell; +Cc: David S. Miller, benh, linux-kernel >>>>> "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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 18:44 ` Roland Dreier @ 2002-06-12 19:13 ` David Brownell 2002-06-12 19:58 ` Oliver Neukum ` (3 more replies) 0 siblings, 4 replies; 26+ messages in thread From: David Brownell @ 2002-06-12 19:13 UTC (permalink / raw) To: Roland Dreier; +Cc: David S. Miller, benh, linux-kernel > 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 19:13 ` David Brownell @ 2002-06-12 19:58 ` Oliver Neukum 2002-06-12 22:51 ` David S. Miller 2002-06-13 4:57 ` David Brownell 2002-06-12 22:46 ` David S. Miller ` (2 subsequent siblings) 3 siblings, 2 replies; 26+ messages in thread From: Oliver Neukum @ 2002-06-12 19:58 UTC (permalink / raw) To: David Brownell, Roland Dreier Cc: David S. Miller, benh, linux-kernel, linux-usb-devel > 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 19:58 ` Oliver Neukum @ 2002-06-12 22:51 ` David S. Miller 2002-06-12 23:17 ` Oliver Neukum 2002-06-13 4:57 ` David Brownell 1 sibling, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-12 22:51 UTC (permalink / raw) To: oliver; +Cc: david-b, roland, benh, linux-kernel, linux-usb-devel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 22:51 ` David S. Miller @ 2002-06-12 23:17 ` Oliver Neukum 0 siblings, 0 replies; 26+ messages in thread From: Oliver Neukum @ 2002-06-12 23:17 UTC (permalink / raw) To: David S. Miller; +Cc: david-b, roland, benh, linux-kernel, linux-usb-devel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 19:58 ` Oliver Neukum 2002-06-12 22:51 ` David S. Miller @ 2002-06-13 4:57 ` David Brownell 1 sibling, 0 replies; 26+ messages in thread From: David Brownell @ 2002-06-13 4:57 UTC (permalink / raw) To: Oliver Neukum; +Cc: linux-kernel, linux-usb-devel >>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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 19:13 ` David Brownell 2002-06-12 19:58 ` Oliver Neukum @ 2002-06-12 22:46 ` David S. Miller 2002-06-13 5:13 ` David Brownell 2002-06-12 22:50 ` David S. Miller 2002-06-13 0:23 ` [PATCH] 2.4 add __dma_buffer alignment macro Roland Dreier 3 siblings, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-12 22:46 UTC (permalink / raw) To: david-b; +Cc: roland, benh, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 22:46 ` David S. Miller @ 2002-06-13 5:13 ` David Brownell 2002-06-13 9:38 ` David S. Miller 0 siblings, 1 reply; 26+ messages in thread From: David Brownell @ 2002-06-13 5:13 UTC (permalink / raw) To: David S. Miller; +Cc: roland, benh, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-13 5:13 ` David Brownell @ 2002-06-13 9:38 ` David S. Miller 0 siblings, 0 replies; 26+ messages in thread From: David S. Miller @ 2002-06-13 9:38 UTC (permalink / raw) To: david-b; +Cc: roland, benh, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 19:13 ` David Brownell 2002-06-12 19:58 ` Oliver Neukum 2002-06-12 22:46 ` David S. Miller @ 2002-06-12 22:50 ` David S. Miller 2002-06-13 0:23 ` [PATCH] 2.4 add __dma_buffer alignment macro Roland Dreier 3 siblings, 0 replies; 26+ messages in thread From: David S. Miller @ 2002-06-12 22:50 UTC (permalink / raw) To: david-b; +Cc: roland, benh, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH] 2.4 add __dma_buffer alignment macro 2002-06-12 19:13 ` David Brownell ` (2 preceding siblings ...) 2002-06-12 22:50 ` David S. Miller @ 2002-06-13 0:23 ` Roland Dreier 2002-06-13 1:07 ` David S. Miller 3 siblings, 1 reply; 26+ messages in thread From: Roland Dreier @ 2002-06-13 0:23 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: David Brownell, Oliver Neukum, linux-kernel 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__ */ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] 2.4 add __dma_buffer alignment macro 2002-06-13 0:23 ` [PATCH] 2.4 add __dma_buffer alignment macro Roland Dreier @ 2002-06-13 1:07 ` David S. Miller 2002-06-13 1:19 ` Roland Dreier 0 siblings, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-13 1:07 UTC (permalink / raw) To: roland; +Cc: marcelo, david-b, oliver, linux-kernel Just use asm/dma.h, no need to make a new file. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] 2.4 add __dma_buffer alignment macro 2002-06-13 1:07 ` David S. Miller @ 2002-06-13 1:19 ` Roland Dreier 0 siblings, 0 replies; 26+ messages in thread From: Roland Dreier @ 2002-06-13 1:19 UTC (permalink / raw) To: David S. Miller; +Cc: marcelo, david-b, oliver, linux-kernel >>>>> "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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 3:25 ` David S. Miller 2002-06-11 17:33 ` Benjamin Herrenschmidt @ 2002-06-12 6:25 ` David Brownell 2002-06-12 6:24 ` David S. Miller 1 sibling, 1 reply; 26+ messages in thread From: David Brownell @ 2002-06-12 6:25 UTC (permalink / raw) To: David S. Miller; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 6:25 ` PCI DMA to small buffers on cache-incoherent arch David Brownell @ 2002-06-12 6:24 ` David S. Miller 2002-06-12 7:06 ` David Brownell 0 siblings, 1 reply; 26+ messages in thread From: David S. Miller @ 2002-06-12 6:24 UTC (permalink / raw) To: david-b; +Cc: linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 6:24 ` David S. Miller @ 2002-06-12 7:06 ` David Brownell 2002-06-12 9:22 ` David S. Miller 0 siblings, 1 reply; 26+ messages in thread From: David Brownell @ 2002-06-12 7:06 UTC (permalink / raw) To: David S. Miller; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: PCI DMA to small buffers on cache-incoherent arch 2002-06-12 7:06 ` David Brownell @ 2002-06-12 9:22 ` David S. Miller 0 siblings, 0 replies; 26+ messages in thread From: David S. Miller @ 2002-06-12 9:22 UTC (permalink / raw) To: david-b; +Cc: linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2002-06-13 9:43 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2002-06-11 5:31 PCI DMA to small buffers on cache-incoherent arch David Brownell 2002-06-11 5:44 ` David S. Miller 2002-06-11 15:12 ` David Brownell 2002-06-11 15:44 ` Oliver Neukum 2002-06-12 3:25 ` David S. Miller 2002-06-11 17:33 ` Benjamin Herrenschmidt 2002-06-12 9:42 ` David S. Miller 2002-06-12 14:14 ` David Brownell 2002-06-12 15:00 ` Benjamin Herrenschmidt 2002-06-12 18:44 ` Roland Dreier 2002-06-12 19:13 ` David Brownell 2002-06-12 19:58 ` Oliver Neukum 2002-06-12 22:51 ` David S. Miller 2002-06-12 23:17 ` Oliver Neukum 2002-06-13 4:57 ` David Brownell 2002-06-12 22:46 ` David S. Miller 2002-06-13 5:13 ` David Brownell 2002-06-13 9:38 ` David S. Miller 2002-06-12 22:50 ` David S. Miller 2002-06-13 0:23 ` [PATCH] 2.4 add __dma_buffer alignment macro Roland Dreier 2002-06-13 1:07 ` David S. Miller 2002-06-13 1:19 ` Roland Dreier 2002-06-12 6:25 ` PCI DMA to small buffers on cache-incoherent arch David Brownell 2002-06-12 6:24 ` David S. Miller 2002-06-12 7:06 ` David Brownell 2002-06-12 9:22 ` David S. Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox