Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-23 Thread Gleb Natapov
On Mon, Sep 22, 2014 at 09:29:19PM +0200, Paolo Bonzini wrote: Il 22/09/2014 21:20, Christian Borntraeger ha scritto: while using trinity to fuzz KVM, we noticed long stalls on invalid ioctls. Lets bail out early on invalid ioctls. or similar? Okay. David, can you explain how you found

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-23 Thread Christian Borntraeger
On 09/23/2014 08:49 AM, Gleb Natapov wrote: On Mon, Sep 22, 2014 at 09:29:19PM +0200, Paolo Bonzini wrote: Il 22/09/2014 21:20, Christian Borntraeger ha scritto: while using trinity to fuzz KVM, we noticed long stalls on invalid ioctls. Lets bail out early on invalid ioctls. or similar?

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-23 Thread Paolo Bonzini
Il 23/09/2014 10:06, Christian Borntraeger ha scritto: Yes. Davids explanation also makes sense as a commit message. Paolo, if you use David patch with a better description of the why I am fine with this patch. Done, thanks everybody! Paolo -- To unsubscribe from this list: send the line

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Paolo Bonzini
Il 20/09/2014 01:03, David Matlack ha scritto: vcpu ioctls can hang the calling thread if issued while a vcpu is running. If we know ioctl is going to be rejected as invalid anyway, we can fail before trying to take the vcpu mutex. This patch does not change functionality, it just makes

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Christian Borntraeger
On 09/22/2014 12:50 PM, Paolo Bonzini wrote: Il 20/09/2014 01:03, David Matlack ha scritto: vcpu ioctls can hang the calling thread if issued while a vcpu is running. If we know ioctl is going to be rejected as invalid anyway, we can fail before trying to take the vcpu mutex. This patch does

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Paolo Bonzini
Il 22/09/2014 15:45, Christian Borntraeger ha scritto: We now have an extra condition check for every valid ioctl, to make an error case go faster. I know, the extra check is just a 1 or 2 cycles if branch prediction is right, but still. I applied the patch because the delay could be

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread David Matlack
On 09/22, Paolo Bonzini wrote: Il 22/09/2014 15:45, Christian Borntraeger ha scritto: We now have an extra condition check for every valid ioctl, to make an error case go faster. I know, the extra check is just a 1 or 2 cycles if branch prediction is right, but still. I applied the

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Christian Borntraeger
On 09/22/2014 04:31 PM, Paolo Bonzini wrote: Il 22/09/2014 15:45, Christian Borntraeger ha scritto: We now have an extra condition check for every valid ioctl, to make an error case go faster. I know, the extra check is just a 1 or 2 cycles if branch prediction is right, but still. I

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Paolo Bonzini
Il 22/09/2014 21:20, Christian Borntraeger ha scritto: while using trinity to fuzz KVM, we noticed long stalls on invalid ioctls. Lets bail out early on invalid ioctls. or similar? Okay. David, can you explain how you found it so that I can make up my mind? Gleb and Marcelo, a fourth and

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread David Matlack
On 09/22, Christian Borntraeger wrote: On 09/22/2014 04:31 PM, Paolo Bonzini wrote: Il 22/09/2014 15:45, Christian Borntraeger ha scritto: We now have an extra condition check for every valid ioctl, to make an error case go faster. I know, the extra check is just a 1 or 2 cycles if

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Marcelo Tosatti
On Fri, Sep 19, 2014 at 04:03:25PM -0700, David Matlack wrote: vcpu ioctls can hang the calling thread if issued while a vcpu is running. There is a mutex per-vcpu, so thats expected, OK... If we know ioctl is going to be rejected as invalid anyway, we can fail before trying to take the

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Paolo Bonzini
Il 22/09/2014 22:08, Marcelo Tosatti ha scritto: This patch does not change functionality, it just makes invalid ioctls fail faster. Should not be executing vcpu ioctls without interrupt KVM_RUN in the first place. This is not entirely true, there are a couple of asynchronous ioctls

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread David Matlack
On 09/22, Marcelo Tosatti wrote: On Fri, Sep 19, 2014 at 04:03:25PM -0700, David Matlack wrote: vcpu ioctls can hang the calling thread if issued while a vcpu is running. There is a mutex per-vcpu, so thats expected, OK... If we know ioctl is going to be rejected as invalid anyway,

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Marcelo Tosatti
On Mon, Sep 22, 2014 at 11:29:16PM +0200, Paolo Bonzini wrote: Il 22/09/2014 22:08, Marcelo Tosatti ha scritto: This patch does not change functionality, it just makes invalid ioctls fail faster. Should not be executing vcpu ioctls without interrupt KVM_RUN in the first place.

Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-22 Thread Marcelo Tosatti
On Mon, Sep 22, 2014 at 03:58:16PM -0700, David Matlack wrote: Should not be executing vcpu ioctls without interrupt KVM_RUN in the first place. This patch is trying to be nice to code that isn't aware it's probing kvm file descriptors. We saw long hangs with some generic process

[PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls

2014-09-19 Thread David Matlack
vcpu ioctls can hang the calling thread if issued while a vcpu is running. If we know ioctl is going to be rejected as invalid anyway, we can fail before trying to take the vcpu mutex. This patch does not change functionality, it just makes invalid ioctls fail faster. Signed-off-by: David