Re: [PATCH 1/2] vhost: Reduce vhost_work_flush() wakeup latency

Bart Van Assche
2013-08-15T06:13:16Z

On 08/15/13 03:30, Asias He wrote:
> On Wed, Aug 14, 2013 at 08:58:25PM +0300, Michael S. Tsirkin wrote:
>> On Wed, Aug 14, 2013 at 05:22:36PM +0200, Bart Van Assche wrote:
>>> On 08/14/13 13:37, Michael S. Tsirkin wrote:
>>>> On Wed, Aug 14, 2013 at 09:02:32AM +0200, Bart Van Assche wrote:
>>>>> If the TIF_NEED_RESCHED task flag is set, wake up any vhost_work_flush()
>>>>> waiters before rescheduling instead of after rescheduling.
>>>>>
>>>>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>>>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>>>> Cc: Asias He <asias@redhat.com>
>>>>
>>>> Why exactly? It's not like flush needs to be extra fast ...
>>>
>>> I'm not worried about how fast a flush is processed either. But I
>>> think this patch is a nice cleanup of the vhost_worker() function.
>>> It eliminates the uninitialized_var() construct and moves the
>>> assignment of "done_seq" below the read of "queue_seq".
>>>
>>> Bart.
>>
>> I'm not worried about uninitialized_var - it's just a compiler bug.
>> done_seq below read is nice, but this is at the cost of sprinkling
>> lock/unlock, lock/unlock all over the code. Maybe we can come up with
>> something that doesn't have this property.
>
> The extra locking introduced here does not look good to me neither.

Please note that although there are additional locking statements in the
code, there are no additional locking calls at runtime.

Bart.