Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1336670 > unrolled thread

[PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle related code

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle related code Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
      Re: [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:10 +0100
        Re: [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 17:00 +0100
        Re: [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Radim Krčmář <rkrcmar@redhat.com> - 2016-02-19 17:00 +0100
    [PATCH v2 14/14] KVM: x86: move PIT timer function initialization Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 05/14] KVM: x86: tone down WARN_ON pit.state_lock Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 06/14] KVM: x86: pass struct kvm_pit instead of kvm in PIT Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 09/14] KVM: x86: refactor kvm_create_pit Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 04/14] KVM: x86: use atomic_t instead of pit.inject_lock Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 13/14] KVM: x86: don't assume layout of kvm_kpit_state Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
      Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 17:20 +0100
        Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-18 18:00 +0100
          Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 18:40 +0100
            Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:00 +0100
              Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-19 15:50 +0100
      Re: [PATCH v2 01/14] KVM: x86: change PIT discard tick policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:50 +0100
    [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
      Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard  policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:10 +0100
      Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard  policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:10 +0100
        Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard  policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-19 16:10 +0100
          Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard  policy Paolo Bonzini <pbonzini@redhat.com> - 2016-02-19 16:10 +0100
            Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard  policy Radim Krčmář <rkrcmar@redhat.com> - 2016-02-19 16:10 +0100
    [PATCH v2 10/14] KVM: x86: refactor kvm_free_pit Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 11/14] KVM: x86: remove pit and kvm from kvm_kpit_state Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
    [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state lock Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:20 +0100
      Re: [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state  lock Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:10 +0100
        Re: [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state  lock Radim Krčmář <rkrcmar@redhat.com> - 2016-02-19 15:50 +0100
    [PATCH v2 03/14] KVM: x86: add kvm_pit_reset_reinject Radim Krčmář <rkrcmar@redhat.com> - 2016-02-17 20:30 +0100
    Re: [PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle  related code Paolo Bonzini <pbonzini@redhat.com> - 2016-02-18 19:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1337599 — Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy

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?

Paolo

[toc] | [prev] | [next] | [standalone]

#1338266 — Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy

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 ...

[toc] | [prev] | [next] | [standalone]

#1338267 — Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy

On 19/02/2016 16:04, Radim Krčmář wrote:
>> > 
>> > ... 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.

Yes, I understand why you added the mutex here; good catch indeed.  The
atomic_t is just to show that it's okay to read it outside the lock.
It's just for a bit of extra documentation.

Paolo

[toc] | [prev] | [next] | [standalone]

#1338279 — Re: [PATCH v2 08/14] KVM: x86: remove notifiers from PIT discard policy

2016-02-19 16:06+0100, Paolo Bonzini:
> On 19/02/2016 16:04, Radim Krčmář wrote:
>>>> ... 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.
> 
> Yes, I understand why you added the mutex here; good catch indeed.  The
> atomic_t is just to show that it's okay to read it outside the lock.
> It's just for a bit of extra documentation.

Hm, good point.  I will add it.  Thanks.

[toc] | [prev] | [next] | [standalone]

#1336685 — [PATCH v2 10/14] KVM: x86: refactor kvm_free_pit

Could be easier to read, but git history will become deeper.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: new

 arch/x86/kvm/i8254.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 9dd4460033e3..28ad45abf386 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -738,18 +738,16 @@ fail_request:
 
 void kvm_free_pit(struct kvm *kvm)
 {
-	struct hrtimer *timer;
+	struct kvm_pit *pit = kvm->arch.vpit;
 
-	if (kvm->arch.vpit) {
-		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &kvm->arch.vpit->dev);
-		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS,
-					      &kvm->arch.vpit->speaker_dev);
-		kvm_pit_set_reinject(kvm->arch.vpit, false);
-		timer = &kvm->arch.vpit->pit_state.timer;
-		hrtimer_cancel(timer);
-		flush_kthread_work(&kvm->arch.vpit->expired);
-		kthread_stop(kvm->arch.vpit->worker_task);
-		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
-		kfree(kvm->arch.vpit);
+	if (pit) {
+		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->dev);
+		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->speaker_dev);
+		kvm_pit_set_reinject(pit, false);
+		hrtimer_cancel(&pit->pit_state.timer);
+		flush_kthread_work(&pit->expired);
+		kthread_stop(pit->worker_task);
+		kvm_free_irq_source_id(kvm, pit->irq_source_id);
+		kfree(pit);
 	}
 }
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1336689 — [PATCH v2 11/14] KVM: x86: remove pit and kvm from kvm_kpit_state

kvm isn't ever used and pit can be accessed with container_of.
If you *really* need kvm, pit_state_to_pit(ps)->kvm.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: new

 arch/x86/kvm/i8254.c | 14 +++++++++-----
 arch/x86/kvm/i8254.h |  2 --
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 28ad45abf386..d09925e7934a 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -215,10 +215,16 @@ static void pit_latch_status(struct kvm_pit *pit, int channel)
 	}
 }
 
+static inline struct kvm_pit *pit_state_to_pit(struct kvm_kpit_state *ps)
+{
+	return container_of(ps, struct kvm_pit, pit_state);
+}
+
 static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
 {
 	struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state,
 						 irq_ack_notifier);
+	struct kvm_pit *pit = pit_state_to_pit(ps);
 
 	atomic_set(&ps->irq_ack, 1);
 	/* irq_ack should be set before pending is read.  Order accesses with
@@ -226,7 +232,7 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
 	 */
 	smp_mb();
 	if (atomic_dec_if_positive(&ps->pending) > 0)
-		queue_kthread_work(&ps->pit->worker, &ps->pit->expired);
+		queue_kthread_work(&pit->worker, &pit->expired);
 }
 
 void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
@@ -281,7 +287,7 @@ static void pit_do_work(struct kthread_work *work)
 static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
 {
 	struct kvm_kpit_state *ps = container_of(data, struct kvm_kpit_state, timer);
-	struct kvm_pit *pt = ps->kvm->arch.vpit;
+	struct kvm_pit *pt = pit_state_to_pit(ps);
 
 	if (ps->reinject)
 		atomic_inc(&ps->pending);
@@ -338,12 +344,11 @@ static void create_pit_timer(struct kvm_pit *pit, u32 val, int is_period)
 
 	/* TODO The new value only affected after the retriggered */
 	hrtimer_cancel(&ps->timer);
-	flush_kthread_work(&ps->pit->expired);
+	flush_kthread_work(&pit->expired);
 	ps->period = interval;
 	ps->is_periodic = is_period;
 
 	ps->timer.function = pit_timer_fn;
-	ps->kvm = pit->kvm;
 
 	kvm_pit_reset_reinject(pit);
 
@@ -696,7 +701,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	pit->kvm = kvm;
 
 	pit_state = &pit->pit_state;
-	pit_state->pit = pit;
 	hrtimer_init(&pit_state->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
 
 	pit_state->irq_ack_notifier.gsi = 0;
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index 47a96e803aab..2f11f5e55021 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -32,8 +32,6 @@ struct kvm_kpit_state {
 	u32    speaker_data_on;
 
 	struct mutex lock;
-	struct kvm *kvm;
-	struct kvm_pit *pit;
 	atomic_t pending; /* accumulated triggered timers */
 	atomic_t irq_ack;
 	struct kvm_irq_ack_notifier irq_ack_notifier;
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1336690 — [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state lock

- kvm_create_pit had to lock only because it exposed kvm->arch.vpit very
  early, but initialization doesn't use kvm->arch.vpit since the last
  patch, so we can drop locking.
- kvm_free_pit is only run after there are no users of KVM and therefore
  is the sole actor.
- Locking in kvm_vm_ioctl_reinject doesn't do anything, because reinject
  is only protected at that place.
- kvm_pit_reset isn't used anywhere and its locking can be dropped if we
  hide it.

Removing useless locking allows to see what actually is being protected
by PIT state lock (values accessible from the guest).

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: new

 arch/x86/kvm/i8254.c | 7 -------
 arch/x86/kvm/i8254.h | 7 ++++---
 arch/x86/kvm/x86.c   | 4 ++--
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index c11734dd5092..01f9c70ebc21 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -612,7 +612,6 @@ void kvm_pit_reset(struct kvm_pit *pit)
 	int i;
 	struct kvm_kpit_channel_state *c;
 
-	mutex_lock(&pit->pit_state.lock);
 	pit->pit_state.flags = 0;
 	for (i = 0; i < 3; i++) {
 		c = &pit->pit_state.channels[i];
@@ -620,7 +619,6 @@ void kvm_pit_reset(struct kvm_pit *pit)
 		c->gate = (i != 2);
 		pit_load_count(pit, i, 0);
 	}
-	mutex_unlock(&pit->pit_state.lock);
 
 	kvm_pit_reset_reinject(pit);
 }
@@ -663,7 +661,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	}
 
 	mutex_init(&pit->pit_state.lock);
-	mutex_lock(&pit->pit_state.lock);
 
 	pid = get_pid(task_tgid(current));
 	pid_nr = pid_vnr(pid);
@@ -673,7 +670,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	pit->worker_task = kthread_run(kthread_worker_fn, &pit->worker,
 				       "kvm-pit/%d", pid_nr);
 	if (IS_ERR(pit->worker_task)) {
-		mutex_unlock(&pit->pit_state.lock);
 		kvm_free_irq_source_id(kvm, pit->irq_source_id);
 		kfree(pit);
 		return NULL;
@@ -689,7 +685,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
 	pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq;
 	kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
 	pit_state->reinject = true;
-	mutex_unlock(&pit->pit_state.lock);
 
 	kvm_pit_reset(pit);
 
@@ -737,13 +732,11 @@ void kvm_free_pit(struct kvm *kvm)
 					       &kvm->arch.vpit->mask_notifier);
 		kvm_unregister_irq_ack_notifier(kvm,
 				&kvm->arch.vpit->pit_state.irq_ack_notifier);
-		mutex_lock(&kvm->arch.vpit->pit_state.lock);
 		timer = &kvm->arch.vpit->pit_state.timer;
 		hrtimer_cancel(timer);
 		flush_kthread_work(&kvm->arch.vpit->expired);
 		kthread_stop(kvm->arch.vpit->worker_task);
 		kvm_free_irq_source_id(kvm, kvm->arch.vpit->irq_source_id);
-		mutex_unlock(&kvm->arch.vpit->pit_state.lock);
 		kfree(kvm->arch.vpit);
 	}
 }
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index a6aceaf08df5..568d87ec3698 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -22,17 +22,19 @@ struct kvm_kpit_channel_state {
 };
 
 struct kvm_kpit_state {
+	/* All members before "struct mutex lock" are protected by the lock. */
 	struct kvm_kpit_channel_state channels[3];
 	u32 flags;
 	bool is_periodic;
 	s64 period; 				/* unit: ns */
 	struct hrtimer timer;
-	atomic_t pending;			/* accumulated triggered timers */
 	bool reinject;
-	struct kvm *kvm;
 	u32    speaker_data_on;
+
 	struct mutex lock;
+	struct kvm *kvm;
 	struct kvm_pit *pit;
+	atomic_t pending; /* accumulated triggered timers */
 	atomic_t irq_ack;
 	struct kvm_irq_ack_notifier irq_ack_notifier;
 };
