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@kernel.org>
Reported-by: Sachin Sant <sachinp@in.ibm.com>
Reported-by: Michal Simek <michal.simek@petalogix.com>
Reported-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Tested-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Tested-by: Michal Simek <monstr@monstr.eu>

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index d74a529ed13e8926f20288c00f3b299ced328148..5f76001c4e6d54ac5afc92492a5e5995705f6635 100644 (file)
--- 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 c11edc9c9365aee10fa19ca3e7eabbf6943022c1..e5cb7faac58ea83d29b25c35e35e8f260296c105 100644 (file)
--- 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 */