From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] KVM:PPC Issue in exit timing clearance Date: Tue, 22 Mar 2011 15:27:07 +0100 Message-ID: <4D88B1BB.8070306@suse.de> References: <1300293459-32767-1-git-send-email-Bharat.Bhushan@freescale.com> <4D88998B.6050208@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Bharat Bhushan , kvm@vger.kernel.org, bharatb.yadav@gmail.com, Bharat Bhushan To: Avi Kivity Return-path: Received: from cantor.suse.de ([195.135.220.2]:41592 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752928Ab1CVO1J (ORCPT ); Tue, 22 Mar 2011 10:27:09 -0400 In-Reply-To: <4D88998B.6050208@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 03/22/2011 01:43 PM, Avi Kivity wrote: > On 03/16/2011 06:37 PM, Bharat Bhushan wrote: >> Following dump is observed on host when clearing the exit timing >> counters >> >> [root@p1021mds kvm]# echo -n 'c'> vm1200_vcpu0_timing >> INFO: task echo:1276 blocked for more than 120 seconds. >> "echo 0> /proc/sys/kernel/hung_task_timeout_secs" disables this >> message. >> echo D 0ff5bf94 0 1276 1190 0x00000000 >> Call Trace: >> [c2157e40] [c0007908] __switch_to+0x9c/0xc4 >> [c2157e50] [c040293c] schedule+0x1b4/0x3bc >> [c2157e90] [c04032dc] __mutex_lock_slowpath+0x74/0xc0 >> [c2157ec0] [c00369e4] kvmppc_init_timing_stats+0x20/0xb8 >> [c2157ed0] [c0036b00] kvmppc_exit_timing_write+0x84/0x98 >> [c2157ef0] [c00b9f90] vfs_write+0xc0/0x16c >> [c2157f10] [c00ba284] sys_write+0x4c/0x90 >> [c2157f40] [c000e320] ret_from_syscall+0x0/0x3c >> >> The vcpu->mutex is used by kvm_ioctl_* (KVM_RUN etc) and same was >> used when clearing the stats (in kvmppc_init_timing_stats()). >> What happens is that when the guest is idle then it held the >> vcpu->mutx. While the exiting timing process waits for guest to >> release the vcpu->mutex and a hang state is reached. >> >> Now using seprate lock for exit timing stats. >> > > Seems excessive to have a new lock just for timing. The whole thing is only used for debugging, so being excessive doesn't hurt too much here. In normal configurations, it's #ifdef'ed out. > > What about using vcpu->requests to have the statistics cleared in vcpu > context? > > What about dropping the whole thing and replacing it with tracepoints? That should work. The question is if it's worth the effort. The current code is there after all :). Alex