From: Bart Van Assche <bvanassche@acm.org>
To: Asias He <asias@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>, kvm-devel <kvm@vger.kernel.org>
Subject: Re: [PATCH 1/2] vhost: Reduce vhost_work_flush() wakeup latency
Date: Thu, 15 Aug 2013 08:13:14 +0200 [thread overview]
Message-ID: <520C717A.7060007@acm.org> (raw)
In-Reply-To: <20130815013056.GA7304@hj.localdomain>
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.
next prev parent reply other threads:[~2013-08-15 6:13 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-14 7:01 [PATCH 0/2] vhost_worker fixes Bart Van Assche
2013-08-14 7:02 ` [PATCH 1/2] vhost: Reduce vhost_work_flush() wakeup latency Bart Van Assche
2013-08-14 11:37 ` Michael S. Tsirkin
2013-08-14 15:22 ` Bart Van Assche
2013-08-14 17:58 ` Michael S. Tsirkin
2013-08-15 1:30 ` Asias He
2013-08-15 6:13 ` Bart Van Assche [this message]
2013-08-15 7:35 ` Michael S. Tsirkin
2013-08-14 7:03 ` [PATCH 2/2] vhost: Avoid that vhost_work_flush() returns early Bart Van Assche
2013-08-14 11:46 ` Michael S. Tsirkin
2013-08-14 15:19 ` Bart Van Assche
2013-08-14 11:39 ` [PATCH 0/2] vhost_worker fixes Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=520C717A.7060007@acm.org \
--to=bvanassche@acm.org \
--cc=asias@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mst@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox