arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', d

PATCH In review Fengguang Wu 4 participants 6 messages Sep 20, 2012 – Sep 27, 2012

Subsystems (4)

KVM Kernel janitors s390 KVM s390

lore

Share & tools

lore

Fengguang Wu

author

2012-09-20 00:16 UTC #0 lore raw

PATCH 41/42 arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', d

Hi Michael,

FYI, there are new compile warnings show up in

tree:   git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
head:   879238fecc051d95037ae76332916209a7770709
commit: 9fc77441e5e1bf80b794cc546d2243ee9f4afb75 [41/42] KVM: make processes waiting on vcpu mutex killable
config: s390-defconfig

All error/warnings:

arch/s390/kvm/interrupt.c: In function 'kvm_s390_handle_wait':
arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result [-Wunused-result]

vim +428 arch/s390/kvm/interrupt.c
418 add_wait_queue(&vcpu->arch.local_int.wq, &wait);
419 while (list_empty(&vcpu->arch.local_int.list) &&
420 list_empty(&vcpu->arch.local_int.float_int->list) &&
421 (!vcpu->arch.local_int.timer_due) &&
422 !signal_pending(current)) {
423 set_current_state(TASK_INTERRUPTIBLE);
424 spin_unlock_bh(&vcpu->arch.local_int.lock);
425 spin_unlock(&vcpu->arch.local_int.float_int->lock);
426 vcpu_put(vcpu);
427 schedule();

Quoted 1 line

428 vcpu_load(vcpu);

429 spin_lock(&vcpu->arch.local_int.float_int->lock);
430 spin_lock_bh(&vcpu->arch.local_int.lock);
431 }
432 __unset_cpu_idle(vcpu);
433 __set_current_state(TASK_RUNNING);
434 remove_wait_queue(&vcpu->arch.local_int.wq, &wait);
435 spin_unlock_bh(&vcpu->arch.local_int.lock);
436 spin_unlock(&vcpu->arch.local_int.float_int->lock);
437 hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
438 return 0;

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu, Yuanhan Liu Intel Corporation

Michael S. Tsirkin

2012-09-20 05:20 UTC #1 lore raw

Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load

Quoted 26 lines · Fengguang Wu

Hi Michael,
FYI, there are new compile warnings show up in
tree:   git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
head:   879238fecc051d95037ae76332916209a7770709
commit: 9fc77441e5e1bf80b794cc546d2243ee9f4afb75 [41/42] KVM: make processes waiting on vcpu mutex killable
config: s390-defconfig
All error/warnings:
arch/s390/kvm/interrupt.c: In function 'kvm_s390_handle_wait':
arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result [-Wunused-result]
vim +428 arch/s390/kvm/interrupt.c
   418		add_wait_queue(&vcpu->arch.local_int.wq, &wait);
   419		while (list_empty(&vcpu->arch.local_int.list) &&
   420			list_empty(&vcpu->arch.local_int.float_int->list) &&
   421			(!vcpu->arch.local_int.timer_due) &&
   422			!signal_pending(current)) {
   423			set_current_state(TASK_INTERRUPTIBLE);
   424			spin_unlock_bh(&vcpu->arch.local_int.lock);
   425			spin_unlock(&vcpu->arch.local_int.float_int->lock);
   426			vcpu_put(vcpu);
   427			schedule();
Quoted 1 line
428 vcpu_load(vcpu);

Quoted 10 lines

   429			spin_lock(&vcpu->arch.local_int.float_int->lock);
   430			spin_lock_bh(&vcpu->arch.local_int.lock);
   431		}
   432		__unset_cpu_idle(vcpu);
   433		__set_current_state(TASK_RUNNING);
   434		remove_wait_queue(&vcpu->arch.local_int.wq, &wait);
   435		spin_unlock_bh(&vcpu->arch.local_int.lock);
   436		spin_unlock(&vcpu->arch.local_int.float_int->lock);
   437		hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
   438		return 0;

Thanks for the report. This is because vcpu_load can now fail if task is being killed. I'm guessing the following is the right fix but have no way to tell. Warning: completely untested.

--->

kvm/arm: handle vcpu_load failure gracefully

Signed-off-by Michael S. Tsirkin <[email protected]>

arch/s390/kvm/interrupt.c +5 -1

index b7bc1aa..5aedf25 100644
@@ -362,6 +362,7 @@ int kvm_s390_handle_wait(struct kvm_vcpu *vcpu)
 {
 	u64 now, sltime;
 	DECLARE_WAITQUEUE(wait, current);
+	int ret;
 
 	vcpu->stat.exit_wait_state++;
 	if (kvm_cpu_has_interrupt(vcpu))

@@ -407,7 +408,9 @@ no_timer:
 		spin_unlock(&vcpu->arch.local_int.float_int->lock);
 		vcpu_put(vcpu);
 		schedule();
-		vcpu_load(vcpu);
+		ret = vcpu_load(vcpu);
+		if (ret)
+			goto cancel;
 		spin_lock(&vcpu->arch.local_int.float_int->lock);
 		spin_lock_bh(&vcpu->arch.local_int.lock);
 	}

@@ -416,6 +419,7 @@ no_timer:
 	remove_wait_queue(&vcpu->arch.local_int.wq, &wait);
 	spin_unlock_bh(&vcpu->arch.local_int.lock);
 	spin_unlock(&vcpu->arch.local_int.float_int->lock);
+cancel:
 	hrtimer_try_to_cancel(&vcpu->arch.ckc_timer);
 	return 0;
 }

Quoted 3 lines

