From: Vitaly Wool <vwool@ru.mvista.com>
To: David Brownell <david-b@pacbell.net>
Cc: Greg KH <greg@kroah.com>,
linux-kernel@vger.kernel.org, dpervushin@gmail.com,
akpm@osdl.org, basicmark@yahoo.com, komal_shah802003@yahoo.com,
stephen@streetfiresound.com,
spi-devel-general@lists.sourceforge.net, Joachim_Jaeger@digi.com
Subject: Re: [PATCH/RFC] SPI: add DMAUNSAFE analog
Date: Wed, 14 Dec 2005 23:11:05 +0300 [thread overview]
Message-ID: <43A07C59.4050807@ru.mvista.com> (raw)
In-Reply-To: <200512141117.11244.david-b@pacbell.net>
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
next prev parent reply other threads:[~2005-12-14 20:11 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-12-12 15:20 [PATCH 2.6-git 0/4] SPI core refresh Vitaly Wool
2005-12-12 15:22 ` [PATCH 2.6-git 1/4] SPI core refresh: SPI core patch Vitaly Wool
2005-12-12 15:49 ` Russell King
2005-12-12 15:24 ` [PATCH 2.6-git 2/4] SPI core refresh: MTD dataflash driver Vitaly Wool
2005-12-12 15:26 ` [PATCH 2.6-git 3/4] SPI core refresh: SPI/PNX controller Vitaly Wool
2005-12-12 15:27 ` [PATCH 2.6-git 4/4] SPI core refresh: dumb EEPROM driver Vitaly Wool
2005-12-12 18:01 ` [PATCH 2.6-git 0/4] SPI core refresh Rui Sousa
2005-12-13 12:09 ` [spi-devel-general] " dmitry pervushin
2005-12-13 15:11 ` Rui Sousa
2005-12-13 17:06 ` dmitry pervushin
2005-12-14 6:57 ` Vitaly Wool
2005-12-14 14:28 ` Rui Sousa
2005-12-13 16:35 ` David Brownell
2005-12-13 18:02 ` Rui Sousa
2005-12-13 14:06 ` [PATCH/RFC] SPI: add async message handing library to David Brownell's core Vitaly Wool
2005-12-13 16:53 ` [PATCH/RFC] SPI: add DMAUNSAFE analog " Vitaly Wool
2005-12-13 19:01 ` David Brownell
2005-12-13 19:15 ` Greg KH
2005-12-14 13:50 ` Vitaly Wool
2005-12-14 17:18 ` Greg KH
2005-12-14 17:53 ` Vitaly Wool
2005-12-14 18:50 ` [PATCH/RFC] SPI: add DMAUNSAFE analog David Brownell
2005-12-14 19:29 ` Vitaly Wool
2005-12-14 19:02 ` [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core David Brownell
2005-12-14 19:19 ` Vitaly Wool
2005-12-14 19:33 ` [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog David Brownell
2005-12-14 19:34 ` [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core Vitaly Wool
2005-12-15 6:47 ` Vitaly Wool
2005-12-15 16:44 ` Greg KH
2005-12-15 22:23 ` Vitaly Wool
2005-12-15 23:02 ` Greg KH
2005-12-16 8:37 ` Vitaly Wool
2005-12-16 17:34 ` Greg KH
2005-12-16 18:32 ` [spi-devel-general] Re: [PATCH/RFC] SPI: add DMAUNSAFE analog David Brownell
2005-12-15 20:06 ` David Brownell
2005-12-15 22:17 ` Vitaly Wool
2005-12-15 22:33 ` Greg KH
2005-12-16 3:34 ` Andy Isaacson
2005-12-16 5:17 ` Greg KH
2005-12-14 19:16 ` [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core Greg KH
2005-12-14 19:30 ` Vitaly Wool
2005-12-15 10:00 ` [spi-devel-general] " dmitry pervushin
2005-12-14 17:22 ` David Brownell
2005-12-14 17:50 ` Vitaly Wool
2005-12-14 19:17 ` [PATCH/RFC] SPI: add DMAUNSAFE analog David Brownell
2005-12-14 20:11 ` Vitaly Wool [this message]
2005-12-13 21:47 ` [PATCH/RFC] SPI: add DMAUNSAFE analog to David Brownell's core Vitaly Wool
2005-12-13 22:15 ` Vitaly Wool
2005-12-14 16:55 ` David Brownell
2005-12-14 17:23 ` Vitaly Wool
2005-12-14 18:48 ` [PATCH/RFC] SPI: add async message handing library " Stephen Street
2005-12-14 19:41 ` Vitaly Wool
2005-12-14 21:19 ` Stephen Street
2005-12-14 19:31 ` [PATCH/RFC] SPI: add async message handing library David Brownell
2005-12-15 12:19 ` [PATCH/RFC] SPI: async message handing library update Vitaly Wool
2005-12-18 18:59 ` David Brownell
2005-12-19 15:40 ` [spi-devel-general] " dmitry pervushin
2005-12-20 7:23 ` David Brownell
2005-12-20 18:02 ` Vitaly Wool
2005-12-22 17:28 ` David Brownell
2005-12-22 22:10 ` Vitaly Wool
2005-12-22 23:55 ` David Brownell
2005-12-21 13:17 ` Vitaly Wool
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=43A07C59.4050807@ru.mvista.com \
--to=vwool@ru.mvista.com \
--cc=Joachim_Jaeger@digi.com \
--cc=akpm@osdl.org \
--cc=basicmark@yahoo.com \
--cc=david-b@pacbell.net \
--cc=dpervushin@gmail.com \
--cc=greg@kroah.com \
--cc=komal_shah802003@yahoo.com \
--cc=linux-kernel@vger.kernel.org \
--cc=spi-devel-general@lists.sourceforge.net \
--cc=stephen@streetfiresound.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox