* [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result
@ 2012-09-20 0:16 Fengguang Wu
2012-09-20 5:22 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Fengguang Wu @ 2012-09-20 0:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Yuanhan Liu, kernel-janitors, Marcelo Tosatti, kvm
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();
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result
2012-09-20 0:16 [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result Fengguang Wu
@ 2012-09-20 5:22 ` Michael S. Tsirkin
2012-09-27 15:02 ` Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2012-09-20 5:22 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Yuanhan Liu, kernel-janitors, Marcelo Tosatti, kvm
On Thu, Sep 20, 2012 at 08:16:08AM +0800, Fengguang Wu wrote:
> 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();
> > 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;
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 <mst@redhat.com>
--
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index b7bc1aa..5aedf25 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -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;
}
> ---
> 0-DAY kernel build testing backend Open Source Technology Centre
> Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result
2012-09-20 5:22 ` Michael S. Tsirkin
@ 2012-09-27 15:02 ` Christian Borntraeger
2012-09-27 15:12 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2012-09-27 15:02 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Fengguang Wu, Yuanhan Liu, kernel-janitors, Marcelo Tosatti, kvm,
Avi Kivity, Heiko Carstens
On 20/09/12 07:22, Michael S. Tsirkin wrote:
> On Thu, Sep 20, 2012 at 08:16:08AM +0800, Fengguang Wu wrote:
>> 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();
>> > 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result
2012-09-27 15:02 ` Christian Borntraeger
@ 2012-09-27 15:12 ` Avi Kivity
2012-09-27 15:29 ` [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2012-09-27 15:12 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Michael S. Tsirkin, Fengguang Wu, Yuanhan Liu, kernel-janitors,
Marcelo Tosatti, kvm, Heiko Carstens
On 09/27/2012 05:02 PM, Christian Borntraeger wrote:
> On 20/09/12 07:22, Michael S. Tsirkin wrote:
>> On Thu, Sep 20, 2012 at 08:16:08AM +0800, Fengguang Wu wrote:
>>> 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();
>>> > 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.
Yes, this is correct. On a preemptible kernel this happens all the time.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code
2012-09-27 15:12 ` Avi Kivity
@ 2012-09-27 15:29 ` Christian Borntraeger
2012-09-27 16:20 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2012-09-27 15:29 UTC (permalink / raw)
To: Marcelo Tosatti, Avi Kivity
Cc: Michael S. Tsirkin, Fengguang Wu, Yuanhan Liu, kernel-janitors,
kvm, Heiko Carstens, 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.
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
arch/s390/kvm/interrupt.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 7556231..ff1e2f8 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -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);
}
--
1.7.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code
2012-09-27 15:29 ` [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code Christian Borntraeger
@ 2012-09-27 16:20 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2012-09-27 16:20 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Marcelo Tosatti, Michael S. Tsirkin, Fengguang Wu, Yuanhan Liu,
kernel-janitors, kvm, Heiko Carstens
On 09/27/2012 05:29 PM, Christian Borntraeger wrote:
> 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.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-27 16:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-20 0:16 [kvm:queue 41/42] arch/s390/kvm/interrupt.c:428:12: warning: ignoring return value of 'vcpu_load', declared with attribute warn_unused_result Fengguang Wu
2012-09-20 5:22 ` Michael S. Tsirkin
2012-09-27 15:02 ` Christian Borntraeger
2012-09-27 15:12 ` Avi Kivity
2012-09-27 15:29 ` [PATCH] s390/kvm: Fix vcpu_load handling in interrupt code Christian Borntraeger
2012-09-27 16:20 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).