lkml.org 
[lkml]   [2012]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.



\
 
 \ /
  Last update: 2012-10-10 20:41    [W:0.058 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site