All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <aliguori@us.ibm.com>
To: kvm-ia64@vger.kernel.org
Subject: Re: [patch] remove vcpu_info array v5
Date: Mon, 10 Nov 2008 15:22:53 +0000	[thread overview]
Message-ID: <491851CD.9030401@us.ibm.com> (raw)
In-Reply-To: <4909C00F.8050704@sgi.com>

Avi Kivity wrote:
>
>> My patch gets rid of the pointless MAX_CPUS sized array, which doesn't
>> buy us anything. In fact, most of the changes in my patch makes the
>> code simpler, because it removes a stack of silly cases where qemu uses
>> env->cpu_index to get into the array, just to hide CPUState from libkvm,
>> just to have the callback in QEMU go from int vcpu back to CPUState.
>>
>> Lets just do it right and get rid of this silliness.
>
> I agree that vcpu_info should die.  But we should still have a fast 
> way of getting a cpu from a cpu_index.  I don't see a problem with a 
> static array of pointers, make it 16K in size if you want.  Our real 
> scaling limits are much lower; they involve the big qemu lock and the 
> single iothread which will both limit scaling far below any static 
> vcpu array.

Just do a linear search of the CPUState list and be done with it.  This 
smells of premature optimization greatly.  I would be amazed if walking 
the CPUState list is ever on the fast path or will ever be.  Really, if 
you need to go from cpu_index => CPUState, it suggests you're doing 
something wrong.

Regards,

Anthony Liguori


WARNING: multiple messages have this Message-ID (diff)
From: Anthony Liguori <aliguori@us.ibm.com>
To: Avi Kivity <avi@redhat.com>
Cc: Jes Sorensen <jes@sgi.com>, Glauber Costa <glommer@gmail.com>,
	Hollis Blanchard <hollisb@us.ibm.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"kvm-ia64@vger.kernel.org" <kvm-ia64@vger.kernel.org>
Subject: Re: [patch] remove vcpu_info array v5
Date: Mon, 10 Nov 2008 09:22:53 -0600	[thread overview]
Message-ID: <491851CD.9030401@us.ibm.com> (raw)
In-Reply-To: <491839CF.9060105@redhat.com>

Avi Kivity wrote:
>
>> My patch gets rid of the pointless MAX_CPUS sized array, which doesn't
>> buy us anything. In fact, most of the changes in my patch makes the
>> code simpler, because it removes a stack of silly cases where qemu uses
>> env->cpu_index to get into the array, just to hide CPUState from libkvm,
>> just to have the callback in QEMU go from int vcpu back to CPUState.
>>
>> Lets just do it right and get rid of this silliness.
>
> I agree that vcpu_info should die.  But we should still have a fast 
> way of getting a cpu from a cpu_index.  I don't see a problem with a 
> static array of pointers, make it 16K in size if you want.  Our real 
> scaling limits are much lower; they involve the big qemu lock and the 
> single iothread which will both limit scaling far below any static 
> vcpu array.

Just do a linear search of the CPUState list and be done with it.  This 
smells of premature optimization greatly.  I would be amazed if walking 
the CPUState list is ever on the fast path or will ever be.  Really, if 
you need to go from cpu_index => CPUState, it suggests you're doing 
something wrong.

Regards,

Anthony Liguori


  parent reply	other threads:[~2008-11-10 15:22 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-30 14:09 [patch] remove vcpu_info array v5 Jes Sorensen
2008-10-30 14:09 ` Jes Sorensen
2008-11-04 11:54 ` Avi Kivity
2008-11-04 11:54   ` Avi Kivity
2008-11-04 13:55 ` Glauber Costa
2008-11-04 13:55   ` Glauber Costa
2008-11-04 14:30 ` Avi Kivity
2008-11-04 14:30   ` Avi Kivity
2008-11-04 14:35 ` Glauber Costa
2008-11-04 14:35   ` Glauber Costa
2008-11-04 14:40 ` Avi Kivity
2008-11-04 14:40   ` Avi Kivity
2008-11-04 14:43 ` Glauber Costa
2008-11-04 14:43   ` Glauber Costa
2008-11-04 14:47 ` Avi Kivity
2008-11-04 14:47   ` Avi Kivity
2008-11-10 12:30 ` Jes Sorensen
2008-11-10 12:30   ` Jes Sorensen
2008-11-10 13:24 ` Jes Sorensen
2008-11-10 13:24   ` Jes Sorensen
2008-11-10 13:30 ` Jes Sorensen
2008-11-10 13:30   ` Jes Sorensen
2008-11-10 13:40 ` Avi Kivity
2008-11-10 13:40   ` Avi Kivity
2008-11-10 13:48 ` Avi Kivity
2008-11-10 13:48   ` Avi Kivity
2008-11-10 15:22 ` Anthony Liguori [this message]
2008-11-10 15:22   ` Anthony Liguori
2008-11-10 15:45 ` Avi Kivity
2008-11-10 15:45   ` Avi Kivity
2008-11-10 15:47 ` Jes Sorensen
2008-11-10 15:47   ` Jes Sorensen
2008-11-10 15:54 ` Avi Kivity
2008-11-10 15:54   ` Avi Kivity
2008-11-10 15:58 ` Jes Sorensen
2008-11-10 15:58   ` Jes Sorensen
2008-11-11  9:26 ` Avi Kivity
2008-11-11  9:26   ` Avi Kivity
2008-11-12 13:02 ` Jes Sorensen
2008-11-12 13:02   ` Jes Sorensen
2008-11-12 13:09 ` Glauber Costa
2008-11-12 13:09   ` Glauber Costa
2008-11-12 13:12 ` Avi Kivity
2008-11-12 13:12   ` Avi Kivity
2008-11-12 13:14 ` Avi Kivity
2008-11-12 13:14   ` Avi Kivity
  -- strict thread matches above, loose matches on Subject: below --
2008-11-13 16:46 [patch] pass opague CPUState through libkvm instead of int vcpu Jes Sorensen
2008-11-13 16:46 ` Jes Sorensen
2008-11-18 12:55 ` Avi Kivity
2008-11-18 12:55   ` Avi Kivity
2008-11-18 12:59 ` Jes Sorensen
2008-11-18 12:59   ` Jes Sorensen

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=491851CD.9030401@us.ibm.com \
    --to=aliguori@us.ibm.com \
    --cc=kvm-ia64@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.