Messages in this thread | | | From | Lai Jiangshan <> | Date | Sat, 25 Mar 2023 17:20:14 +0800 | Subject | Re: [PATCH] workqueue: Drop the NOT_RUNNING check to flags in worker_{set,clr}_flags |
| |
On Fri, Mar 24, 2023 at 9:58 AM Tejun Heo <tj@kernel.org> wrote: > > On Mon, Mar 20, 2023 at 05:26:52PM +0800, Yue Hu wrote: > > From: Yue Hu <huyue2@coolpad.com> > > > > We know whether the worker flags are belong to WORKER_NOT_RUNNING or not > > when we are setting and clearing them. So check the flags not running > > related is unnecessary for both the cases. > > > > Currently, worker_{set,clr}_flags() are all used for WORKER_NOT_RUNNING > > except for clearing WORKER_IDLE. Let's change to directly clear it > > instead. Also, update the comment a little in worker_clr_flags(). > > I'm not sure this is better. Semantically, the existing code seems clearer > and less error-prone to me and this isn't gonna make any meaningful perf > difference. Lai, what do you think?
objdump -DSr kernel/workqueue.o | less
3275: 00 00 3273: R_X86_64_32S current_task WARN_ON_ONCE(worker->task != current); 3277: 48 39 43 40 cmp %rax,0x40(%rbx) 327b: 0f 85 91 00 00 00 jne 3312 <process_one_work+0x3a2> !(worker->flags & WORKER_NOT_RUNNING)) { 3281: 8b 43 68 mov 0x68(%rbx),%eax if ((flags & WORKER_NOT_RUNNING) && 3284: a9 c8 01 00 00 test $0x1c8,%eax 3289: 75 0b jne 3296 <process_one_work+0x326> struct worker_pool *pool = worker->pool; 328b: 48 8b 43 48 mov 0x48(%rbx),%rax pool->nr_running--; 328f: 83 68 20 01 subl $0x1,0x20(%rax) 3293: 8b 43 68 mov 0x68(%rbx),%eax worker->flags |= flags; 3296: 83 c8 40 or $0x40,%eax 3299: 89 43 68 mov %eax,0x68(%rbx)
It seems the compiler will do the trick. The clearer existing code seems better.
Thanks Lai
> > Thanks. > > -- > tejun
| |