| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4C4838B8.1080400@kernel.org>
Date: Thu, 22 Jul 2010 14:25:28 +0200
From: Tejun Heo <tj@...nel.org>
To: Martin Schwidefsky <schwidefsky@...ibm.com>
CC: Sachin Sant <sachinp@...ibm.com>, linux-s390@...r.kernel.org,
linux-kernel <linux-kernel@...r.kernel.org>,
"linux-next@...r.kernel.org" <linux-next@...r.kernel.org>
Subject: [PATCH wq#for-next] workqueue: fix how cpu number is stored in work->data
Once a work starts execution, its data contains the cpu number it was
on instead of pointing to cwq. This is added by commit 7a22ad75
(workqueue: carry cpu number in work data once execution starts) to
reliably determine the work was last on even if the workqueue itself
was destroyed inbetween.
Whether data points to a cwq or contains a cpu number was
distinguished by comparing the value against PAGE_OFFSET. The
assumption was that a cpu number should be below PAGE_OFFSET while a
pointer to cwq should be above it. However, on architectures which
use separate address spaces for user and kernel spaces, this doesn't
hold as PAGE_OFFSET is zero.
Fix it by using an explicit flag, WORK_STRUCT_CWQ, to mark what the
data field contains. If the flag is set, it's pointing to a cwq;
otherwise, it contains a cpu number.
Reported on s390 and microblaze during linux-next testing.
Signed-off-by: Tejun Heo <tj@...nel.org>
Reported-by: Sachin Sant <sachinp@...ibm.com>
Reported-by: Michal Simek <michal.simek@...alogix.com>
Reported-by: Martin Schwidefsky <schwidefsky@...ibm.com>
---
Yeah, that was a stupid assumption by me. Can you guys please test
whether this fixes the problem?
Thanks.
include/linux/workqueue.h | 14 ++++++++------
kernel/workqueue.c | 36 +++++++++++++-----------------------
2 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index d74a529..5f76001 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -25,17 +25,19 @@ typedef void (*work_func_t)(struct work_struct *work);
enum {
WORK_STRUCT_PENDING_BIT = 0, /* work item is pending execution */
- WORK_STRUCT_LINKED_BIT = 1, /* next work is linked to this one */
+ WORK_STRUCT_CWQ_BIT = 1, /* data points to cwq */
+ WORK_STRUCT_LINKED_BIT = 2, /* next work is linked to this one */
#ifdef CONFIG_DEBUG_OBJECTS_WORK
- WORK_STRUCT_STATIC_BIT = 2, /* static initializer (debugobjects) */
- WORK_STRUCT_COLOR_SHIFT = 3, /* color for workqueue flushing */
+ WORK_STRUCT_STATIC_BIT = 3, /* static initializer (debugobjects) */
+ WORK_STRUCT_COLOR_SHIFT = 4, /* color for workqueue flushing */
#else
- WORK_STRUCT_COLOR_SHIFT = 2, /* color for workqueue flushing */
+ WORK_STRUCT_COLOR_SHIFT = 3, /* color for workqueue flushing */
#endif
WORK_STRUCT_COLOR_BITS = 4,
WORK_STRUCT_PENDING = 1 << WORK_STRUCT_PENDING_BIT,
+ WORK_STRUCT_CWQ = 1 << WORK_STRUCT_CWQ_BIT,
WORK_STRUCT_LINKED = 1 << WORK_STRUCT_LINKED_BIT,
#ifdef CONFIG_DEBUG_OBJECTS_WORK
WORK_STRUCT_STATIC = 1 << WORK_STRUCT_STATIC_BIT,
@@ -56,8 +58,8 @@ enum {
WORK_CPU_LAST = WORK_CPU_NONE,
/*
- * Reserve 6 bits off of cwq pointer w/ debugobjects turned
- * off. This makes cwqs aligned to 64 bytes which isn't too
+ * Reserve 7 bits off of cwq pointer w/ debugobjects turned
+ * off. This makes cwqs aligned to 128 bytes which isn't too
* excessive while allowing 15 workqueue flush colors.
*/
WORK_STRUCT_FLAG_BITS = WORK_STRUCT_COLOR_SHIFT +
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index c11edc9..e5cb7fa 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -468,10 +468,9 @@ static int work_next_color(int color)
}
/*
- * Work data points to the cwq while a work is on queue. Once
- * execution starts, it points to the cpu the work was last on. This
- * can be distinguished by comparing the data value against
- * PAGE_OFFSET.
+ * A work's data points to the cwq with WORK_STRUCT_CWQ set while the
+ * work is on queue. Once execution starts, WORK_STRUCT_CWQ is
+ * cleared and the work data contains the cpu number it was last on.
*
* set_work_{cwq|cpu}() and clear_work_data() can be used to set the
* cwq, cpu or clear work->data. These functions should only be
@@ -494,7 +493,7 @@ static void set_work_cwq(struct work_struct *work,
unsigned long extra_flags)
{
set_work_data(work, (unsigned long)cwq,
- WORK_STRUCT_PENDING | extra_flags);
+ WORK_STRUCT_PENDING | WORK_STRUCT_CWQ | extra_flags);
}
static void set_work_cpu(struct work_struct *work, unsigned int cpu)
@@ -507,25 +506,24 @@ static void clear_work_data(struct work_struct *work)
set_work_data(work, WORK_STRUCT_NO_CPU, 0);
}
-static inline unsigned long get_work_data(struct work_struct *work)
-{
- return atomic_long_read(&work->data) & WORK_STRUCT_WQ_DATA_MASK;
-}
-
static struct cpu_workqueue_struct *get_work_cwq(struct work_struct *work)
{
- unsigned long data = get_work_data(work);
+ unsigned long data = atomic_long_read(&work->data);
- return data >= PAGE_OFFSET ? (void *)data : NULL;
+ if (data & WORK_STRUCT_CWQ)
+ return (void *)(data & WORK_STRUCT_WQ_DATA_MASK);
+ else
+ return NULL;
}
static struct global_cwq *get_work_gcwq(struct work_struct *work)
{
- unsigned long data = get_work_data(work);
+ unsigned long data = atomic_long_read(&work->data);
unsigned int cpu;
- if (data >= PAGE_OFFSET)
- return ((struct cpu_workqueue_struct *)data)->gcwq;
+ if (data & WORK_STRUCT_CWQ)
+ return ((struct cpu_workqueue_struct *)
+ (data & WORK_STRUCT_WQ_DATA_MASK))->gcwq;
cpu = data >> WORK_STRUCT_FLAG_BITS;
if (cpu == WORK_CPU_NONE)
@@ -3501,14 +3499,6 @@ void __init init_workqueues(void)
unsigned int cpu;
int i;
- /*
- * The pointer part of work->data is either pointing to the
- * cwq or contains the cpu number the work ran last on. Make
- * sure cpu number won't overflow into kernel pointer area so
- * that they can be distinguished.
- */
- BUILD_BUG_ON(WORK_CPU_LAST << WORK_STRUCT_FLAG_BITS >= PAGE_OFFSET);
-
hotcpu_notifier(workqueue_cpu_callback, CPU_PRI_WORKQUEUE);
/* initialize gcwqs */
--
1.6.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists