Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 07/26/2012 09:16 AM, Kevin Wolf wrote:
> 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
>

I'll probably hold off on any optimisation at this point, but I can 
revisit it in the future if it's needed.

-- 
Regards,
Corey