Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct the descriptions about spin_lock_irqsave and spin_unlock_irqrestore #65

Open
jserv opened this issue Aug 16, 2021 · 2 comments
Open
Assignees

Comments

@jserv
Copy link
Contributor

jserv commented Aug 16, 2021

__next__ at Reddit gave the feedback:

Interrupts can't happen during the lock as the main idea of using spin_lock_irq is to disable (on the logical core, let's use "local CPU" term for that) interrupts.

The whole idea of using spin_lock_irqsave instead of just spin_lock_irq is what is happening during the unlock. At the time spin_lock_irq() is used, interrupts could have been already disabled on the local CPU. And now, when we use spin_unlock_irq() it will re-enable interrupts again for the whole (local) CPU! Yes, it means that our code could have enabled interrupts while someone else (who disabled them before we took the lock) is not expecting that. Example to visualize:

  1. Interrupts are disabled on a local CPU,
  2. We use spin_lock_irq() -> interrupts disabled on a local CPU,
  3. We use spin_unlock_irq() -> interrupts enabled on a local CPU

So in point 3. all interrupts were enabled (on the local CPU) and the one who disabled them in point 1. is not aware of that fact at all. We enabled the interrupts for him. _irqsave variants were introduced to solve this problem.

  1. Interrupts are disabled on a local CPU,
  2. We use spin_lock_irqsave(spinlock_t *lock, unsigned long flags) -> we know that interrupts were disabled at this point as we saved the current state of them in the flags parameter,
  3. We use spin_unlock_irqrestore -> and at this point, we know based on the flags value, that the interrupts were disabled before _irqsave, so we just go back to the state before invoking spin_lock_irqsave instead of always re-enabling interrupts like in the case with spin_unlock_irq().
@jserv jserv self-assigned this Sep 8, 2021
@ashevche
Copy link

I don't think I have got what all means in __next__'s comment "all interrupts were enabled". The CPU has a set of the exceptions one of which is IRQ. And enabling IRQ exception on the CPU side is what's done there. Besides that NMI is by definition can't be handled by this.

@linD026
Copy link
Collaborator

linD026 commented Feb 22, 2023

I think the idea __next__ wants to talk about is the CPU may unconditionally enable the interrupts, including those already disabled before the spin_lock_irq().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants