From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Dave Airlie <airlied@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
Tejun Heo <tj@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Nathan Chancellor <nathan@kernel.org>,
Sasha Levin <sashal@kernel.org>,
llvm@lists.linux.dev
Subject: [PATCH AUTOSEL 6.3 17/17] workqueue: clean up WORK_* constant types, clarify masking
Date: Thu, 29 Jun 2023 15:00:46 -0400 [thread overview]
Message-ID: <20230629190049.907558-17-sashal@kernel.org> (raw)
In-Reply-To: <20230629190049.907558-1-sashal@kernel.org>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit afa4bb778e48d79e4a642ed41e3b4e0de7489a6c ]
Dave Airlie reports that gcc-13.1.1 has started complaining about some
of the workqueue code in 32-bit arm builds:
kernel/workqueue.c: In function ‘get_work_pwq’:
kernel/workqueue.c:713:24: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
713 | return (void *)(data & WORK_STRUCT_WQ_DATA_MASK);
| ^
[ ... a couple of other cases ... ]
and while it's not immediately clear exactly why gcc started complaining
about it now, I suspect it's some C23-induced enum type handlign fixup in
gcc-13 is the cause.
Whatever the reason for starting to complain, the code and data types
are indeed disgusting enough that the complaint is warranted.
The wq code ends up creating various "helper constants" (like that
WORK_STRUCT_WQ_DATA_MASK) using an enum type, which is all kinds of
confused. The mask needs to be 'unsigned long', not some unspecified
enum type.
To make matters worse, the actual "mask and cast to a pointer" is
repeated a couple of times, and the cast isn't even always done to the
right pointer, but - as the error case above - to a 'void *' with then
the compiler finishing the job.
That's now how we roll in the kernel.
So create the masks using the proper types rather than some ambiguous
enumeration, and use a nice helper that actually does the type
conversion in one well-defined place.
Incidentally, this magically makes clang generate better code. That,
admittedly, is really just a sign of clang having been seriously
confused before, and cleaning up the typing unconfuses the compiler too.
Reported-by: Dave Airlie <airlied@gmail.com>
Link: https://lore.kernel.org/lkml/CAPM=9twNnV4zMCvrPkw3H-ajZOH-01JVh_kDrxdPYQErz8ZTdA@mail.gmail.com/
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/linux/workqueue.h | 15 ++++++++-------
kernel/workqueue.c | 13 ++++++++-----
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index ac551b8ee7d9f..d3e75a19e40f4 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -68,7 +68,6 @@ enum {
WORK_OFFQ_FLAG_BASE = WORK_STRUCT_COLOR_SHIFT,
__WORK_OFFQ_CANCELING = WORK_OFFQ_FLAG_BASE,
- WORK_OFFQ_CANCELING = (1 << __WORK_OFFQ_CANCELING),
/*
* When a work item is off queue, its high bits point to the last
@@ -79,12 +78,6 @@ enum {
WORK_OFFQ_POOL_SHIFT = WORK_OFFQ_FLAG_BASE + WORK_OFFQ_FLAG_BITS,
WORK_OFFQ_LEFT = BITS_PER_LONG - WORK_OFFQ_POOL_SHIFT,
WORK_OFFQ_POOL_BITS = WORK_OFFQ_LEFT <= 31 ? WORK_OFFQ_LEFT : 31,
- WORK_OFFQ_POOL_NONE = (1LU << WORK_OFFQ_POOL_BITS) - 1,
-
- /* convenience constants */
- WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
- WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
- WORK_STRUCT_NO_POOL = (unsigned long)WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT,
/* bit mask for work_busy() return values */
WORK_BUSY_PENDING = 1 << 0,
@@ -94,6 +87,14 @@ enum {
WORKER_DESC_LEN = 24,
};
+/* Convenience constants - of type 'unsigned long', not 'enum'! */
+#define WORK_OFFQ_CANCELING (1ul << __WORK_OFFQ_CANCELING)
+#define WORK_OFFQ_POOL_NONE ((1ul << WORK_OFFQ_POOL_BITS) - 1)
+#define WORK_STRUCT_NO_POOL (WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT)
+
+#define WORK_STRUCT_FLAG_MASK ((1ul << WORK_STRUCT_FLAG_BITS) - 1)
+#define WORK_STRUCT_WQ_DATA_MASK (~WORK_STRUCT_FLAG_MASK)
+
struct work_struct {
atomic_long_t data;
struct list_head entry;
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 2be9b0ecf22ce..7da2a38f9a54b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -701,12 +701,17 @@ static void clear_work_data(struct work_struct *work)
set_work_data(work, WORK_STRUCT_NO_POOL, 0);
}
+static inline struct pool_workqueue *work_struct_pwq(unsigned long data)
+{
+ return (struct pool_workqueue *)(data & WORK_STRUCT_WQ_DATA_MASK);
+}
+
static struct pool_workqueue *get_work_pwq(struct work_struct *work)
{
unsigned long data = atomic_long_read(&work->data);
if (data & WORK_STRUCT_PWQ)
- return (void *)(data & WORK_STRUCT_WQ_DATA_MASK);
+ return work_struct_pwq(data);
else
return NULL;
}
@@ -734,8 +739,7 @@ static struct worker_pool *get_work_pool(struct work_struct *work)
assert_rcu_or_pool_mutex();
if (data & WORK_STRUCT_PWQ)
- return ((struct pool_workqueue *)
- (data & WORK_STRUCT_WQ_DATA_MASK))->pool;
+ return work_struct_pwq(data)->pool;
pool_id = data >> WORK_OFFQ_POOL_SHIFT;
if (pool_id == WORK_OFFQ_POOL_NONE)
@@ -756,8 +760,7 @@ static int get_work_pool_id(struct work_struct *work)
unsigned long data = atomic_long_read(&work->data);
if (data & WORK_STRUCT_PWQ)
- return ((struct pool_workqueue *)
- (data & WORK_STRUCT_WQ_DATA_MASK))->pool->id;
+ return work_struct_pwq(data)->pool->id;
return data >> WORK_OFFQ_POOL_SHIFT;
}
--
2.39.2
prev parent reply other threads:[~2023-06-29 19:03 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-29 19:00 [PATCH AUTOSEL 6.3 01/17] arm64: dts: rockchip: fix USB regulator on ROCK64 Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 02/17] arm64: dts: rockchip: add missing cache properties Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 03/17] tracing/user_events: Prevent same name but different args event Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 04/17] tracing/user_events: Handle matching arguments that is null from dyn_events Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 05/17] tracing/user_events: Fix the incorrect trace record for empty arguments events Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 06/17] x86/hyperv: Fix hyperv_pcpu_input_arg handling when CPUs go online/offline Sasha Levin
2023-06-29 22:17 ` Michael Kelley (LINUX)
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 07/17] arm64/hyperv: Use CPUHP_AP_HYPERV_ONLINE state to fix CPU online sequencing Sasha Levin
2023-06-29 22:18 ` Michael Kelley (LINUX)
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 08/17] ieee802154/adf7242: Add MODULE_FIRMWARE macro Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 09/17] nfc: fdp: Add MODULE_FIRMWARE macros Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 10/17] ALSA: hda/realtek: Add quirk for ASUS ROG G634Z Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 11/17] net: dpaa2-mac: add 25gbase-r support Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 12/17] drm: use mgr->dev in drm_dbg_kms in drm_dp_add_payload_part2 Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 13/17] netfilter: nf_tables: disallow timeout for anonymous sets Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 14/17] netfilter: nf_tables: drop module reference after updating chain Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 15/17] Revert "virtio-blk: support completion batching for the IRQ path" Sasha Levin
2023-06-29 19:00 ` [PATCH AUTOSEL 6.3 16/17] ALSA: hda/realtek: Add quirk for ASUS ROG GV601V Sasha Levin
2023-06-29 19:00 ` Sasha Levin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230629190049.907558-17-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=airlied@gmail.com \
--cc=arnd@arndb.de \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=stable@vger.kernel.org \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox