History

workqueue: clean up WORK_* constant types, clarify masking

Dave Airlie reports that gcc-13.1.1 has started complaining about some of the workqueue code in 32-bit arm builds: kernel/workqueue.c: In function ‘get_work_pwq’: kernel/workqueue.c:713:24: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] 713 | return (void *)(data & WORK_STRUCT_WQ_DATA_MASK); | ^ [ ... a couple of other cases ... ] and while it's not immediately clear exactly why gcc started complaining about it now, I suspect it's some C23-induced enum type handlign fixup in gcc-13 is the cause. Whatever the reason for starting to complain, the code and data types are indeed disgusting enough that the complaint is warranted. The wq code ends up creating various "helper constants" (like that WORK_STRUCT_WQ_DATA_MASK) using an enum type, which is all kinds of confused. The mask needs to be 'unsigned long', not some unspecified enum type. To make matters worse, the actual "mask and cast to a pointer" is repeated a couple of times, and the cast isn't even always done to the right pointer, but - as the error case above - to a 'void *' with then the compiler finishing the job. That's now how we roll in the kernel. So create the masks using the proper types rather than some ambiguous enumeration, and use a nice helper that actually does the type conversion in one well-defined place. Incidentally, this magically makes clang generate better code. That, admittedly, is really just a sign of clang having been seriously confused before, and cleaning up the typing unconfuses the compiler too.

Linus Torvalds committed 3 years ago

Commit afa4bb71 parent 8a28a0b2 files · +16 −12Browse files

Showing the changes to kernel/workqueue.c (1 of 2 files). Show all

Unified · Split

kernel/workqueue.c+8−5
@@ -705,12 +705,17 @@ static void clear_work_data(struct work_struct *work) · 704 unmodified lines
705 set_work_data(work, WORK_STRUCT_NO_POOL, 0);705 set_work_data(work, WORK_STRUCT_NO_POOL, 0);
706}706}
707 707
708static inline struct pool_workqueue *work_struct_pwq(unsigned long data)
709{
710 return (struct pool_workqueue *)(data & WORK_STRUCT_WQ_DATA_MASK);
711}
712
708static struct pool_workqueue *get_work_pwq(struct work_struct *work)713static struct pool_workqueue *get_work_pwq(struct work_struct *work)
709{714{
710 unsigned long data = atomic_long_read(&work->data);715 unsigned long data = atomic_long_read(&work->data);
711 716
712 if (data & WORK_STRUCT_PWQ)717 if (data & WORK_STRUCT_PWQ)
713 return (void *)(data & WORK_STRUCT_WQ_DATA_MASK);718 return work_struct_pwq(data);
714 else719 else
715 return NULL;720 return NULL;
716}721}
@@ -738,8 +743,7 @@ static struct worker_pool *get_work_pool(struct work_struct *work) · 21 unmodified lines
738 assert_rcu_or_pool_mutex();743 assert_rcu_or_pool_mutex();
739 744
740 if (data & WORK_STRUCT_PWQ)745 if (data & WORK_STRUCT_PWQ)
741 return ((struct pool_workqueue *)746 return work_struct_pwq(data)->pool;
742 (data & WORK_STRUCT_WQ_DATA_MASK))->pool;
743 747
744 pool_id = data >> WORK_OFFQ_POOL_SHIFT;748 pool_id = data >> WORK_OFFQ_POOL_SHIFT;
745 if (pool_id == WORK_OFFQ_POOL_NONE)749 if (pool_id == WORK_OFFQ_POOL_NONE)
@@ -760,8 +764,7 @@ static int get_work_pool_id(struct work_struct *work) · 14 unmodified lines
760 unsigned long data = atomic_long_read(&work->data);764 unsigned long data = atomic_long_read(&work->data);
761 765
762 if (data & WORK_STRUCT_PWQ)766 if (data & WORK_STRUCT_PWQ)
763 return ((struct pool_workqueue *)767 return work_struct_pwq(data)->pool->id;
764 (data & WORK_STRUCT_WQ_DATA_MASK))->pool->id;
765 768
766 return data >> WORK_OFFQ_POOL_SHIFT;769 return data >> WORK_OFFQ_POOL_SHIFT;
767}770}