irq_work: improve the flag definitions
PATCH 1/1 irq_work: improve the flag definitions
IRQ_WORK_FLAGS is defined simply to 3UL. This is confusing as it says nothing about its purpose. Define IRQ_WORK_FLAGS as a bitwise OR of IRQ_WORK_PENDING and IRQ_WORK_BUSY.
While we're at it: use the BIT() macro for all flags.
--- include/linux/irq_work.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
include/linux/irq_work.h +4 -4
@@ -12,10 +12,10 @@* busy NULL, 2 -> {free, claimed} : callback in progress, can be claimed */ -#define IRQ_WORK_PENDING 1UL -#define IRQ_WORK_BUSY 2UL -#define IRQ_WORK_FLAGS 3UL -#define IRQ_WORK_LAZY 4UL/* Doesn't want IPI, wait for tick */ +#define IRQ_WORK_PENDING BIT(0) +#define IRQ_WORK_BUSY BIT(1) +#define IRQ_WORK_FLAGS (IRQ_WORK_PENDING | IRQ_WORK_BUSY) +#define IRQ_WORK_LAZY BIT(3)/* Doesn't want IPI, wait for tick */ struct irq_work {unsigned long flags;
Signature · 1 line
2.13.2
include/linux/irq_work.h line 17
Quoted 9 lines · Bartosz Golaszewski
IRQ_WORK_FLAGSis defined simply to 3UL. This is confusing as it says nothing about its purpose. DefineIRQ_WORK_FLAGSas a bitwise OR ofIRQ_WORK_PENDINGandIRQ_WORK_BUSY.While we're at it: use the
BIT()macro for all flags.include/linux/irq_work.h +3
-0+#define IRQ_WORK_PENDING BIT(0) +#define IRQ_WORK_BUSY BIT(1) +#define IRQ_WORK_FLAGS (IRQ_WORK_PENDING | IRQ_WORK_BUSY)
I dunno which style is preferred, though I would go with simple GENMASK() here, as all definitions right on left :-)
Parameters of GENMASK will show last-first entries and can be easily decoded. Although there are only two for now.
Signature · 2 lines
With Best Regards, Andy Shevchenko
2017-08-14 14:19 GMT+02:00 Andy Shevchenko <[email protected]>:
Quoted 14 lines · Bartosz Golaszewski
include/linux/irq_work.h line 17
Quoted 9 lines
IRQ_WORK_FLAGSis defined simply to 3UL. This is confusing as it says nothing about its purpose. DefineIRQ_WORK_FLAGSas a bitwise OR ofIRQ_WORK_PENDINGandIRQ_WORK_BUSY.While we're at it: use the
BIT()macro for all flags.include/linux/irq_work.h +3
-0+#define IRQ_WORK_PENDING BIT(0) +#define IRQ_WORK_BUSY BIT(1) +#define IRQ_WORK_FLAGS (IRQ_WORK_PENDING | IRQ_WORK_BUSY)I dunno which style is preferred, though I would go with simple
GENMASK()here, as all definitions right on left :-)
Quoted 3 lines
Parameters of GENMASK will show last-first entries and can be easily decoded. Although there are only two for now.
Which is a good enough reason to not over-complicate things and just use an easy to decipher bitwise OR. ;)
Thanks,
Bartosz
2017-08-14 13:56 GMT+02:00 Bartosz Golaszewski <[email protected]>:
Quoted 17 lines
IRQ_WORK_FLAGSis defined simply to 3UL. This is confusing as it says nothing about its purpose. DefineIRQ_WORK_FLAGSas a bitwise OR ofIRQ_WORK_PENDINGandIRQ_WORK_BUSY.While we're at it: use the
BIT()macro for all flags.Signed-off-by Bartosz Golaszewski <[email protected]>--- include/linux/irq_work.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)include/linux/irq_work.h +0
-0@@ -12,10 +12,10 @@* busy NULL, 2 -> {free, claimed} : callback in progress, can be claimed
include/linux/irq_work.h line 18
Quoted 10 lines
include/linux/irq_work.h +4
-4*/ -#define IRQ_WORK_PENDING 1UL -#define IRQ_WORK_BUSY 2UL -#define IRQ_WORK_FLAGS 3UL -#define IRQ_WORK_LAZY 4UL/* Doesn't want IPI, wait for tick */ +#define IRQ_WORK_PENDING BIT(0) +#define IRQ_WORK_BUSY BIT(1) +#define IRQ_WORK_FLAGS (IRQ_WORK_PENDING | IRQ_WORK_BUSY) +#define IRQ_WORK_LAZY BIT(3)/* Doesn't want IPI, wait for tick */
Superseded by v2 - 4UL is BIT(2), not BIT(3).
Best regards,
Bartosz Golaszewski