Skip to contentSkip to conversation

3.0.12-rt30-rc2

PATCH In review Steven Rostedt 9 participants 28 messages Dec 4, 2011 – Nov 14, 2012

Subsystems (8)
Memory management x86 ACPI PCI +4 more subsystems
lore
Share & tools
lore

Steven Rostedt

author reviewer
#0 lore raw

cover 3.0.12-rt30-rc2

Dear RT Folks,

This is the RT stable review cycle of patch 3.0.12-rt30-rc2.

Please scream at me if I messed something up. Please test the patches too.

The difference between this and -rc1 was I fixed up the patches to add the changes, instead of removing them :)

I also added the missing commit:
KVM: fix XSAVE bit scanning (now properly)
That Thomas pointed out was missing.

The -rc release will be uploaded to kernel.org and will be deleted when the final release is out. This is just a review release (or release candidate).

The pre-releases will not be pushed to the git repository, only the final release is.

If all goes well, this patch will be converted to the main release next on Wednesday (11/7/2011).

Enjoy,

-- Steve

To build 3.0.12-rt30-rc2 directly, the following patches should be applied:

http://www.kernel.org/pub/linux/kernel/v3.0/linux-3.0.tar.xz

http://www.kernel.org/pub/linux/kernel/v3.0/patch-3.0.12.xz

http://www.kernel.org/pub/linux/kernel/projects/rt/3.0/patch-3.0.12-rt30-rc2.patch.xz

You can also build from 3.0.12-rt29 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/3.0/incr/patch-3.0.12-rt29-rt30-rc2.patch.xz

Changes from 3.0.12-rt29:

---

Andre Przywara (1):
KVM: fix XSAVE bit scanning (now properly)

Avi Kivity (1):
KVM: Sanitize cpuid

Edward Donovan (1):
genirq: fix regression in irqfixup, irqpoll

Ingo Molnar (1):
tasklet/rt: Prevent tasklets from going into infinite spin in RT

Peter Zijlstra (2):
slab, lockdep: Fix silly bug
slab, lockdep: Annotate all slab caches

Roland Dreier (1):
intel-iommu: Fix AB-BA lockdep report

Steven Rostedt (1):

      Linux v3.0.12-rt30-rc2

Thomas Gleixner (3):
wait: Provide __wake_up_all_locked
pci: Use __wake_up_all_locked pci_unblock_user_cfg_access()
acpi: Make gbl_[hardware|gpe]_lock raw

----

 arch/x86/kvm/x86.c              |   44 +++++++-
 drivers/acpi/acpica/acglobal.h  |    4 +-
 drivers/acpi/acpica/evgpe.c     |    4 +-
 drivers/acpi/acpica/evgpeblk.c  |    8 +-
 drivers/acpi/acpica/evgpeutil.c |   12 +-
 drivers/acpi/acpica/evxface.c   |   10 +-
 drivers/acpi/acpica/evxfgpe.c   |   24 +++---
 drivers/acpi/acpica/hwregs.c    |    4 +-
 drivers/acpi/acpica/hwxface.c   |    4 +-
 drivers/acpi/acpica/utmutex.c   |   21 +----
 drivers/pci/access.c            |    2 +-
 drivers/pci/intel-iommu.c       |    4 +-
 include/linux/interrupt.h       |   39 ++++----
 include/linux/wait.h            |    5 +-
 kernel/irq/spurious.c           |    4 +-
 kernel/sched.c                  |    4 +-
 kernel/softirq.c                |  208 ++++++++++++++++++++++++++++-----------
 localversion-rt                 |    2 +-
 mm/slab.c                       |   55 ++++++-----
 19 files changed, 290 insertions(+), 168 deletions(-)
author reviewer
#1 lore raw

PATCH 1/11 tasklet/rt: Prevent tasklets from going into infinite spin in RT

From: Ingo Molnar <[email protected]>

When CONFIG_PREEMPT_RT_FULL is enabled, tasklets run as threads, and spinlocks turn are mutexes. But this can cause issues with tasks disabling tasklets. A tasklet runs under ksoftirqd, and if a tasklets are disabled with tasklet_disable(), the tasklet count is increased. When a tasklet runs, it checks this counter and if it is set, it adds itself back on the softirq queue and returns.

The problem arises in RT because ksoftirq will see that a softirq is ready to run (the tasklet softirq just re-armed itself), and will not sleep, but instead run the softirqs again. The tasklet softirq will still see that the count is non-zero and will not execute the tasklet and requeue itself on the softirq again, which will cause ksoftirqd to run it again and again and again.

It gets worse because ksoftirqd runs as a real-time thread. If it preempted the task that disabled tasklets, and that task has migration disabled, or can't run for other reasons, the tasklet softirq will never run because the count will never be zero, and ksoftirqd will go into an infinite loop. As an RT task, it this becomes a big problem.

This is a hack solution to have tasklet_disable stop tasklets, and when a tasklet runs, instead of requeueing the tasklet softirqd it delays it. When tasklet_enable() is called, and tasklets are waiting, then the tasklet_enable() will kick the tasklets to continue. This prevents the lock up from ksoftirq going into an infinite loop.

[ ported to 3.0-rt ]
---
 include/linux/interrupt.h |   39 +++++----
 kernel/softirq.c          |  208 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 170 insertions(+), 77 deletions(-)
include/linux/interrupt.h +20 -19
index a62158f..3142442 100644
@@ -501,8 +501,9 @@ extern void __send_remote_softirq(struct call_single_data *cp, int cpu,
      to be executed on some cpu at least once after this.
    * If the tasklet is already scheduled, but its execution is still not
      started, it will be executed only once.
-   * If this tasklet is already running on another CPU (or schedule is called
-     from tasklet itself), it is rescheduled for later.
+   * If this tasklet is already running on another CPU, it is rescheduled
+     for later.
+   * Schedule must not be called from the tasklet itself (a lockup occurs)
    * Tasklet is strictly serialized wrt itself, but not
      wrt another tasklets. If client needs some intertask synchronization,
      he makes it with spinlocks.
@@ -527,27 +528,36 @@ struct tasklet_struct name = { NULL, 0, ATOMIC_INIT(1), func, data }
 enum
 {
 	TASKLET_STATE_SCHED,	/* Tasklet is scheduled for execution */
-	TASKLET_STATE_RUN	/* Tasklet is running (SMP only) */
+	TASKLET_STATE_RUN,	/* Tasklet is running (SMP only) */
+	TASKLET_STATE_PENDING	/* Tasklet is pending */
 };
 
-#ifdef CONFIG_SMP
+#define TASKLET_STATEF_SCHED	(1 << TASKLET_STATE_SCHED)
+#define TASKLET_STATEF_RUN	(1 << TASKLET_STATE_RUN)
+#define TASKLET_STATEF_PENDING	(1 << TASKLET_STATE_PENDING)
+
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_RT_FULL)
 static inline int tasklet_trylock(struct tasklet_struct *t)
 {
 	return !test_and_set_bit(TASKLET_STATE_RUN, &(t)->state);
 }
 
+static inline int tasklet_tryunlock(struct tasklet_struct *t)
+{
+	return cmpxchg(&t->state, TASKLET_STATEF_RUN, 0) == TASKLET_STATEF_RUN;
+}
+
 static inline void tasklet_unlock(struct tasklet_struct *t)
 {
 	smp_mb__before_clear_bit(); 
 	clear_bit(TASKLET_STATE_RUN, &(t)->state);
 }
 
-static inline void tasklet_unlock_wait(struct tasklet_struct *t)
-{
-	while (test_bit(TASKLET_STATE_RUN, &(t)->state)) { barrier(); }
-}
+extern void tasklet_unlock_wait(struct tasklet_struct *t);
+
 #else
 #define tasklet_trylock(t) 1
+#define tasklet_tryunlock(t)	1
 #define tasklet_unlock_wait(t) do { } while (0)
 #define tasklet_unlock(t) do { } while (0)
 #endif
@@ -596,17 +606,8 @@ static inline void tasklet_disable(struct tasklet_struct *t)
 	smp_mb();
 }
 
-static inline void tasklet_enable(struct tasklet_struct *t)
-{
-	smp_mb__before_atomic_dec();
-	atomic_dec(&t->count);
-}
-
-static inline void tasklet_hi_enable(struct tasklet_struct *t)
-{
-	smp_mb__before_atomic_dec();
-	atomic_dec(&t->count);
-}
+extern  void tasklet_enable(struct tasklet_struct *t);
+extern  void tasklet_hi_enable(struct tasklet_struct *t);
 
 extern void tasklet_kill(struct tasklet_struct *t);
 extern void tasklet_kill_immediate(struct tasklet_struct *t, unsigned int cpu);
kernel/softirq.c +150 -58

273 lines not on this page · raw message

Signature · 1 line
1.7.7.1
author reviewer
#2 lore raw

PATCH 2/11 genirq: fix regression in irqfixup, irqpoll

In reply to Steven Rostedt

From: Edward Donovan <[email protected]>

Commit fa27271bc8d2("genirq: Fixup poll handling") introduced a regression that broke irqfixup/irqpoll for some hardware configurations.

Amidst reorganizing 'try_one_irq', that patch removed a test that checked for 'action->handler' returning IRQ_HANDLED, before acting on the interrupt. Restoring this test back returns the functionality lost since 2.6.39. In the current set of tests, after 'action' is set, it must precede '!action->next' to take effect.

With this and my previous patch to irq/spurious.c, c75d720fca8a, all IRQ regressions that I have encountered are fixed.

---
 kernel/irq/spurious.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