@@ -59,7 +61,6 @@ struct kvm_pit {
 struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags);
 void kvm_free_pit(struct kvm *kvm);
 
-void kvm_pit_reset(struct kvm_pit *pit);
 void kvm_pit_load_count(struct kvm_pit *pit, int channel, u32 val,
 		int hpet_legacy_start);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 57b296466bf0..f2f7ba74da6b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3685,9 +3685,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
 {
 	if (!kvm->arch.vpit)
 		return -ENXIO;
-	mutex_lock(&kvm->arch.vpit->pit_state.lock);
+
 	kvm->arch.vpit->pit_state.reinject = control->pit_reinject;
-	mutex_unlock(&kvm->arch.vpit->pit_state.lock);
+
 	return 0;
 }
 
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1337597 — Re: [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state lock

On 17/02/2016 20:14, Radim Krčmář wrote:
> +	/* All members before "struct mutex lock" are protected by the lock. */

... except reinject, according to the commit message. :)

Paolo

>  	struct kvm_kpit_channel_state channels[3];
>  	u32 flags;
>  	bool is_periodic;
>  	s64 period; 				/* unit: ns */
>  	struct hrtimer timer;
> -	atomic_t pending;			/* accumulated triggered timers */
>  	bool reinject;
> -	struct kvm *kvm;
>  	u32    speaker_data_on;
> +

[toc] | [prev] | [next] | [standalone]

#1338248 — Re: [PATCH v2 07/14] KVM: x86: remove unnecessary uses of PIT state lock

2016-02-18 19:03+0100, Paolo Bonzini:
> On 17/02/2016 20:14, Radim Krčmář wrote:
> > +	/* All members before "struct mutex lock" are protected by the lock. */
> 
> ... except reinject, according to the commit message. :)

Ah, thanks.

[toc] | [prev] | [next] | [standalone]

#1336697 — [PATCH v2 03/14] KVM: x86: add kvm_pit_reset_reinject

