[v1,17/22] KVM: x86: simplify pic_unlock()
diff mbox

Message ID 20170314133450.13259-18-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand March 14, 2017, 1:34 p.m. UTC
We can easily compact this code and get rid of one local variable.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/i8259.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Peter Xu March 15, 2017, 8:27 a.m. UTC | #1
On Tue, Mar 14, 2017 at 02:34:45PM +0100, David Hildenbrand wrote:
> We can easily compact this code and get rid of one local variable.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/x86/kvm/i8259.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index e77e8c3..c50b839 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -49,7 +49,7 @@ static void pic_unlock(struct kvm_pic *s)
>  	__releases(&s->lock)
>  {
>  	bool wakeup = s->wakeup_needed;
> -	struct kvm_vcpu *vcpu, *found = NULL;
> +	struct kvm_vcpu *vcpu;
>  	int i;
>  
>  	s->wakeup_needed = false;
> @@ -59,16 +59,11 @@ static void pic_unlock(struct kvm_pic *s)
>  	if (wakeup) {
>  		kvm_for_each_vcpu(i, vcpu, s->kvm) {
>  			if (kvm_apic_accept_pic_intr(vcpu)) {
> -				found = vcpu;
> +				kvm_make_request(KVM_REQ_EVENT, vcpu);
> +				kvm_vcpu_kick(vcpu);
>  				break;

Maybe "return" directly here? Besides:

Reviewed-by: Peter Xu <peterx@redhat.com>

>  			}
>  		}
> -
> -		if (!found)
> -			return;
> -
> -		kvm_make_request(KVM_REQ_EVENT, found);
> -		kvm_vcpu_kick(found);
>  	}
>  }
>  
> -- 
> 2.9.3
> 

-- peterx
David Hildenbrand March 15, 2017, 9:07 a.m. UTC | #2
Am 15.03.2017 um 09:27 schrieb Peter Xu:
> On Tue, Mar 14, 2017 at 02:34:45PM +0100, David Hildenbrand wrote:
>> We can easily compact this code and get rid of one local variable.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/x86/kvm/i8259.c | 11 +++--------
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
>> index e77e8c3..c50b839 100644
>> --- a/arch/x86/kvm/i8259.c
>> +++ b/arch/x86/kvm/i8259.c
>> @@ -49,7 +49,7 @@ static void pic_unlock(struct kvm_pic *s)
>>  	__releases(&s->lock)
>>  {
>>  	bool wakeup = s->wakeup_needed;
>> -	struct kvm_vcpu *vcpu, *found = NULL;
>> +	struct kvm_vcpu *vcpu;
>>  	int i;
>>  
>>  	s->wakeup_needed = false;
>> @@ -59,16 +59,11 @@ static void pic_unlock(struct kvm_pic *s)
>>  	if (wakeup) {
>>  		kvm_for_each_vcpu(i, vcpu, s->kvm) {
>>  			if (kvm_apic_accept_pic_intr(vcpu)) {
>> -				found = vcpu;
>> +				kvm_make_request(KVM_REQ_EVENT, vcpu);
>> +				kvm_vcpu_kick(vcpu);
>>  				break;
> 
> Maybe "return" directly here? Besides:
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks, turned that break into return and added your rb!

Patch
diff mbox

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index e77e8c3..c50b839 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -49,7 +49,7 @@  static void pic_unlock(struct kvm_pic *s)
 	__releases(&s->lock)
 {
 	bool wakeup = s->wakeup_needed;
-	struct kvm_vcpu *vcpu, *found = NULL;
+	struct kvm_vcpu *vcpu;
 	int i;
 
 	s->wakeup_needed = false;
@@ -59,16 +59,11 @@  static void pic_unlock(struct kvm_pic *s)
 	if (wakeup) {
 		kvm_for_each_vcpu(i, vcpu, s->kvm) {
 			if (kvm_apic_accept_pic_intr(vcpu)) {
-				found = vcpu;
+				kvm_make_request(KVM_REQ_EVENT, vcpu);
+				kvm_vcpu_kick(vcpu);
 				break;
 			}
 		}
-
-		if (!found)
-			return;
-
-		kvm_make_request(KVM_REQ_EVENT, found);
-		kvm_vcpu_kick(found);
 	}
 }