Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755233AbbGCLYk (ORCPT <rfc822;w@1wt.eu>);
	Fri, 3 Jul 2015 07:24:40 -0400
Received: from mx1.redhat.com ([209.132.183.28]:38975 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754691AbbGCLYX (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 3 Jul 2015 07:24:23 -0400
Subject: Re: [[PATCH 2/2] kvm: enable preemption to register/unregister
 preempt notifier
To: Tiejun Chen <tiejun.chen@intel.com>, kvm@vger.kernel.org
References: <1435913788-4300-1-git-send-email-tiejun.chen@intel.com>
 <1435913788-4300-2-git-send-email-tiejun.chen@intel.com>
Cc: linux-kernel@vger.kernel.org, Gleb Natapov <gleb@kernel.org>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <559670B3.2000705@redhat.com>
Date: Fri, 3 Jul 2015 13:23:31 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.0.1
MIME-Version: 1.0
In-Reply-To: <1435913788-4300-2-git-send-email-tiejun.chen@intel.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3671
Lines: 85

On 03/07/2015 10:56, Tiejun Chen wrote:
> After commit 1cde2930e154 ("sched/preempt: Add static_key() to
> preempt_notifiers") is introduced, preempt_notifier_{register, unregister}
> always hold a mutex, jump_label_mutex. So in current case this shouldn't
> work further under the circumstance of disabled preemption, and its also
> safe since we're just handling a per-vcpu stuff with holding vcpu->mutex.
> Otherwise, some warning messages are posted like this,
> 
> BUG: scheduling while atomic: qemu-system-x86/17177/0x00000002
> 2 locks held by qemu-system-x86/17177:
>  #0:  (&vcpu->mutex){+.+.+.}, at: [<ffffffffa035fb48>] vcpu_load+0x28/0xf0 [kvm]
>  #1:  (jump_label_mutex){+.+.+.}, at: [<ffffffff81244b54>] static_key_slow_inc+0xc4/0x140
> Modules linked in: x86_pkg_temp_thermal kvm_intel kvm
> Preemption disabled at:[<ffffffffa035fd3e>] kvm_vcpu_ioctl+0x7e/0xeb0 [kvm]

Thanks for your work Tiejun.  However, the original patch is crap.  I've
asked to revert it.

Paolo

> CPU: 2 PID: 17177 Comm: qemu-system-x86 Tainted: G        W       4.1.0+ #30
> Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A05 12/05/2013
>  0000000000200206 ffff8801c584bc38 ffffffff81f974ab 0000000000000003
>  ffff880211289a80 ffff8801c584bc58 ffffffff81f8fd3e 0000000000000001
>  ffff8802161d5d00 ffff8801c584bcb8 ffffffff81fa43dc ffff8801c584bd68
> Call Trace:
>  [<ffffffff81f974ab>] dump_stack+0x95/0xf2
>  [<ffffffff81f8fd3e>] __schedule_bug+0x108/0x126
>  [<ffffffff81fa43dc>] __schedule+0x12dc/0x1590
>  [<ffffffff81fa4965>] schedule+0x75/0x150
>  [<ffffffff81fa7683>] ? mutex_lock_nested+0x393/0x780
>  [<ffffffff81fa4db0>] schedule_preempt_disabled+0x30/0x60
>  [<ffffffff81fa753a>] mutex_lock_nested+0x24a/0x780
>  [<ffffffff81244b54>] ? static_key_slow_inc+0xc4/0x140
>  [<ffffffff81244b54>] ? static_key_slow_inc+0xc4/0x140
>  [<ffffffffa035fb48>] ? vcpu_load+0x28/0xf0 [kvm]
>  [<ffffffff81244b54>] static_key_slow_inc+0xc4/0x140
>  [<ffffffff810d36a5>] preempt_notifier_register+0x25/0x70
>  [<ffffffffa035fb96>] vcpu_load+0x76/0xf0 [kvm]
>  [<ffffffffa035fd3e>] kvm_vcpu_ioctl+0x7e/0xeb0 [kvm]
>  [<ffffffff8110bc40>] ? __lock_is_held+0x70/0xa0
>  [<ffffffff810dde79>] ? get_parent_ip+0x19/0x90
>  [<ffffffff8130e4b4>] do_vfs_ioctl+0x3c4/0x910
>  [<ffffffff81320a01>] ? expand_files+0x311/0x360
>  [<ffffffff815370af>] ? selinux_file_ioctl+0x6f/0x150
>  [<ffffffff8130eaad>] SyS_ioctl+0xad/0xe0
>  [<ffffffff81faf857>] entry_SYSCALL_64_fastpath+0x12/0x6f
> 
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  virt/kvm/kvm_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 848af90..bde5f66f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -127,8 +127,8 @@ int vcpu_load(struct kvm_vcpu *vcpu)
>  
>  	if (mutex_lock_killable(&vcpu->mutex))
>  		return -EINTR;
> -	cpu = get_cpu();
>  	preempt_notifier_register(&vcpu->preempt_notifier);
> +	cpu = get_cpu();
>  	kvm_arch_vcpu_load(vcpu, cpu);
>  	put_cpu();
>  	return 0;
> @@ -138,8 +138,8 @@ void vcpu_put(struct kvm_vcpu *vcpu)
>  {
>  	preempt_disable();
>  	kvm_arch_vcpu_put(vcpu);
> -	preempt_notifier_unregister(&vcpu->preempt_notifier);
>  	preempt_enable();
> +	preempt_notifier_unregister(&vcpu->preempt_notifier);
>  	mutex_unlock(&vcpu->mutex);
>  }
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/