[PATCH v2] KVM: x86: add paging gcc optimization

[PATCH v2] KVM: x86: add paging gcc optimization

[Davidlohr Bueso]

From: Davidlohr Bueso <dave@gnu.org>

Since most guests will have paging enabled for memory management, add likely() optimization
around CR0.PG checks.

Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
 arch/x86/kvm/x86.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index cb80c29..3d1134d 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -64,7 +64,7 @@ static inline int is_pse(struct kvm_vcpu *vcpu)
 
 static inline int is_paging(struct kvm_vcpu *vcpu)
 {
-	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
+	return likely(kvm_read_cr0_bits(vcpu, X86_CR0_PG));
 }
 
 static inline u32 bit(int bitno)
-- 
1.7.8.3

Re: [PATCH v2] KVM: x86: add paging gcc optimization

[Christian Borntraeger]

On 08/03/12 12:45, Davidlohr Bueso wrote:
> From: Davidlohr Bueso <dave@gnu.org>
> 
> Since most guests will have paging enabled for memory management, add likely() optimization
> around CR0.PG checks.

>  {
> -	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
> +	return likely(kvm_read_cr0_bits(vcpu, X86_CR0_PG));


IMHO likely/unlikely should be considered more as fast-path/slow-path and not as often/less often.
Is that the case here? This patch  might cause a mis-prediction for non-paging guests all 
the time. 

Non-paging might be really irrelevant, so I am just making a point, since
likely/unlikely is mis-used too often especially for "most users do it that way".


Christian

Re: [PATCH v2] KVM: x86: add paging gcc optimization

[Davidlohr Bueso]

On Thu, 2012-03-08 at 16:47 +0100, Christian Borntraeger wrote:
> On 08/03/12 12:45, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso <dave@gnu.org>
> > 
> > Since most guests will have paging enabled for memory management, add likely() optimization
> > around CR0.PG checks.
> 
> >  {
> > -	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
> > +	return likely(kvm_read_cr0_bits(vcpu, X86_CR0_PG));
> 
> 
> IMHO likely/unlikely should be considered more as fast-path/slow-path and not as often/less often.
> Is that the case here? This patch  might cause a mis-prediction for non-paging guests all 
> the time. 

Branch predictions are all about probability of occurrence, and I cannot
imagine guests having paging disabled and mapping gva->gpa directly, so
for most of the cases it would be beneficial. For those peculiar users
that actually don't use paging, then yes, mispredictions would occur.

- Davidlohr

> 
> Non-paging might be really irrelevant, so I am just making a point, since
> likely/unlikely is mis-used too often especially for "most users do it that way".
> 
> 
> Christian
> 
> 

Re: [PATCH v2] KVM: x86: add paging gcc optimization

[Avi Kivity]

On 03/08/2012 05:47 PM, Christian Borntraeger wrote:
> On 08/03/12 12:45, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso <dave@gnu.org>
> > 
> > Since most guests will have paging enabled for memory management, add likely() optimization
> > around CR0.PG checks.
>
> >  {
> > -	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
> > +	return likely(kvm_read_cr0_bits(vcpu, X86_CR0_PG));
>
>
> IMHO likely/unlikely should be considered more as fast-path/slow-path and not as often/less often.

Agree.

> Is that the case here? This patch  might cause a mis-prediction for non-paging guests all 
> the time. 
>
> Non-paging might be really irrelevant, so I am just making a point, since
> likely/unlikely is mis-used too often especially for "most users do it that way".

In fact this is a classic example.  Almost no guests use real mode (the
last guests to use real mode extensively was DOS; I think Win9x switches
to real mode pretty often).  As it's a user-controlled setting, we're
penalizing users who do things differently.

However the majority if is_paging() == true guests is so huge, and since
non-paging guests don't really expect 2012 performance levels anyway
(being so old) that I think in practice this is a good optimization here.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH v2] KVM: x86: add paging gcc optimization

[Davidlohr Bueso]

On Mon, 2012-03-12 at 14:12 +0200, Avi Kivity wrote:
> On 03/08/2012 05:47 PM, Christian Borntraeger wrote:
> > On 08/03/12 12:45, Davidlohr Bueso wrote:
> > > From: Davidlohr Bueso <dave@gnu.org>
> > > 
> > > Since most guests will have paging enabled for memory management, add likely() optimization
> > > around CR0.PG checks.
> >
> > >  {
> > > -	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
> > > +	return likely(kvm_read_cr0_bits(vcpu, X86_CR0_PG));
> >
> >
> > IMHO likely/unlikely should be considered more as fast-path/slow-path and not as often/less often.
> 
> Agree.
> 
> > Is that the case here? This patch  might cause a mis-prediction for non-paging guests all 
> > the time. 
> >
> > Non-paging might be really irrelevant, so I am just making a point, since
> > likely/unlikely is mis-used too often especially for "most users do it that way".
> 
> In fact this is a classic example.  Almost no guests use real mode (the
> last guests to use real mode extensively was DOS; I think Win9x switches
> to real mode pretty often).  As it's a user-controlled setting, we're
> penalizing users who do things differently.
> 
> However the majority if is_paging() == true guests is so huge, and since
> non-paging guests don't really expect 2012 performance levels anyway
> (being so old) that I think in practice this is a good optimization here.

Avi, will you be taking this patch? I don't see it applied or for pull
in 3.4.

Thanks.

Re: [PATCH v2] KVM: x86: add paging gcc optimization

[Avi Kivity]

On 03/23/2012 11:51 PM, Davidlohr Bueso wrote:
> Avi, will you be taking this patch? I don't see it applied or for pull
> in 3.4.
>
>

Applied now, thanks for the reminder.

-- 
error compiling committee.c: too many arguments to function