Messages in this thread | | | Date | Tue, 20 Dec 2005 21:15:53 +0300 | From | Vitaly Wool <> | Subject | Re: [PATCH/RFC] SPI core: turn transfers to be linked list |
| |
David Brownell wrote:
>>>Hmm, color me confused. Is there something preventing a driver from >>>having its own freelist (or whatever), in cases where kmalloc doesn't >>>suffice? If not, why should the core change? And what sort of driver >>>measurements are you doing, to conclude that kmalloc doesn't suffice? >>> >>> >>> >>> >>Can't get what you're talking about. A freelist of what? >> >> > >Of whatever the driver wants. You had pointed at some code that >boiled down to keeping a freelist of some structures in the core. >Such code doesn't _need_ to be in the core at all ... > > Correct. But you're missing two things: a) it wasn't in the core, it was the library and b) it doesn't matter. It's just a simple stack-based memory allocation model which is _very attractive_ to use and which *can't* be used with transfers as arrays.
> > > >>Basically the idea of the custom lightweight allocation is the >>following: allocate a page and divide into regions of the same size, >>initialize and use these regions as a stack. >> >> > >There already a lot of allocators that support that, like kmem_cache_t >or dma_pool or mempool_t ... no more, please! They don't all have that >"stack"/freelist notion though; easy for drivers to do that, IMO. > >Drivers that don't want to hit the heap will preallocate everything >they can, and they'll probably have their own freelists. That wins >by eliminating the slab subroutine calls from what are often hot >code paths, along with their fault handling logic ... and removing >the need (and testing) for fault handling wins by improving robustness. > > See b) above. Doesn't matter where to put an implementation of this memory allocation model. Matters whether it's usable or not. Currently it's not, especially if a driver is complicated enuff to have transfers of arbitrary size.
>>>I'd have said that since this increases the per-transfer costs (to set >>>up and manage the list memberships) you want to increase the weight of >>>that part of the API, not decrease it. ;) >>> >>> >>> >>> >>Disagree. Let's look deeper. kmalloc itself is more heavyweght than >>setting up list memberships. >> >> > >But kmalloc was previously optional, yes? And should still be ... > > Whoops. How can it be optional if we want to have an async transfer?
> > > >>The list setting commands are pretty essential and will not add a lot to >>the assembly code. >> >> > >I'm not totally averse to such changes, but you don't seem to be making >the best arguments. Example: they're clearly not "essential" because >transfer queues work today with the lists at the spi_message level. > > Not sure if I got you here, sorry.
> > > >>And your understanding is not quite correct. What if I'm going to send a >>chain of 5 messages? I'll allocate 5 spi_msg's in my case which all are >>gonna be of the same size -- thus the technique described above is >>applicable. In case of your core it's not. >> >> > >I must have been deceived then by this little utility: > >static inline struct spi_message *spi_message_alloc(unsigned ntrans, gfp_t flags) >{ > struct spi_message *m; > > m = kzalloc(sizeof(struct spi_message) > + ntrans * sizeof(struct spi_transfer), > flags); > if (m) { > m->transfers = (void *)(m + 1); > m->n_transfer = ntrans; > } > return m; >} > >Sure looks to me like spi_message_alloc(5, SLAB_ATOMIC) should do the trick. >One allocation, always the same size. And kzalloc() could easily optimize >that to use the "size-192" slab cache (or whatever), like kmalloc() does; >that'd be a small speedup. > > '5' is only an example. What if there gonna be 7, 10, 12? What if the possible cases for a driver are 1, 2, 5, 7, 12 and 14 transfers per message? Of course the allocation won't be of the same size always. Moreover, the size is gonna differ a lot.
> > > >>>Could you elaborate on this problem you perceive? This isn't the only >>>driver API in Linux to talk in terms of arrays describing transfers, >>>even potentially large arrays. >>> >>> >>The problem is: we're using real-time enhancements patch developed by >>Ingo/Sven/Daniel etc. You cannot call kmalloc from the interrupt >>context if you're using this patch. Yeah, yeah -- the interrupt >>handlers are in threads by default, but we can't go for that since we >>want immediate acknowledgement from the interrupt context, and that >>implies spi_message/spi_transfer allocation. >> >> > >Could you elaborate a bit here? You seem to be implying that for some >reason one of your SPI related drivers must use non-threaded hardirqs, >AND (news to me, if true) that such hardirqs can't kmalloc(), AND that >it can't use any of several widely used strategies to avoid hitting >things like the slab allocator. (That last seems hardest to believe...) > >I asked earlier what sort of performance measurements you're making >that are leading you to these conclusions. I'm still wondering. :) > > This is mostly a stability issue, not a performance thing.
>>>Consider how "struct scatterlist" is used, and how USB manages the >>>descriptors for isochronous transfers. They don't use linked lists >>>there, and haven't seemed to suffer from it. >>> >>> >>> >>> >>Not sure if I understand why it's relevant to what we're discussing. >> >> > >Examples of driver interfaces in Linux that work fine using arrays to couple >a group of transfers. If you see some problem that makes a compelling >argument that the SPI must change, it would also affect drivers using >those interfaces too, at least a little bit ... right? Does it? > > I'm not concerned much with USB mow :), though I know guys having _a lot_ of trouble trying USB devices work within the real-time Linux enhancememts environment I was referring to earlier ;) Probably a lot of memory allocations within interrupt context... which is a always-avoid thing for any real-time system BTW...
Vitaly - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |