From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1427862AbcBSPEk (ORCPT <rfc822;w@1wt.eu>);
	Fri, 19 Feb 2016 10:04:40 -0500
Received: from mx1.redhat.com ([209.132.183.28]:58154 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1427834AbcBSPEi (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 19 Feb 2016 10:04:38 -0500
Date: Fri, 19 Feb 2016 16:04:35 +0100
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Yuki Shibuya <shibuya.yk@ncos.nec.co.jp>
Subject: Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard
 policy
Message-ID: <20160219150434.GA25910@potion.brq.redhat.com>
References: <1455736496-374-1-git-send-email-rkrcmar@redhat.com>
 <1455736496-374-9-git-send-email-rkrcmar@redhat.com>
 <56C6088A.5080207@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <56C6088A.5080207@redhat.com>
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 19 Feb 2016 15:04:38 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

2016-02-18 19:08+0100, Paolo Bonzini:
> On 17/02/2016 20:14, Radim Krčmář wrote:
>> +	/* pit->pit_state.lock was overloaded to prevent userspace from getting
>> +	 * an inconsistent state after running multiple KVM_REINJECT_CONTROL
>> +	 * ioctls in parallel.  Use a separate lock if that ioctl isn't rare.
>> +	 */
>> +	mutex_lock(&pit->pit_state.lock);
>> +	kvm_pit_set_reinject(pit, control->pit_reinject);
>> +	mutex_unlock(&pit->pit_state.lock);
> 
> ... so in patch 7 concurrent _writes_ of reinject are protected by the
> lock, but reads are done outside it (in pit_timer_fn).  WDYT about
> making reinject an atomic_t?

There was/is no harm in having reinject non-atomic.  This patch added
notifiers, which is the reason for re-introducing a mutex.

Userspace can (and SHOULDN'T) call this function multiple times,
concurrently, so the mutex prevents a situations where, e.g. only one
notifier is registered in the end.

I thought about really stupid stuff when doing this series ...