workqueue: Preserve OFFQ bits in cancel[_sync] paths

The cancel[_sync] paths acquire and release WORK_STRUCT_PENDING, and
manipulate WORK_OFFQ_CANCELING. However, they assume that all the OFFQ bit
values except for the pool ID are statically known and don't preserve them,
which is not wrong in the current code as the pool ID and CANCELING are the
only information carried. However, the planned disable/enable support will
add more fields and need them to be preserved.

This patch updates work data handling so that only the bits which need
updating are updated.

- struct work_offq_data is added along with work_offqd_unpack() and
work_offqd_pack_flags() to help manipulating multiple fields contained in
work->data. Note that the helpers look a bit silly right now as there
isn't that much to pack. The next patch will add more.

- mark_work_canceling() which is used only by __cancel_work_sync() is
replaced by open-coded usage of work_offq_data and
set_work_pool_and_keep_pending() in __cancel_work_sync().

- __cancel_work[_sync]() uses offq_data helpers to preserve other OFFQ bits
when clearing WORK_STRUCT_PENDING and WORK_OFFQ_CANCELING at the end.

- This removes all users of get_work_pool_id() which is dropped. Note that
get_work_pool_id() could handle both WORK_STRUCT_PWQ and !WORK_STRUCT_PWQ
cases; however, it was only being called after try_to_grab_pending()
succeeded, in which case WORK_STRUCT_PWQ is never set and thus it's safe
to use work_offqd_unpack() instead.

No behavior changes intended.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>

Tejun Heo 2 years ago 1211f3b2 4cece764

+33 -21

2 changed files

expand all

include

linux

workqueue.h

kernel

workqueue.c

include/linux/workqueue.h

··· 97 97 98 98 /* Convenience constants - of type 'unsigned long', not 'enum'! */ 99 99 #define WORK_OFFQ_CANCELING (1ul << WORK_OFFQ_CANCELING_BIT) 100 + #define WORK_OFFQ_FLAG_MASK (((1ul << WORK_OFFQ_FLAG_BITS) - 1) << WORK_OFFQ_FLAG_SHIFT) 100 101 #define WORK_OFFQ_POOL_NONE ((1ul << WORK_OFFQ_POOL_BITS) - 1) 101 102 #define WORK_STRUCT_NO_POOL (WORK_OFFQ_POOL_NONE << WORK_OFFQ_POOL_SHIFT) 102 103 #define WORK_STRUCT_PWQ_MASK (~((1ul << WORK_STRUCT_PWQ_SHIFT) - 1))

+32 -21

kernel/workqueue.c

··· 392 392 int *cpu_pod; /* cpu -> pod */ 393 393 }; 394 394 395 + struct work_offq_data { 396 + u32 pool_id; 397 + u32 flags; 398 + }; 399 + 395 400 static const char *wq_affn_names[WQ_AFFN_NR_TYPES] = { 396 401 [WQ_AFFN_DFL] = "default", 397 402 [WQ_AFFN_CPU] = "cpu", ··· 897 892 return idr_find(&worker_pool_idr, pool_id); 898 893 } 899 894 900 - /** 901 - * get_work_pool_id - return the worker pool ID a given work is associated with 902 - * @work: the work item of interest 903 - * 904 - * Return: The worker_pool ID @work was last associated with. 905 - * %WORK_OFFQ_POOL_NONE if none. 906 - */ 907 - static int get_work_pool_id(struct work_struct *work) 895 + static unsigned long shift_and_mask(unsigned long v, u32 shift, u32 bits) 908 896 { 909 - unsigned long data = atomic_long_read(&work->data); 910 - 911 - if (data & WORK_STRUCT_PWQ) 912 - return work_struct_pwq(data)->pool->id; 913 - 914 - return data >> WORK_OFFQ_POOL_SHIFT; 897 + return (v >> shift) & ((1 << bits) - 1); 915 898 } 916 899 917 - static void mark_work_canceling(struct work_struct *work) 900 + static void work_offqd_unpack(struct work_offq_data *offqd, unsigned long data) 918 901 { 919 - unsigned long pool_id = get_work_pool_id(work); 902 + WARN_ON_ONCE(data & WORK_STRUCT_PWQ); 920 903 921 - pool_id <<= WORK_OFFQ_POOL_SHIFT; 922 - set_work_data(work, pool_id | WORK_STRUCT_PENDING | WORK_OFFQ_CANCELING); 904 + offqd->pool_id = shift_and_mask(data, WORK_OFFQ_POOL_SHIFT, 905 + WORK_OFFQ_POOL_BITS); 906 + offqd->flags = data & WORK_OFFQ_FLAG_MASK; 907 + } 908 + 909 + static unsigned long work_offqd_pack_flags(struct work_offq_data *offqd) 910 + { 911 + return (unsigned long)offqd->flags; 923 912 } 924 913 925 914 static bool work_is_canceling(struct work_struct *work) ··· 4270 4271 4271 4272 static bool __cancel_work(struct work_struct *work, u32 cflags) 4272 4273 { 4274 + struct work_offq_data offqd; 4273 4275 unsigned long irq_flags; 4274 4276 int ret; 4275 4277 ··· 4281 4281 if (unlikely(ret < 0)) 4282 4282 return false; 4283 4283 4284 - set_work_pool_and_clear_pending(work, get_work_pool_id(work), 0); 4284 + work_offqd_unpack(&offqd, *work_data_bits(work)); 4285 + set_work_pool_and_clear_pending(work, offqd.pool_id, 4286 + work_offqd_pack_flags(&offqd)); 4285 4287 local_irq_restore(irq_flags); 4286 4288 return ret; 4287 4289 } 4288 4290 4289 4291 static bool __cancel_work_sync(struct work_struct *work, u32 cflags) 4290 4292 { 4293 + struct work_offq_data offqd; 4291 4294 unsigned long irq_flags; 4292 4295 bool ret; 4293 4296 4294 4297 /* claim @work and tell other tasks trying to grab @work to back off */ 4295 4298 ret = work_grab_pending(work, cflags, &irq_flags); 4296 - mark_work_canceling(work); 4299 + 4300 + work_offqd_unpack(&offqd, *work_data_bits(work)); 4301 + offqd.flags |= WORK_OFFQ_CANCELING; 4302 + set_work_pool_and_keep_pending(work, offqd.pool_id, 4303 + work_offqd_pack_flags(&offqd)); 4297 4304 local_irq_restore(irq_flags); 4298 4305 4299 4306 /* ··· 4310 4303 if (wq_online) 4311 4304 __flush_work(work, true); 4312 4305 4306 + work_offqd_unpack(&offqd, *work_data_bits(work)); 4307 + 4313 4308 /* 4314 4309 * smp_mb() at the end of set_work_pool_and_clear_pending() is paired 4315 4310 * with prepare_to_wait() above so that either waitqueue_active() is 4316 4311 * visible here or !work_is_canceling() is visible there. 4317 4312 */ 4318 - set_work_pool_and_clear_pending(work, WORK_OFFQ_POOL_NONE, 0); 4313 + offqd.flags &= ~WORK_OFFQ_CANCELING; 4314 + set_work_pool_and_clear_pending(work, WORK_OFFQ_POOL_NONE, 4315 + work_offqd_pack_flags(&offqd)); 4319 4316 4320 4317 if (waitqueue_active(&wq_cancel_waitq)) 4321 4318 __wake_up(&wq_cancel_waitq, TASK_NORMAL, 1, work);

Configure Feed

Configure Feed