BUG: sleeping function called from invalid context at kernel/mutex.c

I've got following warnings when I'm trying to resume from suspend, so
sparse_lock is not safe enough as a mutex for the suspend resume case, can we simply convert sparse_lock back to spinlock?

And it is safe enough for the resume case as well, it simply cannot go to sleep because nothing else can hold that mutex in the early resume code. We take the mutex during early boot as well, but there the might sleep checks are disabled. And we should do the very same thing in the early resume code as well. Rafael, PeterZ ???

Quoted 6 lines · Thomas Gleixner

And it is safe enough for the resume case as well, it simply cannot go to sleep because nothing else can hold that mutex in the early resume code. We take the mutex during early boot as well, but there the might sleep checks are disabled. And we should do the very same thing in the early resume code as well. Rafael, PeterZ ???

the $subject text sound like it triggered might_sleep(), and that had a system_state != SYSTEM_RUNNING bail condition, but then, I've no clue what resume looks like.

Quoted 3 lines

the $subject text sound like it triggered might_sleep(), and that had a system_state != SYSTEM_RUNNING bail condition, but then, I've no clue what resume looks like.

Early resume looks pretty much like the system startup, e.g. everything called from syscore_ops should not be sleepable (although mutexes shouldn't trigger, because that code is effectively single-threaded, unless somebody holds the mutex in question when that code is being executed, but that would deadlock anyway).

Quoted 3 lines · Xiaotian Feng

I've got following warnings when I'm trying to resume from suspend, so
sparse_lock is not safe enough as a mutex for the suspend resume case, can we simply convert sparse_lock back to spinlock?

What the heck? We do not just convert a mutex to a spinlock because a warning triggers. And it was a mutex already in 2.6.38 w/o any such problems.

And it is safe enough for the resume case as well, it simply cannot go to sleep because nothing else can hold that mutex in the early resume code. We take the mutex during early boot as well, but there the might sleep checks are disabled. And we should do the very same thing in the early resume code as well. Rafael, PeterZ ???

But that's a different issue and not the real problem.

So now instead of thinking about spinlock it would have been pretty simple to figure out that the warning is triggered due to commit d72274e5.

But of course we dont want to revert d72274e5 either, though it's easy to figure out from the call chain that the fundamental stupidity is in the hpet resume code.

Why the hell does it call arch_setup_hpet_msi()? That interrupt was completely setup _BEFORE_ we went into suspend. And I don't see any code which tears down the interrupt on suspend. So we run through a full setup for nothing. And when we have interrupt remapping enabled it even leaks an irte, because it allocates a new one. Utter crap. init_one_hpet_msi_clockevent() has the same stupid call.

So no, the mutex stays a mutex and we fix crap where the crap is.

The following completely untested patch should solve that. Actually we should be more clever and do the affinity magic in the new function as well, so we can get rid of this weird disable/set_affinity/enable hack, but that's a different story.

Thanks,

tglx

---
  arch/x86/include/asm/hpet.h    |    2 ++
  arch/x86/kernel/apic/io_apic.c |   11 +++++++++++
  arch/x86/kernel/hpet.c         |    5 +++--
  3 files changed, 16 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/include/asm/hpet.h
===================================================================

arch/x86/include/asm/hpet.h +2 -0

@@ -83,11 +83,13 @@ extern void hpet_msi_read(struct hpet_de
 
  #ifdef CONFIG_PCI_MSI
  extern int arch_setup_hpet_msi(unsigned int irq, unsigned int id);
+extern int arch_start_hpet_msi(unsigned int irq, unsigned int id);
  #else
  static inline int arch_setup_hpet_msi(unsigned int irq, unsigned int id)
  {
  	return -EINVAL;
  }
+static inline int arch_start_hpet_msi(unsigned int irq, unsigned int id) { }
  #endif
 
  #ifdef CONFIG_HPET_EMULATE_RTC

Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================

arch/x86/kernel/apic/io_apic.c +11 -0

@@ -3477,6 +3477,17 @@ int arch_setup_hpet_msi(unsigned int irq
  	irq_set_chip_and_handler_name(irq, chip, handle_edge_irq, "edge");
  	return 0;
  }
+
+int arch_start_hpet_msi(unsigned int irq, unsigned int id)
+{
+	struct msi_msg msg;
+	int ret = msi_compose_msg(NULL, irq,&msg, id);
+
+	if (!ret)
+		hpet_msi_write(irq_get_handler_data(irq),&msg);
+	return ret;
+}
+
  #endif
 
  #endif /* CONFIG_PCI_MSI */

Index: linux-2.6/arch/x86/kernel/hpet.c
===================================================================

arch/x86/kernel/hpet.c +3 -2

@@ -370,7 +370,8 @@ static void hpet_set_mode(enum clock_eve
  			hpet_enable_legacy_int();
  		} else {
  			struct hpet_dev *hdev = EVT_TO_HPET_DEV(evt);
-			hpet_setup_msi_irq(hdev->irq);
+
+			arch_start_hpet_msi(hdev->irq, hpet_blockid);
  			disable_irq(hdev->irq);
  			irq_set_affinity(hdev->irq, cpumask_of(hdev->cpu));
  			enable_irq(hdev->irq);

@@ -555,7 +556,7 @@ static void init_one_hpet_msi_clockevent
  	if (!(hdev->flags&  HPET_DEV_VALID))
  		return;
 
-	if (hpet_setup_msi_irq(hdev->irq))
+	if (arch_start_hpet_msi(hdev->irq, hpet_blockid))
  		return;
 
  	hdev->cpu = cpu;

Quoted 4 lines · Rafael J. Wysocki

Quoted 3 lines

the $subject text sound like it triggered might_sleep(), and that had a system_state != SYSTEM_RUNNING bail condition, but then, I've no clue what resume looks like.

Quoted 5 lines

Early resume looks pretty much like the system startup, e.g. everything called from syscore_ops should not be sleepable (although mutexes shouldn't trigger, because that code is effectively single-threaded, unless somebody holds the mutex in question when that code is being executed, but that would deadlock anyway).

Right, so system_state != SYSTEM_RUNNING should be true for resume?