David Brownell wrote:
static inline ssize_t spi_w8r8(struct spi_device *spi, u8 cmd)
{
ssize_t status;
u8 result;
status = spi_write_then_read(spi, &cmd, 1, &result, 1);
/* return negative errno or unsigned value */
return (status < 0) ? status : result;
}
You're allocating u8 var on stack, then allocate a 1-byte-long buffer
and copy the data instead of letting the controller driver decide
whether this allocation/copy is necessary or not.
Yeah, like that matters in the face of the overhead to queue
the message, get to the head of the SPI transfer queue, go
through that queue, then finally wake up the task that was
synchronously blocking in write_then_read(). Oh, and since
that's inlined, GCC may be re-using existing state...
If folk want an "it looks simple" convenient/friendly API,
there is always a price to pay. In this case, that cost is
dwarfed by the mere fact that they're using a synchronous
model to access shared resources (the SPI controller).
It's just words; the patch I'm proposing cuts this price to nothing but
you're just being too stubborn to accept that. :(
Then he starts messing with allocate-or-use-preallocated stuff etc. etc.
Why isn't he just kmalloc'ing/kfree'ing buffers each time these
functions are called
So that the typical case, with little SPI contention, doesn't
hit the heap? That's sure what I thought ... though I can't speak
for what other people may think I thought. You were the one that
wanted to optimize the atypical case to remove a blocking path!
I meant kmalloc'ing/kfree'ing buffers is spi_w8r8/spi_w8r16/etc.
As I said: so the _typical_ case doesn't hit the heap. There are
inherent overheads for such RPC-style calls. But there's also no
point in gratuitously increasing
Sorry, increasing what?
Okay, lemme summarize: I've spent a day working out the minimal changes
I'd like to have added to your core and two days trying to convince
those changes are necessary. You just don't want to listen to me. So the
conclusion is that the convergence process has been deadlocked, and the
only way out for us is -- we'll continue to work on our core, as
there're things we'd like to have in SPI core and there're other people
using our core.
Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
