Search code examples
pointerslinux-kernelstacklinux-device-driverspinlock

Is it allowed to allocate spin locks on the stack?


On Linux (system mode), we usually set up spinlocks with dynamically allocated memory, for example:

spinlock_t *my_lock = kzalloc(sizeof(spinlock_t), GFP_KERNEL);
spin_lock_init(my_lock);

I guess that it is also (likely) possible to set them up without any dynamic allocations, just with on-the-stack space:

char spinspace[sizeof(spinlock_t)];

spinlock_t *my_lock = (void*)&spinspace;
spin_lock_init(my_lock);

It seems to work, but I want to know if it is allowed and safe to use spinlocks that way in my kernel module. That spin lock may eventually be exported or passed to other kernel code.


Solution

  • There is no need to over-complicate the declaration with a char array, you can very well do:

    spinlock_t my_lock;
    spin_lock_init(&my_lock);
    

    The above is valid code.

    You can also use DEFINE_SPINLOCK(my_lock); to define and initialize it at the same time.

    It could be easier to declare a spinlock on the stack, but if you want to share it with some other piece of code that runs concurrently you will have to be careful. Declaring a variable with automatic storage duration inside a function only makes it valid to reference the variable for the lifetime of the function. Once the function returns (or, depending on the case, possibly even earlier than that), that variable will become invalid and so will become any reference you have passed around. So in such case, your function needs to outlive any other users of the spinlock it defines. This is why you see most (if not all) kernel code using kmalloc() or defining spinlocks globally. It is definitely an anti-pattern to do otherwise.

    I only noticed one occurrence of kernel code that does this here in arch/mips/sgi-ip22/ip22-mc.c. In this specific case the spinlock isn't even passed around and it only seems to be used to temporarily disable IRQs, so that's fine, but the usage is nonetheless weird. It seems like it was also not properly initialized, which is bad.