lkml.org 
[lkml]   [2021]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [GIT pull] timers/urgent for v5.16-rc1
On Sun, Nov 14, 2021 at 11:02:31AM -0800, Linus Torvalds wrote:
> On Sun, Nov 14, 2021 at 5:31 AM Thomas Gleixner <tglx@linutronix.de> wrote:

> But apparently it matters for posix_cputimers_work and for numa_work,
> and so I think it's very illogical that init_task_work() will not
> actually initialize it properly.

The problem with the posix timers thing seems to be that it can race
against fork() but afaict it can't actually mis-behave if it has garbage
in ->next, so the clearing here is pure paranoia.

> And that task_tick_numa() special case is truly horrendous, and really
> should go after the init_task_work() regardless, exactly because you'd
> expect that init_task_work() to initialize the work even if it doesn't
> happen to right now.

Yeah, it's a wee bit 'special' allright :-)

> Or is somebody doing init_task_work() to only change the work-function
> on an already initialized work entry? Becuase that sounds both racy
> and broken to me, and none of the things I looked at from a quick grep
> looked like that at all.

The worst I found is someone sharing an rcu_head between task_work and
call_rcu (supposedly at different stages in the life-time).

I couldn't find any other weird cases.

---
include/linux/task_work.h | 1 +
kernel/sched/fair.c | 4 ++--
kernel/time/posix-cpu-timers.c | 2 --
3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/task_work.h b/include/linux/task_work.h
index 5b8a93f288bb..fbbc9aa8e4ae 100644
--- a/include/linux/task_work.h
+++ b/include/linux/task_work.h
@@ -10,6 +10,7 @@ typedef void (*task_work_func_t)(struct callback_head *);
static inline void
init_task_work(struct callback_head *twork, task_work_func_t func)
{
+ twork->next = NULL;
twork->func = func;
}

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6e476f6d9435..d03dacdecf73 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2823,14 +2823,14 @@ void init_numa_balancing(unsigned long clone_flags, struct task_struct *p)
p->node_stamp = 0;
p->numa_scan_seq = mm ? mm->numa_scan_seq : 0;
p->numa_scan_period = sysctl_numa_balancing_scan_delay;
- /* Protect against double add, see task_tick_numa and task_numa_work */
- p->numa_work.next = &p->numa_work;
p->numa_faults = NULL;
RCU_INIT_POINTER(p->numa_group, NULL);
p->last_task_numa_placement = 0;
p->last_sum_exec_runtime = 0;

init_task_work(&p->numa_work, task_numa_work);
+ /* Protect against double add, see task_tick_numa and task_numa_work */
+ p->numa_work.next = &p->numa_work;

/* New address space, reset the preferred nid */
if (!(clone_flags & CLONE_VM)) {
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 96b4e7810426..3352759e6916 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1167,8 +1167,6 @@ void clear_posix_cputimers_work(struct task_struct *p)
* A copied work entry from the old task is not meaningful, clear it.
* N.B. init_task_work will not do this.
*/
- memset(&p->posix_cputimers_work.work, 0,
- sizeof(p->posix_cputimers_work.work));
init_task_work(&p->posix_cputimers_work.work,
posix_cpu_timers_work);
p->posix_cputimers_work.scheduled = false;
\
 
 \ /
  Last update: 2021-11-15 16:52    [W:0.361 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site