[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext tim
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll |
Date: |
Tue, 17 Sep 2013 18:09:38 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2013-08-13 16:22, Stefan Hajnoczi wrote:
> On Tue, Aug 13, 2013 at 03:45:44PM +0200, Jan Kiszka wrote:
>> On 2013-08-13 15:39, Alex Bligh wrote:
>>> Jan,
>>>
>>> On 13 Aug 2013, at 14:25, Jan Kiszka wrote:
>>>
>>>> To my understanding, the use case behind the current behavior is
>>>> qemu_aio_wait() which is only supposed to block when there are pending
>>>> requests for the main aio context. We should be able to address this
>>>> scenarios also in a different way. I would definitely prefer to not
>>>> depend on that hack above.
>>>
>>> I don't *think* so. If I'm right the problem is line 233 of
>>> aio-posix.c (and similar in the windows variant):
>>>
>>> /* No AIO operations? Get us out of here */
>>> if (!busy) {
>>> return progress;
>>> }
>>>
>>> ... do qemu_poll_ns ...
>>>
>>> busy is set to true if there are any FDs for which ->flush
>>> is true and ->io_flush() returns non-zero.
>>
>> Right.
>>
>>>
>>> I think this should instead be looking the equivalent of
>>> FD_ISSET across all FDs (read and write) and the blocking flag.
>>> IE if blocking is set to true, then it should ALWAYS do
>>> qemu_poll_ns, lest it busyloop rather than wait for the
>>> next timer expiry.
>>
>> Yes, that would be needed.
>>
>>>
>>> More here:
>>> https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg03950.html
>>>
>>> I'm not very happy with this logic (but it's the same as before),
>>> and I note Stefan removes the horrible busy flag in this
>>> series:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg00092.html
>>
>> Yeah:
>>
>> - /* No AIO operations? Get us out of here */
>> - if (!busy) {
>> + /* early return if we only have the aio_notify() fd */
>> + if (ctx->pollfds->len == 1) {
>> return progress;
>> }
>>
>> So this is even worse for my use case.
>
> We can change the semantics of aio_poll() so long as we don't break
> existing callers and tests. It would make sense to do that after
> merging the io_flush and AioContext timers series.
Need to pick up this topic again because above change is now mainline
and breaks aio_poll-based timer threads:
How can we make progress with overcoming that check, at least for the
timer thread use case? Additional argument "truly_block" for aio_poll?
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll,
Jan Kiszka <=
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Paolo Bonzini, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Alex Bligh, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Jan Kiszka, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Alex Bligh, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Paolo Bonzini, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Alex Bligh, 2013/09/17
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Paolo Bonzini, 2013/09/18
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Alex Bligh, 2013/09/18
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Alex Bligh, 2013/09/18
- Re: [Qemu-devel] [RFC] [PATCHv10 00/31] aio / timers: Add AioContext timers and use ppoll, Paolo Bonzini, 2013/09/18