Re: [Qemu-devel] [PATCH for-2.1?!?] AioContext: speed up aio_notify

[Ming Lei]

On Fri, Jul 4, 2014 at 2:51 PM, Paolo Bonzini <address@hidden> wrote:
> Il 03/07/2014 20:17, Stefan Hajnoczi ha scritto:
>
>> On Thu, Jul 3, 2014 at 6:59 PM, Paolo Bonzini <address@hidden> wrote:
>>
>> Looks like a very interesting optimization for the block layer.  If we
>> can get some performance results - especially how it improves the
>> virtio-blk dataplane regression - maybe we should merge it for 2.1.
>>
>>> @@ -247,9 +248,22 @@ ThreadPool *aio_get_thread_pool(AioContext *ctx)
>>>      return ctx->thread_pool;
>>>  }
>>>
>>> +void aio_set_dispatching(AioContext *ctx, bool dispatching)
>>> +{
>>> +    ctx->dispatching = dispatching;
>>> +    /* Write ctx->dispatching before reading e.g. bh->scheduled.  */
>>> +    if (!dispatching) {
>>> +        smp_mb();
>>> +    }
>>
>>
>> Is the if statement necessary?  It seems like an optimization to avoid
>> the memory barrier in 50% of the calls.
>
>
> Yes.
>
>
>> But by avoiding the barrier it could cause other threads to "miss" the
>> dispatching period on machines with weak memory models.  I guess
>> that's fine since we're not trying to optimize other threads but more
>> focussing on callers from the event loop's own thread.
>>
>> Anyway, the if statement makes this function trickier than it has to
>> be IMO.  Want to drop it?
>
>
> I think this is the last "trickiness" problem of this patch, but of course I
> have no problem removing it if the patch goes into 2.1.  Ming Lei's latest
> message suggests it is not necessary though.

I think it is good and better to go to 2.1, and it should save lots of
write syscall.

Also should regression be caused, the per thread trick may be
resorted to, which should be simple.

With multi virtqueue's coming for virtio-blk, it should save more, and I
also plan to use the improved qemu bh to help merge requests from
multi queue, without introducing extra notifier.

Thanks,
--
Ming Lei