pit_state.pending and pit_state.irq_ack are always reset at the same
time.  Create a function for them.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: split from a bigger patch.

 arch/x86/kvm/i8254.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 7d694ac7f4a4..bdbb3f076e72 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -321,6 +321,12 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
 		return HRTIMER_NORESTART;
 }
 
+static inline void kvm_pit_reset_reinject(struct kvm_pit *pit)
+{
+	atomic_set(&pit->pit_state.pending, 0);
+	pit->pit_state.irq_ack = 1;
+}
+
 static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
 {
 	struct kvm_kpit_state *ps = &kvm->arch.vpit->pit_state;
@@ -343,8 +349,7 @@ static void create_pit_timer(struct kvm *kvm, u32 val, int is_period)
 	ps->timer.function = pit_timer_fn;
 	ps->kvm = ps->pit->kvm;
 
-	atomic_set(&ps->pending, 0);
-	ps->irq_ack = 1;
+	kvm_pit_reset_reinject(ps->pit);
 
 	/*
 	 * Do not allow the guest to program periodic timers with small
@@ -644,18 +649,15 @@ void kvm_pit_reset(struct kvm_pit *pit)
 	}
 	mutex_unlock(&pit->pit_state.lock);
 
-	atomic_set(&pit->pit_state.pending, 0);
-	pit->pit_state.irq_ack = 1;
+	kvm_pit_reset_reinject(pit);
 }
 
 static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, bool mask)
 {
 	struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier);
 
-	if (!mask) {
-		atomic_set(&pit->pit_state.pending, 0);
-		pit->pit_state.irq_ack = 1;
-	}
+	if (!mask)
+		kvm_pit_reset_reinject(pit);
 }
 
 static const struct kvm_io_device_ops pit_dev_ops = {
-- 
2.7.1

[toc] | [prev] | [next] | [standalone]

#1337608 — Re: [PATCH v2 00/14] KVM: x86: change PIT discard policy and untangle related code

On 17/02/2016 20:14, Radim Krčmář wrote:
> v2
>  - Different ordering; the important fix is first, which makes it easier
>    to backport as improvements of [1/14] have complex dependencies.
>  - Correct choice of atomics in [2/14].
>  - Explicit SMP barriers for lockless updates in [3/14].
>  - Protection against userspace races in kvm_vm_ioctl_reinject, [8/14].
>  - New code churn.  (I tried to annotate locking, which required me to
>    really look at the code and I couldn't leave it alone at that point;
>    sorry.)
> 
> This series only works with the discard policy, do you want to "fix" NMI
> in the delay policy as well?
> (NMI delivery is still is going to be wrong by any standard, but will
>  make some sense, at the cost of ugly code:  we would always inject NMI
>  when the timer fires and suppress NMI injection on EOI reinject.)
> 
> Anatomy of the series:
>  [1/14] fixes legacy NMI watchdog under discard policy.
>  [2-7/14] prepare for optimization of the discard policy.
>  [8/14] optimizes discard policy by removing notifiers.
>  [9-14/14] slightly improve related code.
> 
> I'm ok with dropping patches [2-14/14].
> 
> v1: http://www.spinics.net/lists/kvm/msg127017.html
> 
> Radim Krčmář (14):
>   KVM: x86: change PIT discard tick policy
>   KVM: x86: simplify atomics in kvm_pit_ack_irq
>   KVM: x86: add kvm_pit_reset_reinject
>   KVM: x86: use atomic_t instead of pit.inject_lock
>   KVM: x86: tone down WARN_ON pit.state_lock
>   KVM: x86: pass struct kvm_pit instead of kvm in PIT
>   KVM: x86: remove unnecessary uses of PIT state lock
>   KVM: x86: remove notifiers from PIT discard policy
>   KVM: x86: refactor kvm_create_pit
>   KVM: x86: refactor kvm_free_pit
>   KVM: x86: remove pit and kvm from kvm_kpit_state
>   KVM: x86: remove pointless dereference of PIT
>   KVM: x86: don't assume layout of kvm_kpit_state
>   KVM: x86: move PIT timer function initialization
> 
>  arch/x86/kvm/i8254.c | 318 +++++++++++++++++++++++----------------------------
>  arch/x86/kvm/i8254.h |  15 +--
>  arch/x86/kvm/x86.c   |  52 ++++++---
>  3 files changed, 187 insertions(+), 198 deletions(-)
> 

Patches 1 and 9-14:

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks!

Paolo

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web