kernel/irq/spurious.c +3 -1
index bfe1004..d09e0f5 100644
@@ -84,7 +84,9 @@ static int try_one_irq(int irq, struct irq_desc *desc, bool force)
 	 */
 	action = desc->action;
 	if (!action || !(action->flags & IRQF_SHARED) ||
-	    (action->flags & __IRQF_TIMER) || !action->next)
+	    (action->flags & __IRQF_TIMER) ||
+	    (action->handler(irq, action->dev_id) == IRQ_HANDLED) ||
+	    !action->next)
 		goto out;
 
 	/* Already running on another processor */
Signature · 1 line
1.7.7.1
author reviewer
#3 lore raw

PATCH 3/11 intel-iommu: Fix AB-BA lockdep report

In reply to Steven Rostedt

From: Roland Dreier <[email protected]>

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

- domain_context_mapping_one() takes iommu->lock and calls iommu_support_dev_iotlb(), which takes device_domain_lock (inside iommu->lock).

- domain_remove_one_dev_info() starts by taking device_domain_lock then takes iommu->lock inside it (near the end of the function).

So this is the classic AB-BA deadlock. It looks like a safe fix is to simply release device_domain_lock a bit earlier, since as far as I can tell, it doesn't protect any of the stuff accessed at the end of domain_remove_one_dev_info() anyway.

BTW, the use of device_domain_lock looks a bit unsafe to me... it's at least not obvious to me why we aren't vulnerable to the race below:

iommu_support_dev_iotlb()
domain_remove_dev_info()

lock device_domain_lock
find info
unlock device_domain_lock

lock device_domain_lock
find same info
unlock device_domain_lock

free_devinfo_mem(info)

do stuff with info after it's free

However I don't understand the locking here well enough to know if this is a real problem, let alone what the best fix is.

