Commit 0f900049cbe2767d47c2a62b54f0e822e1d66840

Authored by Tejun Heo
1 parent 1537663f57

workqueue: update cwq alignement

work->data field is used for two purposes.  It points to cwq it's
queued on and the lower bits are used for flags.  Currently, two bits
are reserved which is always safe as 4 byte alignment is guaranteed on
every architecture.  However, future changes will need more flag bits.

On SMP, the percpu allocator is capable of honoring larger alignment
(there are other users which depend on it) and larger alignment works
just fine.  On UP, percpu allocator is a thin wrapper around
kzalloc/kfree() and don't honor alignment request.

This patch introduces WORK_STRUCT_FLAG_BITS and implements
alloc/free_cwqs() which guarantees max(1 << WORK_STRUCT_FLAG_BITS,
__alignof__(unsigned long long) alignment both on SMP and UP.  On SMP,
simply wrapping percpu allocator is enough.  On UP, extra space is
allocated so that cwq can be aligned and the original pointer can be
stored after it which is used in the free path.

* Alignment problem on UP is reported by Michal Simek.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Reported-by: Michal Simek <michal.simek@petalogix.com>

Showing 2 changed files with 59 additions and 6 deletions Side-by-side Diff

include/linux/workqueue.h
... ... @@ -26,6 +26,9 @@
26 26 WORK_STRUCT_PENDING_BIT = 0, /* work item is pending execution */
27 27 #ifdef CONFIG_DEBUG_OBJECTS_WORK
28 28 WORK_STRUCT_STATIC_BIT = 1, /* static initializer (debugobjects) */
  29 + WORK_STRUCT_FLAG_BITS = 2,
  30 +#else
  31 + WORK_STRUCT_FLAG_BITS = 1,
29 32 #endif
30 33  
31 34 WORK_STRUCT_PENDING = 1 << WORK_STRUCT_PENDING_BIT,
... ... @@ -35,7 +38,7 @@
35 38 WORK_STRUCT_STATIC = 0,
36 39 #endif
37 40  
38   - WORK_STRUCT_FLAG_MASK = 3UL,
  41 + WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
39 42 WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
40 43 };
41 44  
... ... @@ -46,7 +46,9 @@
46 46  
47 47 /*
48 48 * The per-CPU workqueue (if single thread, we always use the first
49   - * possible cpu).
  49 + * possible cpu). The lower WORK_STRUCT_FLAG_BITS of
  50 + * work_struct->data are used for flags and thus cwqs need to be
  51 + * aligned at two's power of the number of flag bits.
50 52 */
51 53 struct cpu_workqueue_struct {
52 54  
... ... @@ -59,7 +61,7 @@
59 61  
60 62 struct workqueue_struct *wq; /* I: the owning workqueue */
61 63 struct task_struct *thread;
62   -} ____cacheline_aligned;
  64 +};
63 65  
64 66 /*
65 67 * The externally visible workqueue abstraction is an array of
... ... @@ -967,6 +969,53 @@
967 969  
968 970 }
969 971  
  972 +static struct cpu_workqueue_struct *alloc_cwqs(void)
  973 +{
  974 + /*
  975 + * cwqs are forced aligned according to WORK_STRUCT_FLAG_BITS.
  976 + * Make sure that the alignment isn't lower than that of
  977 + * unsigned long long.
  978 + */
  979 + const size_t size = sizeof(struct cpu_workqueue_struct);
  980 + const size_t align = max_t(size_t, 1 << WORK_STRUCT_FLAG_BITS,
  981 + __alignof__(unsigned long long));
  982 + struct cpu_workqueue_struct *cwqs;
  983 +#ifndef CONFIG_SMP
  984 + void *ptr;
  985 +
  986 + /*
  987 + * On UP, percpu allocator doesn't honor alignment parameter
  988 + * and simply uses arch-dependent default. Allocate enough
  989 + * room to align cwq and put an extra pointer at the end
  990 + * pointing back to the originally allocated pointer which
  991 + * will be used for free.
  992 + *
  993 + * FIXME: This really belongs to UP percpu code. Update UP
  994 + * percpu code to honor alignment and remove this ugliness.
  995 + */
  996 + ptr = __alloc_percpu(size + align + sizeof(void *), 1);
  997 + cwqs = PTR_ALIGN(ptr, align);
  998 + *(void **)per_cpu_ptr(cwqs + 1, 0) = ptr;
  999 +#else
  1000 + /* On SMP, percpu allocator can do it itself */
  1001 + cwqs = __alloc_percpu(size, align);
  1002 +#endif
  1003 + /* just in case, make sure it's actually aligned */
  1004 + BUG_ON(!IS_ALIGNED((unsigned long)cwqs, align));
  1005 + return cwqs;
  1006 +}
  1007 +
  1008 +static void free_cwqs(struct cpu_workqueue_struct *cwqs)
  1009 +{
  1010 +#ifndef CONFIG_SMP
  1011 + /* on UP, the pointer to free is stored right after the cwq */
  1012 + if (cwqs)
  1013 + free_percpu(*(void **)per_cpu_ptr(cwqs + 1, 0));
  1014 +#else
  1015 + free_percpu(cwqs);
  1016 +#endif
  1017 +}
  1018 +
970 1019 static int create_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu)
971 1020 {
972 1021 struct workqueue_struct *wq = cwq->wq;
... ... @@ -1012,7 +1061,7 @@
1012 1061 if (!wq)
1013 1062 goto err;
1014 1063  
1015   - wq->cpu_wq = alloc_percpu(struct cpu_workqueue_struct);
  1064 + wq->cpu_wq = alloc_cwqs();
1016 1065 if (!wq->cpu_wq)
1017 1066 goto err;
1018 1067  
... ... @@ -1031,6 +1080,7 @@
1031 1080 for_each_possible_cpu(cpu) {
1032 1081 struct cpu_workqueue_struct *cwq = get_cwq(cpu, wq);
1033 1082  
  1083 + BUG_ON((unsigned long)cwq & WORK_STRUCT_FLAG_MASK);
1034 1084 cwq->wq = wq;
1035 1085 cwq->cpu = cpu;
1036 1086 spin_lock_init(&cwq->lock);
... ... @@ -1059,7 +1109,7 @@
1059 1109 return wq;
1060 1110 err:
1061 1111 if (wq) {
1062   - free_percpu(wq->cpu_wq);
  1112 + free_cwqs(wq->cpu_wq);
1063 1113 kfree(wq);
1064 1114 }
1065 1115 return NULL;
... ... @@ -1112,7 +1162,7 @@
1112 1162 for_each_possible_cpu(cpu)
1113 1163 cleanup_workqueue_thread(get_cwq(cpu, wq));
1114 1164  
1115   - free_percpu(wq->cpu_wq);
  1165 + free_cwqs(wq->cpu_wq);
1116 1166 kfree(wq);
1117 1167 }
1118 1168 EXPORT_SYMBOL_GPL(destroy_workqueue);