CODEWITHSUNDEEP
clinux-kernellinux-device-driver
It'sPete
It'sPete

Reputation: 5211

Linux Device Drivers: Sleeping whiile Holding Lock Revisited

I've always been taught that sleeping while holding a spinlock in kernel code is a no-no. The reason for this is the following scenario:

  1. Thread A acquires the lock, does some work, and calls a kernel function that goes to sleep, releasing the CPU.
  2. Thread B now runs, which attempts to acquire the lock, but can't since its held by Thread A.
  3. This is a deadlock. Thread A can't get woken up since Thread B is constantly spinning and Thread B can't make any progress since it can't get the lock.

I am maintaining a driver which uses lots of locks and inside some of the locked sections are obvious things like memory allocations, copy_to_user(), etc.

However, I am not entirely convinced that I have a bug on my hands. Using the above scenario, Thread A is user context (namely inside the implementation of read()) while Thread B is the interrupt context (inside the ISR). The lock is locked via spin_lock_irqsave(). As a result, Thread B cannot run while Thread A holds the lock, making the deadlock not possible.

I have also considered the following:

  1. Thread A = interrupt context. Here Thread A cannot sleep (and doesn't) so we never get into the deadlock condition.
  2. Thread A and B are both user context (i.e., concurrent calls to read() ). This cannot happen due to other mechanisms in place.

Is there anything that I am missing? In what I described above, is there any real danger related to sleeping while holding the lock?

Upvotes: 2

Views: 798

Answers (2)

Tsyvarev
Tsyvarev

Reputation: 65928

I've always been taught that sleeping while holding a spinlock in kernel code is a no-no.

You confuse spinlocks and disabled interrupts (atomic context, IRQ).

  1. Just sleeping while having a spinlock (acquired with spin_lock) harms perfomance, because the other thread, wanting to lock the spinlock, wastes a time with busy wait. But otherwise the system is OK.

  2. But sleeping with disabled interrupts means the CPU core is dead: it executes nothing and doesn't react on interrupts from the other cores and outer world.

    This is what would happen, if your call spin_lock_irqsave followed by copy_from_user: the first operations disables interrupts, and the second may sleep.

Usually disabled interrupts accompanies spinlocks (by means of spin_lock_irqsave or similar). Otherwise, if IRQ thread will attempt to acquire the same spinlock, deadlock will be observed: owner thread cannot proceed because it is preempted, and IRQ thread cannot proceed because it waits on spinlock.

Disabling interrupts is not needed for taking some spinlock, if no IRQ thread(or a thread with interrupts disabled for other reason) locks given spinlock. When disabling interrupts is not needed, spin_lock can be used instead of spin_lock_irqsave. But it is very recommended to replace non-IRQ spinlocks with mutexes.

Upvotes: 3

user4822941
user4822941

Reputation:

The question does not add up.

Using the above scenario, Thread A is user context (namely inside the implementation of read()) while Thread B is the interrupt context (inside the ISR). The lock is locked via spin_lock_irqsave(). As a result, Thread B cannot run while Thread A holds the lock, making the deadlock not possible.

If you hold the spinlock across copy_to_user & friends, the kernel with debug will warn you that you are doing it wrong. Should the need to put the thread to sleep occur, it will go to sleep. Then you are back to square one.

Upvotes: 1

Related Questions