On 03/14, liguang wrote: > > Signed-off-by: liguang <lig.f...@cn.fujitsu.com>
Changelog please... > --- > kernel/task_work.c | 15 +++------------ > 1 files changed, 3 insertions(+), 12 deletions(-) > > diff --git a/kernel/task_work.c b/kernel/task_work.c > index 65bd3c9..0bf4258 100644 > --- a/kernel/task_work.c > +++ b/kernel/task_work.c > @@ -13,11 +13,12 @@ task_work_add(struct task_struct *task, struct > callback_head *work, bool notify) > head = ACCESS_ONCE(task->task_works); > if (unlikely(head == &work_exited)) > return -ESRCH; > - work->next = head; > - } while (cmpxchg(&task->task_works, head, work) != head); > + head = head->next; > + } while (cmpxchg(&head, NULL, work) == head); I simply can't understand how this can work... The patch assumes that head->next == NULL after head = head->next, why? And then compares the result with head and succeeds if not equal. Could you please explain how it was supposed to work? If nothing else, Suppose we have task->task_works -> W1 -> W2 -> W3. How this code can add W4 after W3? And cmpxchg(&head) should be cmpxchg(&head->next).... Anyway, whatever I missed this is racy. head = head->next; nothing protects "head" after this. Say, it can be task_work_cancel'ed and freed. So, cmpxchg(&head, ...) can modify the freed and reused memory. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/