1

Robert Love's "Linux Kernel Development" book says that it is required that we pass the flags to the local_irq_save() as a stack variable.

Why is this required? Is it okay to bypass this requirement in x86?

Sam Protsenko
  • 14,045
  • 4
  • 59
  • 75
  • Can you provide the citation with this requirement? Related question: http://stackoverflow.com/questions/34276995/is-returning-while-holding-a-spinlock-automatically-unsafe (about `spin_lock_irqsave` instead of `local_irq_save`). – Tsyvarev Jan 09 '17 at 08:01
  • I don't see how it matters where the flags are stored. It's just a value. Although I often wonder why the various irqsave macros were defined to take an lvalue for the flags. – Ian Abbott Jan 10 '17 at 11:07
  • @Tsyvarev here is the citation "Fast interrupt handlers run with all interrupts disabled on the local processor. This enables a fast handler to complete quickly, without possible interruption from other interrupts. By default (without this flag), all interrupts are enabled except the interrupt lines of any running handlers, which are masked out on all processors. Sans the timer interrupt, most interrupts do not want to enable this flag.Read more at location 2514 • Delete this highlight" – eric johnson Jan 11 '17 at 07:37
  • @Tsyvarev thanks for the link. It does discuss almost a similar problem. Also since spin_lock_irqsave uses local_irq_save, the latter can impose the restriction on the former. My conclusion is that this requirement is s only a sparc specific requirement and it is okay to not care about this in x86. – eric johnson Jan 11 '17 at 07:45
  • I don't see in the provided citation anything about *stack nature* of the flags variable. May be, it is described in some other place? Also, on Stack Overflow it is perfectly OK to **edit the question for improve it**: adding details or providing meaningful clarification. Please, [edit] your question and add the citation into it (instead of comment). – Tsyvarev Jan 11 '17 at 08:22

1 Answers1

1

You probably refer to this quote from LKD3:

local_irq_save(flags);    /* interrupts are now disabled */
/* ... */
local_irq_restore(flags); /* interrupts are restored to their previous state */

Note that these methods are implemented at least in part as macros, so the flags parameter (which must be defined as an unsigned long) is seemingly passed by value. This parameter contains architecture-specific data containing the state of the interrupt systems. Because at least one supported architecture incorporates stack information into the value (ahem, SPARC), flags cannot be passed to another function (specifically, it must remain on the same stack frame). For this reason, the call to save and the call to restore interrupts must occur in the same function.

I don't see any requirements for the flags variable to be declared on stack. What book says is:

  • some architectures (e.g. SPARC) add some stack-specific information to flags variable, when doing local_irq_save(), along with interrupts information
  • so you shouldn't pass flags variable to another function
  • and you should run local_irq_save() / local_irq_restore() in the same stack frame

You must be confused by this statement:

specifically, it must remain on the same stack frame

I'd change it a little bit:

specifically, it must remain on the same stack frame between save/restore calls

More than that, if you do:

$ git grep --all-match -e 'local_irq_save(\*' -- kernel/ include/linux/

you will see that even core kernel code declares flags on heap, sometimes.

As for mentioned implementation on SPARC architecture: book probably refers to this code. So on SPARC architecture the flags variable will contain PSR register, which in turn contains CWP field, and it probably has to do with stack, somehow.

Is it okay to bypass this requirement in x86?

When you are writing architecture-independent code (like drivers), you should consider behavior on all architectures. So when using architecture-independent API, you shouldn't usually think about quirks on some particular platforms. But again, you can declare flags on heap any time you want.

Sam Protsenko
  • 14,045
  • 4
  • 59
  • 75