From: David Herrmann <dh.herrmann@gmail.com>
To: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Austin S Hemmelgarn <ahferroin7@gmail.com>,
Daniel Mack <daniel@zonque.org>, Arnd Bergmann <arnd@arndb.de>,
"Eric W. Biederman" <ebiederm@xmission.com>,
One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>,
Tom Gundersen <teg@jklm.no>, "Theodore T'so" <tytso@mit.edu>,
Andy Lutomirski <luto@amacapital.net>,
Linux API <linux-api@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Djalal Harouni <tixxdz@opendz.org>,
Johannes Stezenbach <js@sig21.net>,
Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 01/13] kdbus: add documentation
Date: Tue, 27 Jan 2015 16:05:20 +0100 [thread overview]
Message-ID: <CANq1E4SkHhs1pWUe-TzG7bzk1M-Q++mB2vmQGuYx0RMF53wg4Q@mail.gmail.com> (raw)
In-Reply-To: <54C65346.5070504@gmail.com>
Hi
On Mon, Jan 26, 2015 at 3:46 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> Hello Greg,
>
> On 01/23/2015 05:08 PM, Greg Kroah-Hartman wrote:
>> On Thu, Jan 22, 2015 at 09:49:00AM -0500, Austin S Hemmelgarn wrote:
>>> While I agree that there should be a way for userspace to get the list of
>>> supported operations, userspace apps will only actually care about that
>>> once, when they begin talking to kdbus, because (ignoring the live kernel
>>> patching that people have been working on recently) the list of supported
>>> operations isn't going to change while the system is running. While a u64
>>> copy has relatively low overhead, it does have overhead, and that is very
>>> significant when you consider part of the reason some people want kdbus is
>>> for the performance gain. Especially for those automotive applications that
>>> have been mentioned which fire off thousands of messages during start-up,
>>> every little bit of performance is significant.
>>
>> A single u64 in a structure is not going to be measurable at all,
>> processors just copy memory too fast these days for 4 extra bytes to be
>> noticable.
>
> It depends on the definition of measurable, I suppose, but this statement
> appears incorrect to me. In some cases (e.g., kdbus_msg_info) we're talking
> about *two* u64 fields (kernel_gs, kernel_msg_flags) being used to pass back
> sets of valid flags. That's 16 bytes, and it definitely makes a difference.
> Simply running a loop that does a naive memcpy() in a tight user-space
> loop (code below), I see the following for the execution of 1e9 loops:
>
> Including the two extra u64 fields: 3.2 sec
> Without the two extra u64 fields: 2.6 sec
>
> On the same box, doing 1e9 calls to getppid() (i.e., pretty much the
> simplest syscall, giving us a rough measure of the context switch) takes
> 68 seconds. In other words, the cost of copying those 16 bytes is about 1%
> of the base context switch/syscall cost. I assume the costs of copying
> those 16 bytes across the kernel-user-space boundary would not be cheaper,
> but have not tested that. If my assumption is correct, then 1% seems a
> significant figure to me in an API whose raison d'être is speed.
I have no idea how this is related to any kdbus ioctl?
A 16byte copy does not affect the performance of kdbus message
transactions in any way that matters.
>> So let's make this as easy as possible for userspace, making
>> it simpler logic there, which is much more important than saving
>> theoretical time in the kernel.
>
> But this also missed the other part of the point. Copying these fields on
> every operation, when in fact they are only needed once, clutters the API,
> in my opinion. Good APIs are as simple as they can be to do their job.
> Redundancy is an enemy of simplicity. Simplest would have been a one time
> API that returns a structure containing all of the supported flags across
> the API. Alternatively, the traditional EINVAL approach is well understood,
> and suffices.
We're going to drop "kernel_flags" in favor of a new
KDBUS_FLAG_NEGOTIATE flag which asks the kernel to do feature
negotiation for this ioctl and return the supported flags/items inline
(overwriting the passed data). The ioctl will not be executed and will
not affect the state of the FD.
I hope this keeps the API simple.
Thanks
David
next prev parent reply other threads:[~2015-01-27 15:05 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-16 19:16 [PATCH v3 00/13] Add kdbus implementation Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 01/13] kdbus: add documentation Greg Kroah-Hartman
2015-01-20 13:53 ` Michael Kerrisk (man-pages)
2015-01-20 14:31 ` David Herrmann
2015-01-20 14:42 ` Josh Boyer
2015-01-20 14:53 ` Djalal Harouni
2015-01-20 16:08 ` Johannes Stezenbach
2015-01-20 17:00 ` David Herrmann
2015-01-20 22:00 ` Johannes Stezenbach
2015-01-21 10:28 ` Michael Kerrisk (man-pages)
2015-01-20 18:23 ` Daniel Mack
2015-01-21 10:32 ` Michael Kerrisk (man-pages)
2015-01-21 15:19 ` Theodore Ts'o
2015-01-21 16:58 ` Daniel Mack
2015-01-22 10:18 ` Michael Kerrisk (man-pages)
2015-01-22 13:46 ` David Herrmann
2015-01-22 14:49 ` Austin S Hemmelgarn
2015-01-23 16:08 ` Greg Kroah-Hartman
2015-01-26 14:46 ` Michael Kerrisk (man-pages)
2015-01-27 15:05 ` David Herrmann [this message]
2015-01-27 16:03 ` Andy Lutomirski
2015-01-29 8:53 ` Daniel Mack
2015-01-29 11:25 ` Andy Lutomirski
2015-01-29 11:42 ` Daniel Mack
2015-01-29 12:09 ` Andy Lutomirski
2015-02-02 9:34 ` Daniel Mack
2015-02-02 20:12 ` Andy Lutomirski
2015-02-03 10:09 ` Daniel Mack
2015-02-04 0:41 ` Andy Lutomirski
2015-02-04 2:47 ` Eric W. Biederman
2015-02-04 3:14 ` Greg Kroah-Hartman
2015-02-04 6:30 ` Eric W. Biederman
2015-02-04 23:03 ` Andy Lutomirski
2015-02-05 0:16 ` David Herrmann
2015-02-08 16:54 ` Andy Lutomirski
2015-01-27 18:03 ` Michael Kerrisk (man-pages)
2015-01-23 11:47 ` Michael Kerrisk (man-pages)
2015-01-23 15:54 ` Greg Kroah-Hartman
2015-01-26 14:42 ` Michael Kerrisk (man-pages)
2015-01-26 15:26 ` Tom Gundersen
2015-01-26 16:44 ` christoph Hellwig
2015-01-26 16:45 ` Michael Kerrisk (man-pages)
2015-01-27 15:23 ` David Herrmann
2015-01-27 17:53 ` Michael Kerrisk (man-pages)
2015-01-27 18:14 ` Daniel Mack
2015-01-28 10:46 ` Michael Kerrisk (man-pages)
2015-01-20 13:58 ` Michael Kerrisk (man-pages)
2015-01-20 17:50 ` Daniel Mack
2015-01-21 8:57 ` Michael Kerrisk (man-pages)
2015-01-21 9:07 ` Daniel Mack
2015-01-21 9:07 ` Michael Kerrisk (man-pages)
2015-01-21 9:12 ` Daniel Mack
2015-01-23 6:28 ` Ahmed S. Darwish
2015-01-23 13:19 ` Greg Kroah-Hartman
2015-01-23 13:29 ` Greg Kroah-Hartman
2015-01-25 3:30 ` Ahmed S. Darwish
2015-01-16 19:16 ` [PATCH 02/13] kdbus: add header file Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 03/13] kdbus: add driver skeleton, ioctl entry points and utility functions Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 04/13] kdbus: add connection pool implementation Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 05/13] kdbus: add connection, queue handling and message validation code Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 06/13] kdbus: add node and filesystem implementation Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 07/13] kdbus: add code to gather metadata Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 08/13] kdbus: add code for notifications and matches Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 09/13] kdbus: add code for buses, domains and endpoints Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 10/13] kdbus: add name registry implementation Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 11/13] kdbus: add policy database implementation Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 12/13] kdbus: add Makefile, Kconfig and MAINTAINERS entry Greg Kroah-Hartman
2015-01-16 19:16 ` [PATCH 13/13] kdbus: add selftests Greg Kroah-Hartman
2015-01-16 22:07 ` [PATCH v3 00/13] Add kdbus implementation Josh Boyer
2015-01-16 22:18 ` Greg Kroah-Hartman
2015-01-17 0:26 ` Daniel Mack
2015-01-17 0:41 ` Josh Boyer
2015-01-19 18:06 ` Johannes Stezenbach
2015-01-19 18:38 ` Greg Kroah-Hartman
2015-01-19 20:19 ` Johannes Stezenbach
2015-01-19 20:31 ` Greg Kroah-Hartman
2015-01-19 23:38 ` Johannes Stezenbach
2015-01-20 1:13 ` Greg Kroah-Hartman
2015-01-20 10:57 ` Johannes Stezenbach
2015-01-20 11:26 ` Greg Kroah-Hartman
2015-01-20 13:24 ` Johannes Stezenbach
2015-01-20 14:12 ` Michael Kerrisk (man-pages)
2015-01-26 21:32 ` One Thousand Gnomes
2015-01-19 18:33 ` Johannes Stezenbach
2015-01-20 14:05 ` Michael Kerrisk (man-pages)
2015-01-20 14:15 ` Michael Kerrisk (man-pages)
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=CANq1E4SkHhs1pWUe-TzG7bzk1M-Q++mB2vmQGuYx0RMF53wg4Q@mail.gmail.com \
--to=dh.herrmann@gmail.com \
--cc=ahferroin7@gmail.com \
--cc=arnd@arndb.de \
--cc=daniel@zonque.org \
--cc=ebiederm@xmission.com \
--cc=gnomes@lxorguk.ukuu.org.uk \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=js@sig21.net \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mtk.manpages@gmail.com \
--cc=teg@jklm.no \
--cc=tixxdz@opendz.org \
--cc=tytso@mit.edu \
/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;
as well as URLs for NNTP newsgroup(s).