Re: [Minios-devel] [UNIKRAFT PATCH] arch/spinlock: Add spin_lock_irq variants

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hey,

in principle this looks fine except that I would move those variants to 
include/uk/plat/spinlock.h . Logically they are platform dependent because they 
are using ukplat_lcpu_*() calls.
Can you move them and rename them to ukplat_spinlock_*()?

Thanks a lot,

Simon

On 13.03.19, 19:14, "Cristian Banu" <cristb@xxxxxxxxx> wrote:

    Spin_lock_irq* variants are used for synchronizing data that might be
    accessed from within an interrupt handler. Added (void)(lock) to each
    of the spin_lock macros to force the compiler to check the existence
    of the lock.
    
    The DEFINE_SPINLOCK() macro now defines the variable, and thus it can
    be used as `static DEFINE_SPINLOCK(x)`.
    
    Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
    ---
     include/uk/arch/spinlock.h | 38 ++++++++++++++++++++++++++++++++------
     1 file changed, 32 insertions(+), 6 deletions(-)
    
    diff --git a/include/uk/arch/spinlock.h b/include/uk/arch/spinlock.h
    index b7a7501e3bfb..fc9c63fcb936 100644
    --- a/include/uk/arch/spinlock.h
    +++ b/include/uk/arch/spinlock.h
    @@ -26,6 +26,8 @@
     #ifndef __UKARCH_SPINLOCK_H__
     #define __UKARCH_SPINLOCK_H__
     
    +#include <uk/plat/lcpu.h>
    +
     #ifdef __cplusplus
     extern "C" {
     #endif
    @@ -36,14 +38,38 @@ typedef struct {} spinlock_t;
     #error "Define your spinlock operations!"
     #else
     
    -#define ukarch_spin_lock_init(lock)      do {} while (0)
    -#define ukarch_spin_is_locked(lock)      do {} while (0)
    -#define ukarch_spin_lock(lock)           do {} while (0)
    -#define ukarch_spin_trylock(lock)        do {} while (0)
    -#define ukarch_spin_unlock(lock)         do {} while (0)
    +#define ukarch_spin_lock_init(lock)      (void)(lock)
    +#define ukarch_spin_is_locked(lock)      (void)(lock)
    +#define ukarch_spin_lock(lock)           (void)(lock)
    +#define ukarch_spin_trylock(lock)        (void)(lock)
    +#define ukarch_spin_unlock(lock)         (void)(lock)
    +
    +#define ukarch_spin_lock_irq(lock) \
    +   do { \
    +           (void)(lock); \
    +           ukplat_lcpu_disable_irq(); \
    +   } while (0)
    +
    +#define ukarch_spin_unlock_irq(lock) \
    +   do { \
    +           (void)(lock); \
    +           ukplat_lcpu_enable_irq(); \
    +   } while (0)
    +
    +#define ukarch_spin_lock_irqsave(lock, flags) \
    +   do { \
    +           (void)(lock); \
    +           flags = ukplat_lcpu_save_irqf(); \
    +   } while (0)
    +
    +#define ukarch_spin_unlock_irqrestore(lock, flags) \
    +   do { \
    +           (void)(lock); \
    +           ukplat_lcpu_restore_irqf(flags); \
    +   } while (0)
     
     /* Defines a preinitialized spin_lock in unlocked state */
    -#define DEFINE_SPINLOCK(lock)            do {} while (0)
    +#define DEFINE_SPINLOCK(lock)            spinlock_t lock = {}
     
     #endif
     
    -- 
    2.11.0
    
    

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel