[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC] queue_work proposal
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [RFC] queue_work proposal |
Date: |
Thu, 03 Sep 2009 16:43:27 +0300 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.1) Gecko/20090814 Fedora/3.0-2.6.b3.fc11 Thunderbird/3.0b3 |
On 09/03/2009 03:11 PM, Glauber Costa wrote:
On Thu, Sep 03, 2009 at 02:32:45PM +0300, Avi Kivity wrote:
On 09/03/2009 02:15 PM, Glauber Costa wrote:
on_vcpu() and queue_work() are fundamentally different (yes, I see the
wait parameter, and I think there should be two separate functions for
such different behaviours).
Therefore, the name change. The exact on_vcpu behaviour, however, can be
implemented ontop of queue_work().
Will there be any use for asynchronous queue_work()?
It's a dangerous API.
Initially, I thought we could use it for batching, if we forced a flush in the
end of
a sequence of operations. This can makes things faster if we are doing a bunch
of calls
in a row, from the wrong thread.
It's a lot easier and safer to write a function that does your batch job
and on_vcpu() it.
I think it's reasonable to demand that whoever calls kvm_vcpu_ioctl()
know what they are doing (and they'll get surprising results if it
switches threads implicitly).
I respectfully disagree. Not that I want people not to know what they are
doing, but I
believe that, forcing something that can only run in a specific thread to be
run there,
provides us with a much saner interface, that will make code a lot more
readable and
maintainable.
Example:
kvm_vcpu_ioctl(get_regs)
regs->rflags |= some_flag
kvm_vcpu_ioctl(set_regs)
This looks totally sane but is racy if kvm_vcpu_ioctl() does an implicit
on_vcpu().
One of them is synchronous, meaning the data can live on stack and no
special synchronization is needed, while the other is synchronous,
meaning explicit memory management and end-of-work synchronization is
needed.
I will assume you meant "the other is assynchronous". It does not need to be.
I though about including the assynchronous version in this RFC to let doors
open for performance improvements *if* we needed them. But again: the absolute
majority of the calls will be local. So it is not that important.
Then there's even less reason to include the async version.
on_vcpu() is a subset of queue_work(). I meant, why to we need the
extra functionality?
As I said, if you oppose it hardly, we don't really need to.
I do oppose it, but the reason for not including it should be the
reasons I cited, not that I oppose it.
A more powerful API comes with increased responsibilities.
You suddenly sounds like spider man.
I hate it when I get unmasked.
--
error compiling committee.c: too many arguments to function