---
0-DAY kernel build testing backend Open Source Technology Centre
Fengguang Wu, Yuanhan Liu Intel Corporation

Christian Borntraeger

2012-09-27 15:02 UTC #2 lore raw

Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load

Quoted 15 lines · Michael S. Tsirkin

Quoted 14 lines · Fengguang Wu
Hi Michael,
FYI, there are new compile warnings show up in
tree:   git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
head:   879238fecc051d95037ae76332916209a7770709
commit: 9fc77441e5e1bf80b794cc546d2243ee9f4afb75 [41/42] KVM: make processes waiting on vcpu mutex killable
config: s390-defconfig
All error/warnings:
arch/s390/kvm/interrupt.c: In function 'kvm_s390_handle_wait':
arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result [-Wunused-result]

Quoted 14 lines

Quoted 11 lines

vim +428 arch/s390/kvm/interrupt.c

   418		add_wait_queue(&vcpu->arch.local_int.wq, &wait);
   419		while (list_empty(&vcpu->arch.local_int.list) &&
   420			list_empty(&vcpu->arch.local_int.float_int->list) &&
   421			(!vcpu->arch.local_int.timer_due) &&
   422			!signal_pending(current)) {
   423			set_current_state(TASK_INTERRUPTIBLE);
   424			spin_unlock_bh(&vcpu->arch.local_int.lock);
   425			spin_unlock(&vcpu->arch.local_int.float_int->lock);
   426			vcpu_put(vcpu);
   427			schedule();

Quoted 3 lines

Quoted 1 line

428 vcpu_load(vcpu);

   429			spin_lock(&vcpu->arch.local_int.float_int->lock);
   430			spin_lock_bh(&vcpu->arch.local_int.lock);

IIRC schedule will do vcpu put/load anyway via the preempt notifiers. So the right fix is probably to just remove the vcpu_put/load around that schedule.
Will double check and send a patch.

Avi Kivity

2012-09-27 15:12 UTC #3 lore raw

Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load

Quoted 31 lines · Christian Borntraeger

Quoted 15 lines · Michael S. Tsirkin
Quoted 14 lines · Fengguang Wu
Hi Michael,
FYI, there are new compile warnings show up in
tree:   git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
head:   879238fecc051d95037ae76332916209a7770709
commit: 9fc77441e5e1bf80b794cc546d2243ee9f4afb75 [41/42] KVM: make processes waiting on vcpu mutex killable
config: s390-defconfig
All error/warnings:
arch/s390/kvm/interrupt.c: In function 'kvm_s390_handle_wait':
arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result [-Wunused-result]
Quoted 14 lines
Quoted 11 lines
vim +428 arch/s390/kvm/interrupt.c
   418		add_wait_queue(&vcpu->arch.local_int.wq, &wait);
   419		while (list_empty(&vcpu->arch.local_int.list) &&
   420			list_empty(&vcpu->arch.local_int.float_int->list) &&
   421			(!vcpu->arch.local_int.timer_due) &&
   422			!signal_pending(current)) {
   423			set_current_state(TASK_INTERRUPTIBLE);
   424			spin_unlock_bh(&vcpu->arch.local_int.lock);
   425			spin_unlock(&vcpu->arch.local_int.float_int->lock);
   426			vcpu_put(vcpu);
   427			schedule();
Quoted 3 lines
Quoted 1 line
428 vcpu_load(vcpu);
   429			spin_lock(&vcpu->arch.local_int.float_int->lock);
   430			spin_lock_bh(&vcpu->arch.local_int.lock);

Quoted 3 lines

IIRC schedule will do vcpu put/load anyway via the preempt notifiers. So the right fix is probably to just remove the vcpu_put/load around that schedule.
Will double check and send a patch.

Yes, this is correct. On a preemptible kernel this happens all the time.

Signature · 1 line

error compiling committee.c: too many arguments to function

Christian Borntraeger

2012-09-27 15:29 UTC #4 lore raw

PATCH 1/1 s390/kvm: Fix vcpu_load handling in interrupt code

Recent changes (KVM: make processes waiting on vcpu mutex killable) now requires to check the return value of vcpu_load. This triggered a warning in s390 specific kvm code. Turns out that we can actually remove the put/load, since schedule will do the right thing via the preempt notifiers.

Reported-by Fengguang Wu <[email protected]>

Signed-off-by Christian Borntraeger <[email protected]>

---
 arch/s390/kvm/interrupt.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

arch/s390/kvm/interrupt.c +0 -2

index 7556231..ff1e2f8 100644
@@ -423,9 +423,7 @@ no_timer:
 		set_current_state(TASK_INTERRUPTIBLE);
 		spin_unlock_bh(&vcpu->arch.local_int.lock);
 		spin_unlock(&vcpu->arch.local_int.float_int->lock);
-		vcpu_put(vcpu);
 		schedule();
-		vcpu_load(vcpu);
 		spin_lock(&vcpu->arch.local_int.float_int->lock);
 		spin_lock_bh(&vcpu->arch.local_int.lock);
 	}

Signature · 1 line

1.7.0.1

Avi Kivity

Applied

2012-09-27 16:20 UTC #5 lore raw

Re: [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code

Quoted 5 lines · Christian Borntraeger

Recent changes (KVM: make processes waiting on vcpu mutex killable) now requires to check the return value of vcpu_load. This triggered a warning in s390 specific kvm code. Turns out that we can actually remove the put/load, since schedule will do the right thing via the preempt notifiers.

Thanks, applied.

Signature · 1 line

error compiling committee.c: too many arguments to function