Anyway here's the full lockdep output that prompted all of this:

     =======================================================
     [ INFO: possible circular locking dependency detected ]
     2.6.39.1+ #1
     -------------------------------------------------------
     bash/13954 is trying to acquire lock:
      (&(&iommu->lock)->rlock){......}, at: [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230

     but task is already holding lock:
      (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     which lock already depends on the new lock.

     the existing dependency chain (in reverse order) is:

     -> #1 (device_domain_lock){-.-...}:
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f8350>] domain_context_mapping_one+0x600/0x750
            [<ffffffff812f84df>] domain_context_mapping+0x3f/0x120
            [<ffffffff812f9175>] iommu_prepare_identity_map+0x1c5/0x1e0
            [<ffffffff81ccf1ca>] intel_iommu_init+0x88e/0xb5e
            [<ffffffff81cab204>] pci_iommu_init+0x16/0x41
            [<ffffffff81002165>] do_one_initcall+0x45/0x190
            [<ffffffff81ca3d3f>] kernel_init+0xe3/0x168
            [<ffffffff8157ac24>] kernel_thread_helper+0x4/0x10

     -> #0 (&(&iommu->lock)->rlock){......}:
            [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
            [<ffffffff812f8b42>] device_notifier+0x72/0x90
            [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
            [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
            [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
            [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
            [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
            [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
            [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
            [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
            [<ffffffff8117569e>] vfs_write+0xce/0x190
            [<ffffffff811759e4>] sys_write+0x54/0xa0
            [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b

     other info that might help us debug this:

     6 locks held by bash/13954:
      #0:  (&buffer->mutex){+.+.+.}, at: [<ffffffff811e4464>] sysfs_write_file+0x44/0x170
      #1:  (s_active#3){++++.+}, at: [<ffffffff811e44ed>] sysfs_write_file+0xcd/0x170
      #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81372edb>] driver_unbind+0x9b/0xc0
      #3:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81373cc7>] device_release_driver+0x27/0x50
      #4:  (&(&priv->bus_notifier)->rwsem){.+.+.+}, at: [<ffffffff8108974f>] __blocking_notifier_call_chain+0x5f/0xb0
      #5:  (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     stack backtrace:
     Pid: 13954, comm: bash Not tainted 2.6.39.1+ #1
     Call Trace:
      [<ffffffff810993a7>] print_circular_bug+0xf7/0x100
      [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff8109d57d>] ? trace_hardirqs_on_caller+0x13d/0x180
      [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
      [<ffffffff812f8b42>] device_notifier+0x72/0x90
      [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
      [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
      [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
      [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
      [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
      [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
      [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
      [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
      [<ffffffff8117569e>] vfs_write+0xce/0x190
      [<ffffffff811759e4>] sys_write+0x54/0xa0
      [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b
---
 drivers/pci/intel-iommu.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/pci/intel-iommu.c +2 -2
index 8c2564d..bc05a51 100644
@@ -3569,6 +3569,8 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			found = 1;
 	}
 
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
 	if (found == 0) {
 		unsigned long tmp_flags;
 		spin_lock_irqsave(&domain->iommu_lock, tmp_flags);
@@ -3585,8 +3587,6 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			spin_unlock_irqrestore(&iommu->lock, tmp_flags);
 		}
 	}
-
-	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
 static void vm_domain_remove_all_dev_info(struct dmar_domain *domain)
Signature · 1 line
1.7.7.1
#4 lore raw

Re: [PATCH 03/11] intel-iommu: Fix AB-BA lockdep report

Quoted 5 lines · Steven Rostedt

From: Roland Dreier <[email protected]>

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

Did this patch not make it into stable releases other than 3.1. I couldn't find it in any other stable tress prior to 3.1.

-- Shuah

Quoted 144 lines

- domain_context_mapping_one() takes iommu->lock and calls iommu_support_dev_iotlb(), which takes device_domain_lock (inside iommu->lock).

- domain_remove_one_dev_info() starts by taking device_domain_lock then takes iommu->lock inside it (near the end of the function).

So this is the classic AB-BA deadlock. It looks like a safe fix is to simply release device_domain_lock a bit earlier, since as far as I can tell, it doesn't protect any of the stuff accessed at the end of domain_remove_one_dev_info() anyway.

BTW, the use of device_domain_lock looks a bit unsafe to me... it's at least not obvious to me why we aren't vulnerable to the race below:

iommu_support_dev_iotlb()
domain_remove_dev_info()

lock device_domain_lock
find info
unlock device_domain_lock

lock device_domain_lock
find same info
unlock device_domain_lock

free_devinfo_mem(info)

do stuff with info after it's free

However I don't understand the locking here well enough to know if this is a real problem, let alone what the best fix is.

Anyway here's the full lockdep output that prompted all of this:

     =======================================================
     [ INFO: possible circular locking dependency detected ]
     2.6.39.1+ #1
     -------------------------------------------------------
     bash/13954 is trying to acquire lock:
      (&(&iommu->lock)->rlock){......}, at: [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230

     but task is already holding lock:
      (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     which lock already depends on the new lock.

     the existing dependency chain (in reverse order) is:

     -> #1 (device_domain_lock){-.-...}:
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f8350>] domain_context_mapping_one+0x600/0x750
            [<ffffffff812f84df>] domain_context_mapping+0x3f/0x120
            [<ffffffff812f9175>] iommu_prepare_identity_map+0x1c5/0x1e0
            [<ffffffff81ccf1ca>] intel_iommu_init+0x88e/0xb5e
            [<ffffffff81cab204>] pci_iommu_init+0x16/0x41
            [<ffffffff81002165>] do_one_initcall+0x45/0x190
            [<ffffffff81ca3d3f>] kernel_init+0xe3/0x168
            [<ffffffff8157ac24>] kernel_thread_helper+0x4/0x10

     -> #0 (&(&iommu->lock)->rlock){......}:
            [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
            [<ffffffff812f8b42>] device_notifier+0x72/0x90
            [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
            [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
            [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
            [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
            [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
            [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
            [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
            [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
            [<ffffffff8117569e>] vfs_write+0xce/0x190
            [<ffffffff811759e4>] sys_write+0x54/0xa0
            [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b

     other info that might help us debug this:

     6 locks held by bash/13954:
      #0:  (&buffer->mutex){+.+.+.}, at: [<ffffffff811e4464>] sysfs_write_file+0x44/0x170
      #1:  (s_active#3){++++.+}, at: [<ffffffff811e44ed>] sysfs_write_file+0xcd/0x170
      #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81372edb>] driver_unbind+0x9b/0xc0
      #3:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81373cc7>] device_release_driver+0x27/0x50
      #4:  (&(&priv->bus_notifier)->rwsem){.+.+.+}, at: [<ffffffff8108974f>] __blocking_notifier_call_chain+0x5f/0xb0
      #5:  (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     stack backtrace:
     Pid: 13954, comm: bash Not tainted 2.6.39.1+ #1
     Call Trace:
      [<ffffffff810993a7>] print_circular_bug+0xf7/0x100
      [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff8109d57d>] ? trace_hardirqs_on_caller+0x13d/0x180
      [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
      [<ffffffff812f8b42>] device_notifier+0x72/0x90
      [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
      [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
      [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
      [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
      [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
      [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
      [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
      [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
      [<ffffffff8117569e>] vfs_write+0xce/0x190
      [<ffffffff811759e4>] sys_write+0x54/0xa0
      [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b
---
 drivers/pci/intel-iommu.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/pci/intel-iommu.c +2 -2
index 8c2564d..bc05a51 100644
@@ -3569,6 +3569,8 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			found = 1;
 	}
 
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
 	if (found == 0) {
 		unsigned long tmp_flags;
 		spin_lock_irqsave(&domain->iommu_lock, tmp_flags);
@@ -3585,8 +3587,6 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			spin_unlock_irqrestore(&iommu->lock, tmp_flags);
 		}
 	}
-
-	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
 static void vm_domain_remove_all_dev_info(struct dmar_domain *domain)
author reviewer
#5 lore raw

Re: [PATCH 03/11] intel-iommu: Fix AB-BA lockdep report

Quoted 9 lines · Shuah Khan
Quoted 5 lines · Steven Rostedt

From: Roland Dreier <[email protected]>

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

Did this patch not make it into stable releases other than 3.1. I couldn't find it in any other stable tress prior to 3.1.

Ah, this was done in the early stable-rt releases. Where we took mainline fixes as long as they were in tglx's tree. But today the stable-rt tree waits for mainline fixes to come in via the stable tree so we don't do something like this. That is, miss getting a fix into stable. Yeah, this should be added to 3.0. (looks to be already in 3.2 and 3.4).

Greg,

Can you add the following commit to the 3.0 stable tree. We've had this in v3.0-rt for some time now. :-/

commit 3e7abe2556b583e87dabda3e0e6178a67b20d06f

Author: Roland Dreier <[email protected]>
Date:   Wed Jul 20 06:22:21 2011 -0700

intel-iommu: Fix AB-BA lockdep report

Thanks,

-- Steve

Quoted 147 lines

-- Shuah

Quoted 144 lines

- domain_context_mapping_one() takes iommu->lock and calls iommu_support_dev_iotlb(), which takes device_domain_lock (inside iommu->lock).

- domain_remove_one_dev_info() starts by taking device_domain_lock then takes iommu->lock inside it (near the end of the function).

So this is the classic AB-BA deadlock. It looks like a safe fix is to simply release device_domain_lock a bit earlier, since as far as I can tell, it doesn't protect any of the stuff accessed at the end of domain_remove_one_dev_info() anyway.

BTW, the use of device_domain_lock looks a bit unsafe to me... it's at least not obvious to me why we aren't vulnerable to the race below:

iommu_support_dev_iotlb()
domain_remove_dev_info()

lock device_domain_lock
find info
unlock device_domain_lock

lock device_domain_lock
find same info
unlock device_domain_lock

free_devinfo_mem(info)

do stuff with info after it's free

However I don't understand the locking here well enough to know if this is a real problem, let alone what the best fix is.

Anyway here's the full lockdep output that prompted all of this:

     =======================================================
     [ INFO: possible circular locking dependency detected ]
     2.6.39.1+ #1
     -------------------------------------------------------
     bash/13954 is trying to acquire lock:
      (&(&iommu->lock)->rlock){......}, at: [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230

     but task is already holding lock:
      (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     which lock already depends on the new lock.

     the existing dependency chain (in reverse order) is:

     -> #1 (device_domain_lock){-.-...}:
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f8350>] domain_context_mapping_one+0x600/0x750
            [<ffffffff812f84df>] domain_context_mapping+0x3f/0x120
            [<ffffffff812f9175>] iommu_prepare_identity_map+0x1c5/0x1e0
            [<ffffffff81ccf1ca>] intel_iommu_init+0x88e/0xb5e
            [<ffffffff81cab204>] pci_iommu_init+0x16/0x41
            [<ffffffff81002165>] do_one_initcall+0x45/0x190
            [<ffffffff81ca3d3f>] kernel_init+0xe3/0x168
            [<ffffffff8157ac24>] kernel_thread_helper+0x4/0x10

     -> #0 (&(&iommu->lock)->rlock){......}:
            [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
            [<ffffffff812f8b42>] device_notifier+0x72/0x90
            [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
            [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
            [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
            [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
            [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
            [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
            [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
            [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
            [<ffffffff8117569e>] vfs_write+0xce/0x190
            [<ffffffff811759e4>] sys_write+0x54/0xa0
            [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b

     other info that might help us debug this:

     6 locks held by bash/13954:
      #0:  (&buffer->mutex){+.+.+.}, at: [<ffffffff811e4464>] sysfs_write_file+0x44/0x170
      #1:  (s_active#3){++++.+}, at: [<ffffffff811e44ed>] sysfs_write_file+0xcd/0x170
      #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81372edb>] driver_unbind+0x9b/0xc0
      #3:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81373cc7>] device_release_driver+0x27/0x50
      #4:  (&(&priv->bus_notifier)->rwsem){.+.+.+}, at: [<ffffffff8108974f>] __blocking_notifier_call_chain+0x5f/0xb0
      #5:  (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     stack backtrace:
     Pid: 13954, comm: bash Not tainted 2.6.39.1+ #1
     Call Trace:
      [<ffffffff810993a7>] print_circular_bug+0xf7/0x100
      [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff8109d57d>] ? trace_hardirqs_on_caller+0x13d/0x180
      [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
      [<ffffffff812f8b42>] device_notifier+0x72/0x90
      [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
      [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
      [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
      [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
      [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
      [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
      [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
      [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
      [<ffffffff8117569e>] vfs_write+0xce/0x190
      [<ffffffff811759e4>] sys_write+0x54/0xa0
      [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b
---
 drivers/pci/intel-iommu.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/pci/intel-iommu.c +2 -2
index 8c2564d..bc05a51 100644
@@ -3569,6 +3569,8 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			found = 1;
 	}
 
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
 	if (found == 0) {
 		unsigned long tmp_flags;
 		spin_lock_irqsave(&domain->iommu_lock, tmp_flags);
@@ -3585,8 +3587,6 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			spin_unlock_irqrestore(&iommu->lock, tmp_flags);
 		}
 	}
-
-	spin_unlock_irqrestore(&device_domain_lock, flags);
 }
 
 static void vm_domain_remove_all_dev_info(struct dmar_domain *domain)
#6 lore raw

Re: [PATCH 03/11] intel-iommu: Fix AB-BA lockdep report

Quoted 21 lines · Steven Rostedt
Quoted 9 lines · Shuah Khan
Quoted 5 lines · Steven Rostedt

From: Roland Dreier <[email protected]>

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

Did this patch not make it into stable releases other than 3.1. I couldn't find it in any other stable tress prior to 3.1.

Ah, this was done in the early stable-rt releases. Where we took mainline fixes as long as they were in tglx's tree. But today the stable-rt tree waits for mainline fixes to come in via the stable tree so we don't do something like this. That is, miss getting a fix into stable. Yeah, this should be added to 3.0. (looks to be already in 3.2 and 3.4).

Greg,

Quoted 8 lines

Can you add the following commit to the 3.0 stable tree. We've had this in v3.0-rt for some time now. :-/

commit 3e7abe2556b583e87dabda3e0e6178a67b20d06f

Author: Roland Dreier <[email protected]>
Date:   Wed Jul 20 06:22:21 2011 -0700

intel-iommu: Fix AB-BA lockdep report

I had to edit the path to the file, but it looks like it applies properly now, thanks.

greg k-h

author reviewer
#7 lore raw

Re: [PATCH 03/11] intel-iommu: Fix AB-BA lockdep report

Quoted 2 lines · Greg Kroah-Hartman

I had to edit the path to the file, but it looks like it applies properly now, thanks.

Thanks,

Just in case, below is what I applied to v3.0-rt.

-- Steve

Quoted 1 line

From fd92bf3e21c77fe06647b57feebb1b52da54969a Mon Sep 17 00:00:00 2001

From: Roland Dreier <[email protected]>
Date: Wed, 20 Jul 2011 06:22:21 -0700
Subject: [PATCH] intel-iommu: Fix AB-BA lockdep report

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

- domain_context_mapping_one() takes iommu->lock and calls iommu_support_dev_iotlb(), which takes device_domain_lock (inside iommu->lock).

- domain_remove_one_dev_info() starts by taking device_domain_lock then takes iommu->lock inside it (near the end of the function).

So this is the classic AB-BA deadlock. It looks like a safe fix is to simply release device_domain_lock a bit earlier, since as far as I can tell, it doesn't protect any of the stuff accessed at the end of domain_remove_one_dev_info() anyway.

BTW, the use of device_domain_lock looks a bit unsafe to me... it's at least not obvious to me why we aren't vulnerable to the race below:

iommu_support_dev_iotlb()
domain_remove_dev_info()

lock device_domain_lock
find info
unlock device_domain_lock

lock device_domain_lock
find same info
unlock device_domain_lock

free_devinfo_mem(info)

do stuff with info after it's free

However I don't understand the locking here well enough to know if this is a real problem, let alone what the best fix is.

Anyway here's the full lockdep output that prompted all of this:

     =======================================================
     [ INFO: possible circular locking dependency detected ]
     2.6.39.1+ #1
     -------------------------------------------------------
     bash/13954 is trying to acquire lock:
      (&(&iommu->lock)->rlock){......}, at: [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230

     but task is already holding lock:
      (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     which lock already depends on the new lock.

     the existing dependency chain (in reverse order) is:

     -> #1 (device_domain_lock){-.-...}:
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f8350>] domain_context_mapping_one+0x600/0x750
            [<ffffffff812f84df>] domain_context_mapping+0x3f/0x120
            [<ffffffff812f9175>] iommu_prepare_identity_map+0x1c5/0x1e0
            [<ffffffff81ccf1ca>] intel_iommu_init+0x88e/0xb5e
            [<ffffffff81cab204>] pci_iommu_init+0x16/0x41
            [<ffffffff81002165>] do_one_initcall+0x45/0x190
            [<ffffffff81ca3d3f>] kernel_init+0xe3/0x168
            [<ffffffff8157ac24>] kernel_thread_helper+0x4/0x10

     -> #0 (&(&iommu->lock)->rlock){......}:
            [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
            [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
            [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
            [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
            [<ffffffff812f8b42>] device_notifier+0x72/0x90
            [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
            [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
            [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
            [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
            [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
            [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
            [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
            [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
            [<ffffffff8117569e>] vfs_write+0xce/0x190
            [<ffffffff811759e4>] sys_write+0x54/0xa0
            [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b

     other info that might help us debug this:

     6 locks held by bash/13954:
      #0:  (&buffer->mutex){+.+.+.}, at: [<ffffffff811e4464>] sysfs_write_file+0x44/0x170
      #1:  (s_active#3){++++.+}, at: [<ffffffff811e44ed>] sysfs_write_file+0xcd/0x170
      #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81372edb>] driver_unbind+0x9b/0xc0
      #3:  (&__lockdep_no_validate__){+.+.+.}, at: [<ffffffff81373cc7>] device_release_driver+0x27/0x50
      #4:  (&(&priv->bus_notifier)->rwsem){.+.+.+}, at: [<ffffffff8108974f>] __blocking_notifier_call_chain+0x5f/0xb0
      #5:  (device_domain_lock){-.-...}, at: [<ffffffff812f6508>] domain_remove_one_dev_info+0x208/0x230

     stack backtrace:
     Pid: 13954, comm: bash Not tainted 2.6.39.1+ #1
     Call Trace:
      [<ffffffff810993a7>] print_circular_bug+0xf7/0x100
      [<ffffffff8109bf3e>] __lock_acquire+0x195e/0x1e10
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff8109d57d>] ? trace_hardirqs_on_caller+0x13d/0x180
      [<ffffffff8109ca9d>] lock_acquire+0x9d/0x130
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff81571475>] _raw_spin_lock_irqsave+0x55/0xa0
      [<ffffffff812f6421>] ? domain_remove_one_dev_info+0x121/0x230
      [<ffffffff810972bd>] ? trace_hardirqs_off+0xd/0x10
      [<ffffffff812f6421>] domain_remove_one_dev_info+0x121/0x230
      [<ffffffff812f8b42>] device_notifier+0x72/0x90
      [<ffffffff8157555c>] notifier_call_chain+0x8c/0xc0
      [<ffffffff81089768>] __blocking_notifier_call_chain+0x78/0xb0
      [<ffffffff810897b6>] blocking_notifier_call_chain+0x16/0x20
      [<ffffffff81373a5c>] __device_release_driver+0xbc/0xe0
      [<ffffffff81373ccf>] device_release_driver+0x2f/0x50
      [<ffffffff81372ee3>] driver_unbind+0xa3/0xc0
      [<ffffffff813724ac>] drv_attr_store+0x2c/0x30
      [<ffffffff811e4506>] sysfs_write_file+0xe6/0x170
      [<ffffffff8117569e>] vfs_write+0xce/0x190
      [<ffffffff811759e4>] sys_write+0x54/0xa0
      [<ffffffff81579a82>] system_call_fastpath+0x16/0x1b
---
 drivers/pci/intel-iommu.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/intel-iommu.c +2 -0
index 8c2564d..bc05a51 100644
@@ -3569,6 +3569,8 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			found = 1;
 	}
 
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
 	if (found == 0) {
 		unsigned long tmp_flags;
 		spin_lock_irqsave(&domain->iommu_lock, tmp_flags);
@@ -3585,8 +3587,6 @@ static void domain_remove_one_dev_info(struct dmar_domain *domain,
 			spin_unlock_irqrestore(&iommu->lock, tmp_flags);
 		}
 	}
#8 lore raw

Re: [PATCH 03/11] intel-iommu: Fix AB-BA lockdep report

In reply to Greg Kroah-Hartman
Quoted 25 lines · Greg Kroah-Hartman
Quoted 24 lines · Steven Rostedt
Quoted 9 lines · Shuah Khan
Quoted 5 lines · Steven Rostedt

From: Roland Dreier <[email protected]>

When unbinding a device so that I could pass it through to a KVM VM, I got the lockdep report below. It looks like a legitimate lock ordering problem:

Did this patch not make it into stable releases other than 3.1. I couldn't find it in any other stable tress prior to 3.1.

Ah, this was done in the early stable-rt releases. Where we took mainline fixes as long as they were in tglx's tree. But today the stable-rt tree waits for mainline fixes to come in via the stable tree so we don't do something like this. That is, miss getting a fix into stable. Yeah, this should be added to 3.0. (looks to be already in 3.2 and 3.4).

Greg,

Can you add the following commit to the 3.0 stable tree. We've had this in v3.0-rt for some time now. :-/

Quoted 8 lines
Quoted 5 lines

commit 3e7abe2556b583e87dabda3e0e6178a67b20d06f

Author: Roland Dreier <[email protected]>
Date:   Wed Jul 20 06:22:21 2011 -0700

intel-iommu: Fix AB-BA lockdep report

I had to edit the path to the file, but it looks like it applies properly now, thanks.

Thanks,
-- Shuah

author reviewer
#9 lore raw

PATCH 4/11 KVM: Sanitize cpuid

In reply to Steven Rostedt

From: Avi Kivity <[email protected]>

Instead of blacklisting known-unsupported cpuid leaves, whitelist known- supported leaves. This is more conservative and prevents us from reporting features we don't support. Also whitelist a few more leaves while at it.

---
 arch/x86/kvm/x86.c |   37 +++++++++++++++++++++++++++++++++++--
 1 files changed, 35 insertions(+), 2 deletions(-)
arch/x86/kvm/x86.c +35 -2
index 545c61b..f168c61 100644
@@ -2283,6 +2283,13 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	entry->flags = 0;
 }
 
+static bool supported_xcr0_bit(unsigned bit)
+{
+	u64 mask = ((u64)1 << bit);
+
+	return mask & (XSTATE_FP | XSTATE_SSE | XSTATE_YMM) & host_xcr0;
+}
+
 #define F(x) bit(X86_FEATURE_##x)
 
 static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
@@ -2393,6 +2400,8 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		}
 		break;
 	}
+	case 9:
+		break;
 	case 0xb: {
 		int i, level_type;
 
@@ -2414,7 +2423,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 
 		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
 		for (i = 1; *nent < maxnent && i < 64; ++i) {
-			if (entry[i].eax == 0)
+			if (entry[i].eax == 0 || !supported_xcr0_bit(i))
 				continue;
 			do_cpuid_1_ent(&entry[i], function, i);
 			entry[i].flags |=
@@ -2451,6 +2460,24 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		entry->ecx &= kvm_supported_word6_x86_features;
 		cpuid_mask(&entry->ecx, 6);
 		break;
+	case 0x80000008: {
+		unsigned g_phys_as = (entry->eax >> 16) & 0xff;
+		unsigned virt_as = max((entry->eax >> 8) & 0xff, 48U);
+		unsigned phys_as = entry->eax & 0xff;
+
+		if (!g_phys_as)
+			g_phys_as = phys_as;
+		entry->eax = g_phys_as | (virt_as << 8);
+		entry->ebx = entry->edx = 0;
+		break;
+	}
+	case 0x80000019:
+		entry->ecx = entry->edx = 0;
+		break;
+	case 0x8000001a:
+		break;
+	case 0x8000001d:
+		break;
 	/*Add support for Centaur's CPUID instruction*/
 	case 0xC0000000:
 		/*Just support up to 0xC0000004 now*/
@@ -2460,10 +2487,16 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		entry->edx &= kvm_supported_word5_x86_features;
 		cpuid_mask(&entry->edx, 5);
 		break;
+	case 3: /* Processor serial number */
+	case 5: /* MONITOR/MWAIT */
+	case 6: /* Thermal management */
+	case 0xA: /* Architectural Performance Monitoring */
+	case 0x80000007: /* Advanced power management */
 	case 0xC0000002:
 	case 0xC0000003:
 	case 0xC0000004:
-		/*Now nothing to do, reserved for the future*/
+	default:
+		entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
 		break;
 	}
 
Signature · 1 line
1.7.7.1
author reviewer
#10 lore raw

PATCH 5/11 KVM: fix XSAVE bit scanning (now properly)

In reply to Steven Rostedt

From: Andre Przywara <[email protected]>

commit 123108f1c1aafd51d6a5c79cc04d7999dd88a930 tried to fix KVMs XSAVE valid feature scanning, but it was wrong. It was not considering the sparse nature of this bitfield, instead reading values from uninitialized members of the entries array.
This patch now separates subleaf indicies from KVM's array indicies and fills the entry before querying it's value.
This fixes AVX support in KVM guests.

---
 arch/x86/kvm/x86.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
arch/x86/kvm/x86.c +5 -4
index f168c61..3567c76 100644
@@ -2419,16 +2419,17 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		break;
 	}
 	case 0xd: {
-		int i;
+		int idx, i;
 
 		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
-		for (i = 1; *nent < maxnent && i < 64; ++i) {
-			if (entry[i].eax == 0 || !supported_xcr0_bit(i))
+		for (idx = 1, i = 1; *nent < maxnent && idx < 64; ++idx) {
+			do_cpuid_1_ent(&entry[i], function, idx);
+			if (entry[i].eax == 0 || !supported_xcr0_bit(idx))
 				continue;
-			do_cpuid_1_ent(&entry[i], function, i);
 			entry[i].flags |=
 			       KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
 			++*nent;
+			++i;
 		}
 		break;
 	}
Signature · 1 line
1.7.7.1
author reviewer
#11 lore raw

PATCH 6/11 wait: Provide __wake_up_all_locked

In reply to Steven Rostedt

From: Thomas Gleixner <[email protected]>

For code which protects the waitqueue itself with another lock it makes no sense to acquire the waitqueue lock for wakeup all. Provide __wake_up_all_locked.

---
 include/linux/wait.h |    5 +++--
 kernel/sched.c       |    4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)
include/linux/wait.h +3 -2
index 3efc9f3..1e904b8 100644
@@ -157,7 +157,7 @@ void __wake_up(wait_queue_head_t *q, unsigned int mode, int nr, void *key);
 void __wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key);
 void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode, int nr,
 			void *key);
-void __wake_up_locked(wait_queue_head_t *q, unsigned int mode);
+void __wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr);
 void __wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr);
 void __wake_up_bit(wait_queue_head_t *, void *, int);
 int __wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, int (*)(void *), unsigned);
@@ -170,7 +170,8 @@ wait_queue_head_t *bit_waitqueue(void *, int);
 #define wake_up(x)			__wake_up(x, TASK_NORMAL, 1, NULL)
 #define wake_up_nr(x, nr)		__wake_up(x, TASK_NORMAL, nr, NULL)
 #define wake_up_all(x)			__wake_up(x, TASK_NORMAL, 0, NULL)
-#define wake_up_locked(x)		__wake_up_locked((x), TASK_NORMAL)
+#define wake_up_locked(x)		__wake_up_locked((x), TASK_NORMAL, 1)
+#define wake_up_all_locked(x)		__wake_up_locked((x), TASK_NORMAL, 0)
 
 #define wake_up_interruptible(x)	__wake_up(x, TASK_INTERRUPTIBLE, 1, NULL)
 #define wake_up_interruptible_nr(x, nr)	__wake_up(x, TASK_INTERRUPTIBLE, nr, NULL)
kernel/sched.c +2 -2
index 640f740..63aeba0 100644
@@ -4642,9 +4642,9 @@ EXPORT_SYMBOL(__wake_up);
 /*
  * Same as __wake_up but called with the spinlock in wait_queue_head_t held.
  */
-void __wake_up_locked(wait_queue_head_t *q, unsigned int mode)
+void __wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr)
 {
-	__wake_up_common(q, mode, 1, 0, NULL);
+	__wake_up_common(q, mode, nr, 0, NULL);
 }
 EXPORT_SYMBOL_GPL(__wake_up_locked);
 
Signature · 1 line
1.7.7.1
author reviewer
#12 lore raw

PATCH 7/11 pci: Use __wake_up_all_locked pci_unblock_user_cfg_access()

In reply to Steven Rostedt

From: Thomas Gleixner <[email protected]>

The waitqueue is protected by the pci_lock, so we can just avoid to lock the waitqueue lock itself. That prevents the might_sleep()/scheduling while atomic problem on RT

---
 drivers/pci/access.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/pci/access.c +1 -1
index fdaa42a..1a6cc67 100644
@@ -441,7 +441,7 @@ void pci_unblock_user_cfg_access(struct pci_dev *dev)
 	WARN_ON(!dev->block_ucfg_access);
 
 	dev->block_ucfg_access = 0;
-	wake_up_all(&pci_ucfg_wait);
+	wake_up_all_locked(&pci_ucfg_wait);
 	raw_spin_unlock_irqrestore(&pci_lock, flags);
 }
 EXPORT_SYMBOL_GPL(pci_unblock_user_cfg_access);
Signature · 1 line
1.7.7.1
author reviewer
#13 lore raw

PATCH 8/11 acpi: Make gbl_[hardware|gpe]_lock raw

In reply to Steven Rostedt

From: Thomas Gleixner <[email protected]>

These locks are taken in the guts of the idle code and cannot be converted to "sleeping" spinlocks on RT

---
 drivers/acpi/acpica/acglobal.h  |    4 ++--
 drivers/acpi/acpica/evgpe.c     |    4 ++--
 drivers/acpi/acpica/evgpeblk.c  |    8 ++++----
 drivers/acpi/acpica/evgpeutil.c |   12 ++++++------
 drivers/acpi/acpica/evxface.c   |   10 +++++-----
 drivers/acpi/acpica/evxfgpe.c   |   24 ++++++++++++------------
 drivers/acpi/acpica/hwregs.c    |    4 ++--
 drivers/acpi/acpica/hwxface.c   |    4 ++--
 drivers/acpi/acpica/utmutex.c   |   21 +++------------------
 9 files changed, 38 insertions(+), 53 deletions(-)
drivers/acpi/acpica/acglobal.h +2 -2
index 73863d8..6c169a2 100644
@@ -229,8 +229,8 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_pending;
  * Spinlocks are used for interfaces that can be possibly called at
  * interrupt level
  */
-ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
-ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
+extern raw_spinlock_t acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
+extern raw_spinlock_t acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
 
 /*****************************************************************************
  *
drivers/acpi/acpica/evgpe.c +2 -2
index 65c79ad..36e7e10 100644
@@ -357,7 +357,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
 	 * Note: Not necessary to obtain the hardware lock, since the GPE
 	 * registers are owned by the gpe_lock.
 	 */
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Examine all GPE blocks attached to this interrupt level */
 
@@ -440,7 +440,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
 
       unlock_and_exit:
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return (int_status);
 }
 
drivers/acpi/acpica/evgpeblk.c +4 -4
index ca2c41a..60c47b9 100644
@@ -95,7 +95,7 @@ acpi_ev_install_gpe_block(struct acpi_gpe_block_info *gpe_block,
 
 	/* Install the new block at the end of the list with lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (gpe_xrupt_block->gpe_block_list_head) {
 		next_gpe_block = gpe_xrupt_block->gpe_block_list_head;
 		while (next_gpe_block->next) {
@@ -109,7 +109,7 @@ acpi_ev_install_gpe_block(struct acpi_gpe_block_info *gpe_block,
 	}
 
 	gpe_block->xrupt_block = gpe_xrupt_block;
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
       unlock_and_exit:
 	status = acpi_ut_release_mutex(ACPI_MTX_EVENTS);
@@ -156,7 +156,7 @@ acpi_status acpi_ev_delete_gpe_block(struct acpi_gpe_block_info *gpe_block)
 	} else {
 		/* Remove the block on this interrupt with lock */
 
-		flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+		raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 		if (gpe_block->previous) {
 			gpe_block->previous->next = gpe_block->next;
 		} else {
@@ -167,7 +167,7 @@ acpi_status acpi_ev_delete_gpe_block(struct acpi_gpe_block_info *gpe_block)
 		if (gpe_block->next) {
 			gpe_block->next->previous = gpe_block->previous;
 		}
-		acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+		raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	}
 
 	acpi_current_gpe_count -= gpe_block->gpe_count;
drivers/acpi/acpica/evgpeutil.c +6 -6
index 80a81d0..895b68ab 100644
@@ -70,7 +70,7 @@ acpi_ev_walk_gpe_list(acpi_gpe_callback gpe_walk_callback, void *context)
 
 	ACPI_FUNCTION_TRACE(ev_walk_gpe_list);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Walk the interrupt level descriptor list */
 
@@ -101,7 +101,7 @@ acpi_ev_walk_gpe_list(acpi_gpe_callback gpe_walk_callback, void *context)
 	}
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 
@@ -237,7 +237,7 @@ struct acpi_gpe_xrupt_info *acpi_ev_get_gpe_xrupt_block(u32 interrupt_number)
 
 	/* Install new interrupt descriptor with spin lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (acpi_gbl_gpe_xrupt_list_head) {
 		next_gpe_xrupt = acpi_gbl_gpe_xrupt_list_head;
 		while (next_gpe_xrupt->next) {
@@ -249,7 +249,7 @@ struct acpi_gpe_xrupt_info *acpi_ev_get_gpe_xrupt_block(u32 interrupt_number)
 	} else {
 		acpi_gbl_gpe_xrupt_list_head = gpe_xrupt;
 	}
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	/* Install new interrupt handler if not SCI_INT */
 
@@ -306,7 +306,7 @@ acpi_status acpi_ev_delete_gpe_xrupt(struct acpi_gpe_xrupt_info *gpe_xrupt)
 
 	/* Unlink the interrupt block with lock */
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 	if (gpe_xrupt->previous) {
 		gpe_xrupt->previous->next = gpe_xrupt->next;
 	} else {
@@ -318,7 +318,7 @@ acpi_status acpi_ev_delete_gpe_xrupt(struct acpi_gpe_xrupt_info *gpe_xrupt)
 	if (gpe_xrupt->next) {
 		gpe_xrupt->next->previous = gpe_xrupt->previous;
 	}
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	/* Free the block */
 
drivers/acpi/acpica/evxface.c +5 -5
index e114140..e849c10 100644
@@ -750,7 +750,7 @@ acpi_install_gpe_handler(acpi_handle gpe_device,
 		goto unlock_and_exit;
 	}
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -798,14 +798,14 @@ acpi_install_gpe_handler(acpi_handle gpe_device,
 	    ~(ACPI_GPE_XRUPT_TYPE_MASK | ACPI_GPE_DISPATCH_MASK);
 	gpe_event_info->flags |= (u8) (type | ACPI_GPE_DISPATCH_HANDLER);
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 unlock_and_exit:
 	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
 	return_ACPI_STATUS(status);
 
 free_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	ACPI_FREE(handler);
 	goto unlock_and_exit;
 }
@@ -852,7 +852,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
 		return_ACPI_STATUS(status);
 	}
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -903,7 +903,7 @@ acpi_remove_gpe_handler(acpi_handle gpe_device,
 	ACPI_FREE(handler);
 
 unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 
 	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
 	return_ACPI_STATUS(status);
drivers/acpi/acpica/evxfgpe.c +12 -12
index 52aaff3..ce07ebb 100644
@@ -121,7 +121,7 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
 
 	ACPI_FUNCTION_TRACE(acpi_enable_gpe);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -130,7 +130,7 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
 		status = acpi_ev_add_gpe_reference(gpe_event_info);
 	}
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 ACPI_EXPORT_SYMBOL(acpi_enable_gpe)
@@ -158,7 +158,7 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number)
 
 	ACPI_FUNCTION_TRACE(acpi_disable_gpe);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -167,7 +167,7 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number)
 		status = acpi_ev_remove_gpe_reference(gpe_event_info) ;
 	}
 
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 ACPI_EXPORT_SYMBOL(acpi_disable_gpe)
@@ -214,7 +214,7 @@ acpi_setup_gpe_for_wake(acpi_handle wake_device,
 		return_ACPI_STATUS(AE_BAD_PARAMETER);
 	}
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -270,7 +270,7 @@ acpi_setup_gpe_for_wake(acpi_handle wake_device,
 	status = AE_OK;
 
  unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 ACPI_EXPORT_SYMBOL(acpi_setup_gpe_for_wake)
@@ -300,7 +300,7 @@ acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 ac
 
 	ACPI_FUNCTION_TRACE(acpi_set_gpe_wake_mask);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/*
 	 * Ensure that we have a valid GPE number and that this GPE is in
@@ -346,7 +346,7 @@ acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 ac
 	}
 
 unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 
@@ -372,7 +372,7 @@ acpi_status acpi_clear_gpe(acpi_handle gpe_device, u32 gpe_number)
 
 	ACPI_FUNCTION_TRACE(acpi_clear_gpe);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -385,7 +385,7 @@ acpi_status acpi_clear_gpe(acpi_handle gpe_device, u32 gpe_number)
 	status = acpi_hw_clear_gpe(gpe_event_info);
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 
@@ -415,7 +415,7 @@ acpi_get_gpe_status(acpi_handle gpe_device,
 
 	ACPI_FUNCTION_TRACE(acpi_get_gpe_status);
 
-	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_gpe_lock, flags);
 
 	/* Ensure that we have a valid GPE number */
 
@@ -433,7 +433,7 @@ acpi_get_gpe_status(acpi_handle gpe_device,
 		*event_status |= ACPI_EVENT_FLAG_HANDLE;
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_gpe_lock, flags);
 	return_ACPI_STATUS(status);
 }
 
drivers/acpi/acpica/hwregs.c +2 -2
index 55accb7..4772930 100644
@@ -263,7 +263,7 @@ acpi_status acpi_hw_clear_acpi_status(void)
 			  ACPI_BITMASK_ALL_FIXED_STATUS,
 			  ACPI_FORMAT_UINT64(acpi_gbl_xpm1a_status.address)));
 
-	lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 
 	/* Clear the fixed events in PM1 A/B */
 
@@ -278,7 +278,7 @@ acpi_status acpi_hw_clear_acpi_status(void)
 	status = acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
 
       unlock_and_exit:
-	acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
 	return_ACPI_STATUS(status);
 }
 
drivers/acpi/acpica/hwxface.c +2 -2
index f75f81a..76159ba 100644
@@ -386,7 +386,7 @@ acpi_status acpi_write_bit_register(u32 register_id, u32 value)
 		return_ACPI_STATUS(AE_BAD_PARAMETER);
 	}
 
-	lock_flags = acpi_os_acquire_lock(acpi_gbl_hardware_lock);
+	raw_spin_lock_irqsave(&acpi_gbl_hardware_lock, lock_flags);
 
 	/*
 	 * At this point, we know that the parent register is one of the
@@ -447,7 +447,7 @@ acpi_status acpi_write_bit_register(u32 register_id, u32 value)
 
 unlock_and_exit:
 
-	acpi_os_release_lock(acpi_gbl_hardware_lock, lock_flags);
+	raw_spin_unlock_irqrestore(&acpi_gbl_hardware_lock, lock_flags);
 	return_ACPI_STATUS(status);
 }
 
drivers/acpi/acpica/utmutex.c +3 -18
index 7d797e2..420eecf 100644
@@ -52,6 +52,9 @@ static acpi_status acpi_ut_create_mutex(acpi_mutex_handle mutex_id);
 
 static void acpi_ut_delete_mutex(acpi_mutex_handle mutex_id);
 
+DEFINE_RAW_SPINLOCK(acpi_gbl_gpe_lock);
+DEFINE_RAW_SPINLOCK(acpi_gbl_hardware_lock);
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ut_mutex_initialize
@@ -81,18 +84,6 @@ acpi_status acpi_ut_mutex_initialize(void)
 		}
 	}
 
-	/* Create the spinlocks for use at interrupt level */
-
-	status = acpi_os_create_lock (&acpi_gbl_gpe_lock);
-	if (ACPI_FAILURE (status)) {
-		return_ACPI_STATUS (status);
-	}
-
-	status = acpi_os_create_lock (&acpi_gbl_hardware_lock);
-	if (ACPI_FAILURE (status)) {
-		return_ACPI_STATUS (status);
-	}
-
 	/* Mutex for _OSI support */
 	status = acpi_os_create_mutex(&acpi_gbl_osi_mutex);
 	if (ACPI_FAILURE(status)) {
@@ -132,13 +123,7 @@ void acpi_ut_mutex_terminate(void)
 
 	acpi_os_delete_mutex(acpi_gbl_osi_mutex);
 
-	/* Delete the spinlocks */
-
-	acpi_os_delete_lock(acpi_gbl_gpe_lock);
-	acpi_os_delete_lock(acpi_gbl_hardware_lock);
-
 	/* Delete the reader/writer lock */
-
 	acpi_ut_delete_rw_lock(&acpi_gbl_namespace_rw_lock);
 	return_VOID;
 }
Signature · 1 line
1.7.7.1
author reviewer
#14 lore raw

PATCH 9/11 slab, lockdep: Fix silly bug

In reply to Steven Rostedt

From: Peter Zijlstra <[email protected]>

Commit 30765b92 ("slab, lockdep: Annotate the locks before using them") moves the init_lock_keys() call from after g_cpucache_up = FULL, to before it. And overlooks the fact that init_node_lock_keys() tests for it and ignores everything !FULL.

Introduce a LATE stage and change the lockdep test to be <LATE.

---
 mm/slab.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
mm/slab.c +4 -1
index 015cd76..433b9a2 100644
@@ -594,6 +594,7 @@ static enum {
 	PARTIAL_AC,
 	PARTIAL_L3,
 	EARLY,
+	LATE,
 	FULL
 } g_cpucache_up;
 
@@ -670,7 +671,7 @@ static void init_node_lock_keys(int q)
 {
 	struct cache_sizes *s = malloc_sizes;
 
-	if (g_cpucache_up != FULL)
+	if (g_cpucache_up < LATE)
 		return;
 
 	for (s = malloc_sizes; s->cs_size != ULONG_MAX; s++) {
@@ -1725,6 +1726,8 @@ void __init kmem_cache_init_late(void)
 {
 	struct kmem_cache *cachep;
 
+	g_cpucache_up = LATE;
+
 	/* Annotate slab for lockdep -- annotate the malloc caches */
 	init_lock_keys();
 
Signature · 1 line
1.7.7.1
#15 lore raw

Re: [PATCH 09/11] slab, lockdep: Fix silly bug

Quoted 13 lines · Steven Rostedt

--00GvhwF7k39YY
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

From: Peter Zijlstra <[email protected]>

Commit 30765b92 ("slab, lockdep: Annotate the locks before using them") moves the init_lock_keys() call from after g_cpucache_up =3D FULL, to before it. And overlooks the fact that init_node_lock_keys() tests for it and ignores everything !FULL.

Introduce a LATE stage and change the lockdep test to be <LATE.

Quoted 12 lines

Your emails seem to be damaged in interesting ways.

I assume the patch is going through the lockdep tree? If so, please make sure you include Christoph's ACK in the changelog.

Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author reviewer
#16 lore raw

Re: [PATCH 09/11] slab, lockdep: Fix silly bug

Quoted 1 line · Pekka Enberg

Your emails seem to be damaged in interesting ways.

Really? I'm using quilt mail to send.

Quoted 3 lines

I assume the patch is going through the lockdep tree? If so, please make sure you include Christoph's ACK in the changelog.

This is for the stable-rt tree. This patch is on its way, but you'll have to talk to Peter about Acks and such.

But this brings up a point. I'll start adding [RT] to the subject of the patches as well to not confuse people.

Thanks,

-- Steve

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author reviewer
#17 lore raw

PATCH 10/11 slab, lockdep: Annotate all slab caches

In reply to Steven Rostedt

From: Peter Zijlstra <[email protected]>

Currently we only annotate the kmalloc caches, annotate all of them.

---
 mm/slab.c |   52 ++++++++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 24 deletions(-)
mm/slab.c +28 -24
index 433b9a2..5251b99 100644
@@ -606,6 +606,12 @@ int slab_is_available(void)
 	return g_cpucache_up >= EARLY;
 }
 
+/*
+ * Guard access to the cache-chain.
+ */
+static DEFINE_MUTEX(cache_chain_mutex);
+static struct list_head cache_chain;
+
 #ifdef CONFIG_LOCKDEP
 
 /*
@@ -667,38 +673,41 @@ static void slab_set_debugobj_lock_classes(struct kmem_cache *cachep)
 		slab_set_debugobj_lock_classes_node(cachep, node);
 }
 
-static void init_node_lock_keys(int q)
+static void init_lock_keys(struct kmem_cache *cachep, int node)
 {
-	struct cache_sizes *s = malloc_sizes;
+	struct kmem_list3 *l3;
 
 	if (g_cpucache_up < LATE)
 		return;
 
-	for (s = malloc_sizes; s->cs_size != ULONG_MAX; s++) {
-		struct kmem_list3 *l3;
+	l3 = cachep->nodelists[node];
+	if (!l3 || OFF_SLAB(cachep))
+		return;
 
-		l3 = s->cs_cachep->nodelists[q];
-		if (!l3 || OFF_SLAB(s->cs_cachep))
-			continue;
+	slab_set_lock_classes(cachep, &on_slab_l3_key, &on_slab_alc_key, node);
+}
 
-		slab_set_lock_classes(s->cs_cachep, &on_slab_l3_key,
-				&on_slab_alc_key, q);
-	}
+static void init_node_lock_keys(int node)
+{
+	struct kmem_cache *cachep;
+
+	list_for_each_entry(cachep, &cache_chain, next)
+		init_lock_keys(cachep, node);
 }
 
-static inline void init_lock_keys(void)
+static inline void init_cachep_lock_keys(struct kmem_cache *cachep)
 {
 	int node;
 
 	for_each_node(node)
-		init_node_lock_keys(node);
+		init_lock_keys(cachep, node);
 }
 #else
-static void init_node_lock_keys(int q)
+static void init_node_lock_keys(int node)
 {
 }
 
-static inline void init_lock_keys(void)
+static void init_cachep_lock_keys(struct kmem_cache *cachep)
 {
 }
 
@@ -711,12 +720,6 @@ static void slab_set_debugobj_lock_classes(struct kmem_cache *cachep)
 }
 #endif
 
-/*
- * Guard access to the cache-chain.
- */
-static DEFINE_MUTEX(cache_chain_mutex);
-static struct list_head cache_chain;
-
 static DEFINE_PER_CPU(struct delayed_work, slab_reap_work);
 static DEFINE_PER_CPU(struct list_head, slab_free_list);
 static DEFINE_LOCAL_IRQ_LOCK(slab_lock);
@@ -1728,14 +1731,13 @@ void __init kmem_cache_init_late(void)
 
 	g_cpucache_up = LATE;
 
-	/* Annotate slab for lockdep -- annotate the malloc caches */
-	init_lock_keys();
-
 	/* 6) resize the head arrays to their final sizes */
 	mutex_lock(&cache_chain_mutex);
-	list_for_each_entry(cachep, &cache_chain, next)
+	list_for_each_entry(cachep, &cache_chain, next) {
+		init_cachep_lock_keys(cachep);
 		if (enable_cpucache(cachep, GFP_NOWAIT))
 			BUG();
+	}
 	mutex_unlock(&cache_chain_mutex);
 
 	/* Done! */
@@ -2546,6 +2548,8 @@ kmem_cache_create (const char *name, size_t size, size_t align,
 		slab_set_debugobj_lock_classes(cachep);
 	}
 
+	init_cachep_lock_keys(cachep);
+
 	/* cache setup completed, link it into the list */
 	list_add(&cachep->next, &cache_chain);
 oops:
Signature · 1 line
1.7.7.1
author reviewer
#18 lore raw

PATCH 11/11 Linux v3.0.12-rt30-rc2

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

---
 localversion-rt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
localversion-rt +1 -1
index 90290c6..6eae1e4 100644
@@ -1 +1 @@
--rt29
+-rt30-rc2
Signature · 1 line
1.7.7.1
#19 lore raw
In reply to Steven Rostedt

Hi Steven

Quoted 3 lines

This is the RT stable review cycle of patch 3.0.12-rt30-rc2.

Please scream at me if I messed something up. Please test the patches too.

Ok, this is a nearly blind shot, but i was expieriencing strange idle time behaviour with this Ingo Molnar patch. The idle time was never incremented as read from /proc/stat for several minutes. The system itself was behaving fine and had definetly not 100% load which was also verified with top.

I hadn't had the time to investigate further and my tests where with an ealier release with the Ingo Molnar Tasklet patch on top.

Hopefully i will find some time to test this stuff further but it will probably tuesday CET.

Best regards
Tim

#20 lore raw
Quoted 8 lines · Tim Sander

Hi Steven

Quoted 3 lines

This is the RT stable review cycle of patch 3.0.12-rt30-rc2.

Please scream at me if I messed something up. Please test the patches too.

Ok, this is a nearly blind shot, but i was expieriencing strange idle time behaviour with this Ingo Molnar patch. The idle time was never incremented as read from /proc/stat for several minutes. The system itself was behaving fine and had definetly not 100% load which was also verified with top.

Did some other time accounting increase instead ?

#21 lore raw

Hi

Quoted 1 line

Did some other time accounting increase instead ?

Yes, the other time seemed to be increasing normally.

I should also point out, that this phenomen was not always happening, but only sometimes and then again disapeared without any known reason.

Best regards
Tim

Please ignore:

Hottinger Baldwin Messtechnik GmbH, Im Tiefen See 45, 64293 Darmstadt, Germany | www.hbm.com

Registered as GmbH (German limited liability corporation) in the commercial register at the local court of Darmstadt, HRB 1147 Company domiciled in Darmstadt | CEO: Andreas Huellhorst | Chairman of the board: James Charles Webster

Als Gesellschaft mit beschraenkter Haftung eingetragen im Handelsregister des Amtsgerichts Darmstadt unter HRB 1147 Sitz der Gesellschaft: Darmstadt | Geschaeftsfuehrung: Andreas Huellhorst | Aufsichtsratsvorsitzender: James Charles Webster

The information in this email is confidential. It is intended solely for the addressee. If you are not the intended recipient, please let me know and delete this email.

Die in dieser E-Mail enthaltene Information ist vertraulich und lediglich für den Empfaenger bestimmt. Sollten Sie nicht der eigentliche Empfaenger sein, informieren Sie mich bitte kurz und loeschen diese E-Mail.

author reviewer
#22 lore raw
Quoted 6 lines · Tim Sander

Hi

Quoted 1 line

Did some other time accounting increase instead ?

Yes, the other time seemed to be increasing normally.

I should also point out, that this phenomen was not always happening, but only sometimes and then again disapeared without any known reason.

Hmm, then are you sure that the tasklet patch causes the bug? Or perhaps it only changes the timing to cause it. The bug may be without the patch too.

-- Steve

#23 lore raw

Hi Steve

Quoted 6 lines
Quoted 2 lines

I should also point out, that this phenomen was not always happening, but only sometimes and then again disapeared without any known reason.

Hmm, then are you sure that the tasklet patch causes the bug? Or perhaps it only changes the timing to cause it. The bug may be without the patch too.

Well, we are running the preempt rt patchset 3.0 since the rt-9 release and we have not seen this kind of error. It just started popping up after i applied the patch which i took directly from the mailing list when it was posted, since we had this ksoftirq issue especially after firmware update when ubifs has been updated.

The bad thing is that it is not happening deterministic but seemingly at random. But i will try your next prerelease and will report any results.

My patch is also not completly vanilla mainline. I am using the recomended patch from ubifs/master, some small platform patches and some timer patches Thomas Gleixner knows best about.

Best regards
Tim

Please ignore:

Hottinger Baldwin Messtechnik GmbH, Im Tiefen See 45, 64293 Darmstadt, Germany | www.hbm.com

Registered as GmbH (German limited liability corporation) in the commercial register at the local court of Darmstadt, HRB 1147 Company domiciled in Darmstadt | CEO: Andreas Huellhorst | Chairman of the board: James Charles Webster

Als Gesellschaft mit beschraenkter Haftung eingetragen im Handelsregister des Amtsgerichts Darmstadt unter HRB 1147 Sitz der Gesellschaft: Darmstadt | Geschaeftsfuehrung: Andreas Huellhorst | Aufsichtsratsvorsitzender: James Charles Webster

The information in this email is confidential. It is intended solely for the addressee. If you are not the intended recipient, please let me know and delete this email.

Die in dieser E-Mail enthaltene Information ist vertraulich und lediglich für den Empfaenger bestimmt. Sollten Sie nicht der eigentliche Empfaenger sein, informieren Sie mich bitte kurz und loeschen diese E-Mail. --
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#24 lore raw
In reply to Tim Sander
Quoted 11 lines · Tim Sander

Hi Steven

Quoted 3 lines

This is the RT stable review cycle of patch 3.0.12-rt30-rc2.

Please scream at me if I messed something up. Please test the patches too.

Ok, this is a nearly blind shot, but i was expieriencing strange idle time behaviour with this Ingo Molnar patch. The idle time was never incremented as read from /proc/stat for several minutes. The system itself was behaving fine and had definetly not 100% load which was also verified with top.

I hadn't had the time to investigate further and my tests where with an ealier release with the Ingo Molnar Tasklet patch on top.

Must be either config related, or something got fixed meanwhile. rt30-rc1..

marge:~ # while sleep 1; do grep cpu /proc/stat|grep -v cpu[0-3]; done

cpu  33066 90 10762 519426 36625 0 597 0 0 0
cpu  33110 90 10792 519643 36723 0 601 0 0 0
cpu  33149 91 10819 519869 36817 0 604 0 0 0
cpu  33189 91 10849 520099 36906 0 609 0 0 0
cpu  33231 92 10879 520329 36994 0 614 0 0 0
cpu  33273 92 10906 520561 37083 0 619 0 0 0
cpu  33312 93 10935 520789 37176 0 624 0 0 0
cpu  33356 93 10963 521017 37264 0 628 0 0 0
cpu  33394 94 10991 521250 37352 0 632 0 0 0
cpu  33435 95 11016 521478 37445 0 634 0 0 0
cpu  33480 96 11040 521700 37539 0 638 0 0 0
cpu  33517 96 11068 521911 37650 0 643 0 0 0
cpu  33551 97 11096 522113 37776 0 647 0 0 0

^C
marge:~ #

#25 lore raw
In reply to Steven Rostedt

On 2011-12-04 13:54 -0500, Steven Rostedt wrote [email protected]...:

Hi, seems there is regression in 3.0.12-rt30-rc1/2 somethere in acpi part:

[ 1961.003200] BUG: scheduling while atomic: irq/9-acpi/14/0x00000002
[ 1961.003206] Modules linked in: cpufreq_conservative aes_i586 aes_generic xt_TCPMSS xt_tcpudp iptable_mangle ip_tables x_tables ipv6 pci_slot pktcdvd tcp_cubic microcode fuse hso pata_pcmcia firewire_net firewire_ohci firewire_core crc_itu_t tcp_hybla cdc_acm usbmouse usb_storage usb_libusual usbhid uhci_hcd ohci_hcd ehci_hcd usbcore snd_mpu401_uart snd_seq_dummy snd_seq_oss snd_seq_midi snd_seq_midi_event snd_seq snd_rawmidi snd_seq_device cpufreq_userspace acpi_cpufreq mperf msr cpuid nvram arc4 joydev mousedev snd_intel8x0m 8250_pci 8250_pnp radeon ttm drm_kms_helper pcmcia drm snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss snd_mixer_oss psmouse snd_pcm ath9k mac80211 ath9k_common ath9k_hw ath backlight i2c_algo_bit i2c_core cfbcopyarea cfbimgblt snd_timer snd_page_alloc yenta_socke
 t cfbfillrect pcmcia_rsrc battery processor button ac pcmcia_core serio_raw pcspkr cfg80211 rfkill 8250 intel_agp intel_gtt agpgart power_supply serial_core evdev raid0 btrfs crc32c libcrc32c thermal sr_mod thermal_sys cdrom 8139too atkbd hwmon [last unloaded: scsi_wait_scan]
[ 1961.003364] Pid: 14, comm: irq/9-acpi Not tainted 3.0.12-rt30-rc2-r233 #1
[ 1961.003370] Call Trace:
[ 1961.003379]  [<792d740d>] ? 0x792d740d
[ 1961.003385]  [<790221f4>] ? 0x790221f4
[ 1961.003391]  [<7902246a>] ? 0x7902246a
[ 1961.003396]  [<79022dc9>] ? 0x79022dc9
[ 1961.003402]  [<792d760f>] ? 0x792d760f
[ 1961.003407]  [<792d8615>] ? 0x792d8615
[ 1961.003414]  [<7918d1d3>] ? 0x7918d1d3
[ 1961.003418]  [<7919de5e>] ? 0x7919de5e
[ 1961.003424]  [<7918d2cf>] ? 0x7918d2cf
[ 1961.003429]  [<791908d4>] ? 0x791908d4
[ 1961.003434]  [<79196bd5>] ? 0x79196bd5
[ 1961.003439]  [<7919e040>] ? 0x7919e040
[ 1961.003444]  [<79196e77>] ? 0x79196e77
[ 1961.003450]  [<791958ff>] ? 0x791958ff
[ 1961.003455]  [<791889c7>] ? 0x791889c7
[ 1961.003460]  [<7905d9f0>] ? 0x7905d9f0
[ 1961.003465]  [<7905d864>] ? 0x7905d864
[ 1961.003470]  [<7905d9d0>] ? 0x7905d9d0
[ 1961.003476]  [<7905d770>] ? 0x7905d770
[ 1961.003481]  [<7903ea94>] ? 0x7903ea94
[ 1961.003486]  [<7903ea20>] ? 0x7903ea20
[ 1961.003491]  [<792d99fe>] ? 0x792d99fe

it's appears periodicaly again and again, while it occurse system freezes about 0.5-1 second then work again fine.

SR>
SR>Dear RT Folks,
SR>
SR>This is the RT stable review cycle of patch 3.0.12-rt30-rc2.
SR>
SR>Please scream at me if I messed something up. Please test the patches too.
SR>
SR>The difference between this and -rc1 was I fixed up the patches to
SR>add the changes, instead of removing them :)
SR>
SR>I also added the missing commit:
SR>KVM: fix XSAVE bit scanning (now properly)
SR>That Thomas pointed out was missing.
SR>
SR>
SR>The -rc release will be uploaded to kernel.org and will be deleted when
SR>the final release is out. This is just a review release (or release candidate). SR>
SR>The pre-releases will not be pushed to the git repository, only the
SR>final release is.
SR>
SR>If all goes well, this patch will be converted to the main release
SR>next on Wednesday (11/7/2011).
SR>
SR>Enjoy,
SR>
SR>-- Steve
SR>
SR>
SR>To build 3.0.12-rt30-rc2 directly, the following patches should be applied:
SR>
SR> http://www.kernel.org/pub/linux/kernel/v3.0/linux-3.0.tar.xz
SR>
SR> http://www.kernel.org/pub/linux/kernel/v3.0/patch-3.0.12.xz
SR>
SR> http://www.kernel.org/pub/linux/kernel/projects/rt/3.0/patch-3.0.12-rt30-rc2.patch.xz
SR>
SR>You can also build from 3.0.12-rt29 by applying the incremental patch:
SR> SR>http://www.kernel.org/pub/linux/kernel/projects/rt/3.0/incr/patch-3.0.12-rt29-rt30-rc2.patch.xz SR>
SR>
SR>Changes from 3.0.12-rt29:
SR>
SR>---
SR>
SR>
SR>Andre Przywara (1):
SR> KVM: fix XSAVE bit scanning (now properly)
SR>
SR>Avi Kivity (1):
SR> KVM: Sanitize cpuid
SR>
SR>Edward Donovan (1):
SR> genirq: fix regression in irqfixup, irqpoll
SR>
SR>Ingo Molnar (1):
SR> tasklet/rt: Prevent tasklets from going into infinite spin in RT
SR>
SR>Peter Zijlstra (2):
SR> slab, lockdep: Fix silly bug
SR> slab, lockdep: Annotate all slab caches
SR>
SR>Roland Dreier (1):
SR> intel-iommu: Fix AB-BA lockdep report
SR>
SR>Steven Rostedt (1):
SR> Linux v3.0.12-rt30-rc2
SR>
SR>Thomas Gleixner (3):

SR>      wait: Provide __wake_up_all_locked
SR>      pci: Use __wake_up_all_locked pci_unblock_user_cfg_access()
SR>      acpi: Make gbl_[hardware|gpe]_lock raw
SR>
SR>----
SR> arch/x86/kvm/x86.c              |   44 +++++++-
SR> drivers/acpi/acpica/acglobal.h  |    4 +-
SR> drivers/acpi/acpica/evgpe.c     |    4 +-
SR> drivers/acpi/acpica/evgpeblk.c  |    8 +-
SR> drivers/acpi/acpica/evgpeutil.c |   12 +-
SR> drivers/acpi/acpica/evxface.c   |   10 +-
SR> drivers/acpi/acpica/evxfgpe.c   |   24 +++---
SR> drivers/acpi/acpica/hwregs.c    |    4 +-
SR> drivers/acpi/acpica/hwxface.c   |    4 +-
SR> drivers/acpi/acpica/utmutex.c   |   21 +----
SR> drivers/pci/access.c            |    2 +-
SR> drivers/pci/intel-iommu.c       |    4 +-
SR> include/linux/interrupt.h       |   39 ++++----
SR> include/linux/wait.h            |    5 +-
SR> kernel/irq/spurious.c           |    4 +-
SR> kernel/sched.c                  |    4 +-
SR> kernel/softirq.c                |  208 ++++++++++++++++++++++++++++-----------
SR> localversion-rt                 |    2 +-
SR> mm/slab.c                       |   55 ++++++-----

SR> 19 files changed, 290 insertions(+), 168 deletions(-) SR>

C уважением With Best Regards
Георгиевский Юрий. Georgiewskiy Yuriy
+7 4872 711666 +7 4872 711666
факс +7 4872 711143 fax +7 4872 711143
Компания ООО "Ай Ти Сервис" IT Service Ltd

http://nkoort.ru                  http://nkoort.ru
JID: [email protected]            JID: [email protected]
YG129-RIPE                        YG129-RIPE
#27 lore raw

On 2011-12-05 17:57 +0100, Thomas Gleixner wrote Georgiewskiy Yuriy:

TG>On Mon, 5 Dec 2011, Georgiewskiy Yuriy wrote:
TG>
TG>> On 2011-12-04 13:54 -0500, Steven Rostedt wrote [email protected]...:
TG>>
TG>> Hi, seems there is regression in 3.0.12-rt30-rc1/2 somethere in acpi part:
TG>
TG>Patch is here: https://lkml.org/lkml/2011/12/3/53
TG>

May be patch is corrupted because it say this while patching:

patch -p1 <11.patch
patching file drivers/acpi/ec.c
patching file drivers/acpi/internal.h
patch unexpectedly ends in middle of line

or just not fix this issue but problem steel present after this patch, and seems i hit another one, i attach full dmesg.

C уважением With Best Regards
Георгиевский Юрий. Georgiewskiy Yuriy
+7 4872 711666 +7 4872 711666
факс +7 4872 711143 fax +7 4872 711143
Компания ООО "Ай Ти Сервис" IT Service Ltd

http://nkoort.ru                  http://nkoort.ru
JID: [email protected]            JID: [email protected]
YG129-RIPE                        YG129-RIPE