| Message ID | 1438792381-19453-6-git-send-email-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 05.08.2015 um 18:33 schrieb Radim Kr?má?: > I find the switch easier to read and modify. yes. > Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com> > --- > v2: new > > virt/kvm/kvm_main.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index d7ffe6090520..71598554deed 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2252,12 +2252,15 @@ static long kvm_vcpu_ioctl(struct file *filp, > * Special cases: vcpu ioctls that are asynchronous to vcpu execution, > * so vcpu_load() would break it. > */ > + switch (ioctl) { > #if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) > - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_S390_IRQ || ioctl == KVM_INTERRUPT) > - return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > + case KVM_S390_INTERRUPT: > + case KVM_S390_IRQ: > + case KVM_INTERRUPT: When you are it you might want to put the KVM_S390* withing CONFIG_S390 and KVM_INTERRUPT within CONFIG_PPC || CONFIG_MIPS This might speed up the switch statement for s390/ppc/mips a tiny bit. It will add another ifdef, though. Paolo? > #endif > - if (ioctl == KVM_USER_EXIT) > + case KVM_USER_EXIT: > return kvm_arch_vcpu_ioctl(filp, ioctl, arg); > + } > > r = vcpu_load(vcpu); > if (r) > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-08-12 22:03+0200, Christian Borntraeger: > Am 05.08.2015 um 18:33 schrieb Radim Kr?má?: >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> @@ -2252,12 +2252,15 @@ static long kvm_vcpu_ioctl(struct file *filp, >> * Special cases: vcpu ioctls that are asynchronous to vcpu execution, >> * so vcpu_load() would break it. >> */ >> + switch (ioctl) { >> #if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) >> - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_S390_IRQ || ioctl == KVM_INTERRUPT) >> - return kvm_arch_vcpu_ioctl(filp, ioctl, arg); >> + case KVM_S390_INTERRUPT: >> + case KVM_S390_IRQ: >> + case KVM_INTERRUPT: > > When you are it you might want to put the KVM_S390* withing CONFIG_S390 and > KVM_INTERRUPT within CONFIG_PPC || CONFIG_MIPS Sure, thanks. > This might speed up the switch statement for s390/ppc/mips a tiny bit. It will add > another ifdef, though. Paolo? For v3, I will name the decision as an inline function, which should make the #ifing more acceptable (at the cost of not having ioctls #defs in the body of kvm_vcpu_ioctl). Something like this, static inline bool kvm_asynchronous_ioctl(unsigned ioctl) { switch (ioctl) { #if defined(CONFIG_S390) case KVM_S390_INTERRUPT: case KVM_S390_IRQ: #endif #if defined(CONFIG_MIPS) case KVM_INTERRUPT: #endif case KVM_USER_EXIT: return true; } return false; } [...] if (kvm_asynchronous_ioctl(ioctl)) return kvm_arch_vcpu_ioctl(filp, ioctl, arg); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/08/2015 22:03, Christian Borntraeger wrote: >> > #if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) >> > - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_S390_IRQ || ioctl == KVM_INTERRUPT) >> > - return kvm_arch_vcpu_ioctl(filp, ioctl, arg); >> > + case KVM_S390_INTERRUPT: >> > + case KVM_S390_IRQ: >> > + case KVM_INTERRUPT: > When you are it you might want to put the KVM_S390* withing CONFIG_S390 and > KVM_INTERRUPT within CONFIG_PPC || CONFIG_MIPS > > This might speed up the switch statement for s390/ppc/mips a tiny bit. It will add > another ifdef, though. Paolo? Sure. I wasn't sure of KVM_INTERRUPT's usage on s390. I'm okay with keeping the switch inline too, but if Radim prefers a function that's also fine. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d7ffe6090520..71598554deed 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2252,12 +2252,15 @@ static long kvm_vcpu_ioctl(struct file *filp, * Special cases: vcpu ioctls that are asynchronous to vcpu execution, * so vcpu_load() would break it. */ + switch (ioctl) { #if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_MIPS) - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_S390_IRQ || ioctl == KVM_INTERRUPT) - return kvm_arch_vcpu_ioctl(filp, ioctl, arg); + case KVM_S390_INTERRUPT: + case KVM_S390_IRQ: + case KVM_INTERRUPT: #endif - if (ioctl == KVM_USER_EXIT) + case KVM_USER_EXIT: return kvm_arch_vcpu_ioctl(filp, ioctl, arg); + } r = vcpu_load(vcpu); if (r)
I find the switch easier to read and modify. Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com> --- v2: new virt/kvm/kvm_main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)