qemu-devel
[Top][All Lists]
Advanced

[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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]