Messages in this thread | | | Date | Wed, 10 Oct 2012 19:50:32 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH V2] task_work: avoid unneeded cmpxchg() in task_work_run() |
| |
On 10/10, Lai Jiangshan wrote: > > --- a/kernel/task_work.c > +++ b/kernel/task_work.c > @@ -52,16 +52,7 @@ void task_work_run(void) > struct callback_head *work, *head, *next; > > for (;;) { > - /* > - * work->func() can do task_work_add(), do not set > - * work_exited unless the list is empty. > - */ > - do { > - work = ACCESS_ONCE(task->task_works); > - head = !work && (task->flags & PF_EXITING) ? > - &work_exited : NULL; > - } while (cmpxchg(&task->task_works, work, head) != work); > - > + work = xchg(&task->task_works, NULL); > if (!work) > break; > /* > @@ -90,3 +81,17 @@ void task_work_run(void) > } while (work); > } > } > + > +void exit_task_work(struct task_struct *task) > +{ > + for (;;) { > + /* > + * work->func() can do task_work_add(), do not set > + * work_exited unless the list is empty. > + */ > + if (unlikely(task->task_works)) > + task_work_run(); > + if (cmpxchg(&task->task_works, NULL, &work_exited) == NULL) > + break; > + } > +}
I agree, this looks fine.
But if you add "unlikely" before task_work_run(), then probably it should do
while (cmpxchg(&task->task_works, NULL, work_exited)) task_work_run();
? it looks more simple/clean.
(OTOH I am not sure "unlikely" is true, note that exit_files() will offload ____fput() to task_work_run()).
But you did not answer, and I am curious. What was your original motivation? Is xchg really faster than cmpxchg?
Oleg.
| |