qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com,
	libvir-list@redhat.com, Corey Bryant <coreyb@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets
Date: Thu, 26 Jul 2012 15:16:35 +0200	[thread overview]
Message-ID: <50114333.7090209@redhat.com> (raw)
In-Reply-To: <5011425E.8030201@redhat.com>

Am 26.07.2012 15:13, schrieb Eric Blake:
> On 07/25/2012 09:21 PM, Corey Bryant wrote:
>>
>>
>> On 07/25/2012 03:25 PM, Eric Blake wrote:
>>> On 07/25/2012 02:22 AM, Kevin Wolf wrote:
>>>>>> Hm, not a nice interface where qemu_close() needs the filename and
>>>>>> (worse) could be given a wrong filename. Maybe it would be better to
>>>>>> maintain a list of fd -> fdset mappings in qemu_open/close?
>>>>>>
>>>>>
>>>>> I agree, I don't really like it either.
>>>>>
>>>>> We already have a list of fd -> fdset mappings (mon_fdset_fd_t ->
>>>>> mon_fdset_t).  Would it be too costly to loop through all the
>>>>> fdsets/fds
>>>>> at the beginning of every qemu_close()?
>>>>
>>>> I don't think so. qemu_close() is not a fast path and happens almost
>>>> never, and the list is short enough that searching it isn't a problem
>>>> anyway.
>>>
>>> I agree - just do the loop to do the reverse lookup yourself, rather
>>> than making qemu_close() have a different signature than close().
>>>
>>
>> Great, I'll do this then.
> 
> You may want an optimization of using a bitset for tracking which fds
> are tracked by fdset in the first place, so that the fast path of
> qemu_close() will be a check against the bitset to see if you even have
> to waste time on the reverse lookup in the first place.  The bitset will
> typically be small (bounded not only by the maximum possible fd, but
> further by the fact that we don't usually open that many fds in the
> first place), but I'm not sure if you can get away with static sizing.

Premature optimisation, in my opinion. The list is really small.

Kevin

  reply	other threads:[~2012-07-26 13:17 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 13:07 [Qemu-devel] [PATCH v5 0/6] file descriptor passing using fd sets Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Corey Bryant
2012-07-23 22:50   ` Eric Blake
2012-07-24  2:19     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets Corey Bryant
2012-07-25 18:16   ` Eric Blake
2012-07-26  2:55     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 3/6] monitor: Clean up fd sets on monitor disconnect Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 4/6] block: Convert open calls to qemu_open Corey Bryant
2012-07-25 19:22   ` Eric Blake
2012-07-26  3:11     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 5/6] block: Convert close calls to qemu_close Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets Corey Bryant
2012-07-23 13:14   ` Corey Bryant
2012-08-02 22:21     ` Corey Bryant
2012-08-06  9:15       ` Kevin Wolf
2012-08-06 13:32         ` Corey Bryant
2012-08-06 13:51           ` Kevin Wolf
2012-08-06 14:15             ` Corey Bryant
2012-08-07 16:43               ` Corey Bryant
2012-07-24 12:07   ` Kevin Wolf
2012-07-25  3:41     ` Corey Bryant
2012-07-25  8:22       ` Kevin Wolf
2012-07-25 19:25         ` Eric Blake
2012-07-26  3:21           ` Corey Bryant
2012-07-26 13:13             ` Eric Blake
2012-07-26 13:16               ` Kevin Wolf [this message]
2012-07-27  4:07                 ` Corey Bryant
2012-07-25 19:43   ` Eric Blake
2012-07-26  3:57     ` Corey Bryant
2012-07-26  9:07       ` Kevin Wolf
2012-07-27  3:59         ` Corey Bryant
2012-07-27  4:03         ` Corey Bryant
2012-08-02 15:08       ` Corey Bryant
2012-07-24 12:09 ` [Qemu-devel] [PATCH v5 0/6] file descriptor passing using " Kevin Wolf
2012-07-25  3:42   ` Corey Bryant

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=50114333.7090209@redhat.com \
    --to=kwolf@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=coreyb@linux.vnet.ibm.com \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.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;
as well as URLs for NNTP newsgroup(s).