Skip to contentSkip to conversation

irq_work: improve the flag definitions

PATCH RESEND In review Bartosz Golaszewski 3 participants 5 messages Nov 27, 2017 – Dec 11, 2017

v1 of 3 In review Latest is v3 →

lore
Share & tools
lore

Bartosz Golaszewski

author
#0 lore raw

PATCH RESEND 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 and change its name to IRQ_WORK_CLAIMED.

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 ++++----
 kernel/irq_work.c        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
include/linux/irq_work.h +4 -4
index 0e81035b678f..de6333967d61 100644
@@ -13,10 +13,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_CLAIMED	(IRQ_WORK_PENDING | IRQ_WORK_BUSY)
+#define IRQ_WORK_LAZY		BIT(2) /* Doesn't want IPI, wait for tick */
 
 struct irq_work {
 	unsigned long flags;
kernel/irq_work.c +1 -1
index 40e9d739c169..6b7cdf17ccf8 100644
@@ -36,7 +36,7 @@ static bool irq_work_claim(struct irq_work *work)
 	 */
 	flags = work->flags & ~IRQ_WORK_PENDING;
 	for (;;) {
-		nflags = flags | IRQ_WORK_FLAGS;
+		nflags = flags | IRQ_WORK_CLAIMED;
 		oflags = cmpxchg(&work->flags, flags, nflags);
 		if (oflags == flags)
 			break;
Signature · 1 line
2.15.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Bartosz Golaszewski

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 and change its name to IRQ_WORK_CLAIMED.

While we're at it: use the BIT() macro for all flags.

Reviewed-by Andy Shevchenko <[email protected]>
Quoted 41 lines
Signed-off-by Bartosz Golaszewski <[email protected]>
---
 include/linux/irq_work.h | 8 ++++----
 kernel/irq_work.c        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
include/linux/irq_work.h +4 -4
index 0e81035b678f..de6333967d61 100644
@@ -13,10 +13,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_CLAIMED       (IRQ_WORK_PENDING | IRQ_WORK_BUSY)
+#define IRQ_WORK_LAZY          BIT(2) /* Doesn't want IPI, wait for tick */
 
 struct irq_work {
        unsigned long flags;
kernel/irq_work.c +1 -2
index 40e9d739c169..6b7cdf17ccf8 100644
@@ -36,7 +36,7 @@ static bool irq_work_claim(struct irq_work *work)
         */
        flags = work->flags & ~IRQ_WORK_PENDING;
        for (;;) {
-               nflags = flags | IRQ_WORK_FLAGS;
+               nflags = flags | IRQ_WORK_CLAIMED;
                oflags = cmpxchg(&work->flags, flags, nflags);
                if (oflags == flags)
                        break;
--

2.15.0

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#2 lore raw
In reply to Bartosz Golaszewski

2017-11-27 11:52 GMT+01:00 Bartosz Golaszewski <[email protected]>:

Quoted 46 lines

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 and change its name to IRQ_WORK_CLAIMED.

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 ++++----
 kernel/irq_work.c        | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
include/linux/irq_work.h +4 -4
index 0e81035b678f..de6333967d61 100644
@@ -13,10 +13,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_CLAIMED       (IRQ_WORK_PENDING | IRQ_WORK_BUSY)
+#define IRQ_WORK_LAZY          BIT(2) /* Doesn't want IPI, wait for tick */
 
 struct irq_work {
        unsigned long flags;
kernel/irq_work.c +1 -2
index 40e9d739c169..6b7cdf17ccf8 100644
@@ -36,7 +36,7 @@ static bool irq_work_claim(struct irq_work *work)
         */
        flags = work->flags & ~IRQ_WORK_PENDING;
        for (;;) {
-               nflags = flags | IRQ_WORK_FLAGS;
+               nflags = flags | IRQ_WORK_CLAIMED;
                oflags = cmpxchg(&work->flags, flags, nflags);
                if (oflags == flags)
                        break;
--
Quoted 2 lines

2.15.0

It's been a couple months since I first posted this patch. Can it be applied for 4.16?

@Frederic: I noticed you picked up some irq_work patches in the past so I'm Cc'ing you.

Best regards,
Bartosz Golaszewski

#3 lore raw

2017-12-11 11:59 UTC+01:00, Bartosz Golaszewski <[email protected]>:

Quoted 6 lines

2017-11-27 11:52 GMT+01:00 Bartosz Golaszewski <[email protected]>:
It's been a couple months since I first posted this patch. Can it be applied for 4.16?

@Frederic: I noticed you picked up some irq_work patches in the past so I'm Cc'ing you.

Sure, I'll take it!

Thanks!

Acked-by on v1 1/1
#4 lore raw
In reply to Bartosz Golaszewski

2017-11-27 11:52 UTC+01:00, Bartosz Golaszewski <[email protected]>:

Quoted 8 lines

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 and change its name to IRQ_WORK_CLAIMED.

While we're at it: use the BIT() macro for all flags.

Signed-off-by Bartosz Golaszewski <[email protected]>

Nice!

Acked-by Frederic Weisbecker <[email protected]>