Re: [PATCH] lock: Add doc comments for spin_lock_irq()
From: Waiman Long
Date: Sun Oct 12 2025 - 19:17:57 EST
On 10/12/25 9:48 AM, Daroc Alden wrote:
On Sat, 11 Oct 2025 22:31:17 -0400I believe the above description is correct.
Waiman Long <llong@xxxxxxxxxx> wrote:
On 10/11/25 2:28 PM, Daroc Alden wrote:Ah, okay!
On Fri, 10 Oct 2025 23:15:50 -0400"bh" in spin_lock_bh() stands for bottom half which is essentially
Waiman Long <llong@xxxxxxxxxx> wrote:
On 10/10/25 5:53 PM, Daroc Alden wrote:Because I had to research spin_lock_irq()/spin_lock_irqsave() for a
The commonly used spin_lock_irq(), spin_lock_irqsave(),This patch looks fine. Just wonder why just
spin_unlock_irq(), and spin_unlock_irqrestore() functions do not
currently have any documentation; this commit adds kerneldoc
comments to these four functions describing when their behavior
and when they are appropriate to use.
Signed-off-by: Daroc Alden <daroc@xxxxxxx>
spin_lock_irq()/spin_lock_irqsave() and not
spin_lock()/spin_lock_bh() as these functions also don't have
kerneldoc comments. Also spin_lock_irqsave() is a macro and not
actually a function, maybe we should mention that in the comment.
recent article, and therefore felt confident that I understood how
they behaved and what should go in the doc comment.
If you — as a more experienced kernel person — can describe how/why
the _bh() variants are used, I'm happy to add doc comments for them
as well. My current understanding is that they interact with
whatever is left of the "big kernel lock". Is that right?
what what is being done in the softIRQ context. So spin_lock_bh()
just prevents the softIRQ code from being executed. This is my
understanding, but I may have missed other use cases of
spin_lock_bh(). Others can chime in if there is more to say. Anyway,
I am fine with adding more comments to spinlock code.
I went and read some of the existing locking documentation with that
context in mind, and I think I understand. I think the doc comment
should look something like this:
/**
* spin_lock_bh() - Disable softIRQs and take the provided spinlock.
* @lock: The spinlock to acquire.
*
* When data is shared between code that can run in process context and
* code that can run in a softIRQ, if the softIRQ tries to acquire a
* spinlock that is already held, the system could deadlock. This
* function disables softIRQs before taking the provided spinlock. It
* should typically be paired with a call to spin_unlock_bh() in order
* to reenable softIRQs when the lock is released.
*
* If the interrupt code can run as a hard interrupt instead of a soft
* interrupt, this is the wrong function: use spin_lock_irqsave(). If in
* doubt, using spin_lock_irqsave() instead of spin_lock_bh() is always
* permissible, since the former is a superset of the latter.
*
* If synchronizing between a tasklet or timer and a softIRQ, the plain
* spin_lock() function can be used, because these are not interrupted
* by softIRQs on the same CPU.
*/
Tasklets and timer handling are run in softIRQ context. SoftIRQs are sub-divided into a number of priority levels (see include/linux/interrupt.h) from high to low, they are not going to interrupt each other.
Cheers,
Longman