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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 03/07/2014 20:17, Stefan Hajnoczi ha scritto:
> On Thu, Jul 3, 2014 at 6:59 PM, Paolo Bonzini <pbonzini@redhat.com> 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.

The patch should be tested at least on rbd or gluster, these are the 
main users of remote-thread notification via bottom halves.

Paolo