Commit e120153d authored by Tejun Heo's avatar Tejun Heo

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: default avatarTejun Heo <tj@kernel.org>
Reported-by: default avatarSachin Sant <sachinp@in.ibm.com>
Reported-by: default avatarMichal Simek <michal.simek@petalogix.com>
Reported-by: default avatarMartin Schwidefsky <schwidefsky@de.ibm.com>
Tested-by: default avatarMartin Schwidefsky <schwidefsky@de.ibm.com>
Tested-by: default avatarMichal Simek <monstr@monstr.eu>
parent f2e005aa
...@@ -25,17 +25,19 @@ typedef void (*work_func_t)(struct work_struct *work); ...@@ -25,17 +25,19 @@ typedef void (*work_func_t)(struct work_struct *work);
enum { enum {
WORK_STRUCT_PENDING_BIT = 0, /* work item is pending execution */ 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 #ifdef CONFIG_DEBUG_OBJECTS_WORK
WORK_STRUCT_STATIC_BIT = 2, /* static initializer (debugobjects) */ WORK_STRUCT_STATIC_BIT = 3, /* static initializer (debugobjects) */
WORK_STRUCT_COLOR_SHIFT = 3, /* color for workqueue flushing */ WORK_STRUCT_COLOR_SHIFT = 4, /* color for workqueue flushing */
#else #else
WORK_STRUCT_COLOR_SHIFT = 2, /* color for workqueue flushing */ WORK_STRUCT_COLOR_SHIFT = 3, /* color for workqueue flushing */
#endif #endif
WORK_STRUCT_COLOR_BITS = 4, WORK_STRUCT_COLOR_BITS = 4,
WORK_STRUCT_PENDING = 1 << WORK_STRUCT_PENDING_BIT, WORK_STRUCT_PENDING = 1 << WORK_STRUCT_PENDING_BIT,
WORK_STRUCT_CWQ = 1 << WORK_STRUCT_CWQ_BIT,
WORK_STRUCT_LINKED = 1 << WORK_STRUCT_LINKED_BIT, WORK_STRUCT_LINKED = 1 << WORK_STRUCT_LINKED_BIT,
#ifdef CONFIG_DEBUG_OBJECTS_WORK #ifdef CONFIG_DEBUG_OBJECTS_WORK
WORK_STRUCT_STATIC = 1 << WORK_STRUCT_STATIC_BIT, WORK_STRUCT_STATIC = 1 << WORK_STRUCT_STATIC_BIT,
...@@ -56,8 +58,8 @@ enum { ...@@ -56,8 +58,8 @@ enum {
WORK_CPU_LAST = WORK_CPU_NONE, WORK_CPU_LAST = WORK_CPU_NONE,
/* /*
* Reserve 6 bits off of cwq pointer w/ debugobjects turned * Reserve 7 bits off of cwq pointer w/ debugobjects turned
* off. This makes cwqs aligned to 64 bytes which isn't too * off. This makes cwqs aligned to 128 bytes which isn't too
* excessive while allowing 15 workqueue flush colors. * excessive while allowing 15 workqueue flush colors.
*/ */
WORK_STRUCT_FLAG_BITS = WORK_STRUCT_COLOR_SHIFT + WORK_STRUCT_FLAG_BITS = WORK_STRUCT_COLOR_SHIFT +
......
...@@ -468,10 +468,9 @@ static int work_next_color(int color) ...@@ -468,10 +468,9 @@ static int work_next_color(int color)
} }
/* /*
* Work data points to the cwq while a work is on queue. Once * A work's data points to the cwq with WORK_STRUCT_CWQ set while the
* execution starts, it points to the cpu the work was last on. This * work is on queue. Once execution starts, WORK_STRUCT_CWQ is
* can be distinguished by comparing the data value against * cleared and the work data contains the cpu number it was last on.
* PAGE_OFFSET.
* *
* set_work_{cwq|cpu}() and clear_work_data() can be used to set the * 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 * cwq, cpu or clear work->data. These functions should only be
...@@ -494,7 +493,7 @@ static void set_work_cwq(struct work_struct *work, ...@@ -494,7 +493,7 @@ static void set_work_cwq(struct work_struct *work,
unsigned long extra_flags) unsigned long extra_flags)
{ {
set_work_data(work, (unsigned long)cwq, 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) 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) ...@@ -507,25 +506,24 @@ static void clear_work_data(struct work_struct *work)
set_work_data(work, WORK_STRUCT_NO_CPU, 0); 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) 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) 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; unsigned int cpu;
if (data >= PAGE_OFFSET) if (data & WORK_STRUCT_CWQ)
return ((struct cpu_workqueue_struct *)data)->gcwq; return ((struct cpu_workqueue_struct *)
(data & WORK_STRUCT_WQ_DATA_MASK))->gcwq;
cpu = data >> WORK_STRUCT_FLAG_BITS; cpu = data >> WORK_STRUCT_FLAG_BITS;
if (cpu == WORK_CPU_NONE) if (cpu == WORK_CPU_NONE)
...@@ -3501,14 +3499,6 @@ void __init init_workqueues(void) ...@@ -3501,14 +3499,6 @@ void __init init_workqueues(void)
unsigned int cpu; unsigned int cpu;
int i; 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); hotcpu_notifier(workqueue_cpu_callback, CPU_PRI_WORKQUEUE);
/* initialize gcwqs */ /* initialize gcwqs */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment