lkml.org 
[lkml]   [2005]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2005-12-20 19:17    [W:5.241 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site