Re: [patch V3 8/8] posix-timers: Handle exit in do_exit() completely

[Andrea Parri <parri.andrea@gmail.com>]

> @@ -167,12 +167,6 @@ static void __exit_signal(struct release
>  					lockdep_tasklist_lock_is_held());
>  	spin_lock(&sighand->siglock);
>  
> -#ifdef CONFIG_POSIX_TIMERS
> -	posix_cpu_timers_exit(tsk);
> -	if (group_dead)
> -		posix_cpu_timers_exit_group(tsk);
> -#endif
> -

Now that __exit_signal() no longer touches the CPU timer queues, does
the block comment above timer_lock_sighand() need an update in the same
patch?  It still credits __exit_signal() with invoking
posix_cpu_timers_exit()/posix_cpu_timers_exit_group().

The barrier diagram further down in timer_lock_sighand() describes the
same old callchain, and it is what the WARN_ON_ONCE() right below the
diagram relies on.  Would it help to spell the new chain out so that the
WARN_ON_ONCE() keeps a documented ordering argument?


> @@ -661,18 +661,29 @@ static void cleanup_timers(struct posix_
>  	cleanup_timerqueue(&pct->bases[CPUCLOCK_SCHED].tqhead);
>  }
>  
> +static inline void posix_cpu_timers_exit_work(void);
> +
>  /*
> - * These are both called with the siglock held, when the current thread
> - * is being reaped.  When the final (leader) thread in the group is reaped,
> - * posix_cpu_timers_exit_group will be called after posix_cpu_timers_exit.
> + * Invoked from posixtimer_exit_task() after PF_EXITING was set in tsk::flags or
> + * from posixtimer_exec_cleanup().

Should these be posixtimer_exit() and posixtimer_exec()?  Neither
posixtimer_exit_task() nor posixtimer_exec_cleanup() exists in the tree.
The callers are posixtimer_exit()->posix_cpu_timers_exit_task(),
posixtimer_exec()->posix_cpu_timers_exit_task() and
posix_cpu_timers_exit_group()->posix_cpu_timers_exit_task().


>   */
> -void posix_cpu_timers_exit(struct task_struct *tsk)
> +void posix_cpu_timers_exit_task(void)
>  {
> -	cleanup_timers(&tsk->posix_cputimers);
> +	posix_cpu_timers_exit_work();
> +
> +	guard(spinlock_irq)(&current->sighand->siglock);
> +	cleanup_timers(&current->posix_cputimers);
>  }
> -void posix_cpu_timers_exit_group(struct task_struct *tsk)
> +
> +/*
> + * Invoked from posixtimer_exit_group() after PF_EXITING was set in tsk::flags.

Same question here: the only caller is posixtimer_exit(), and
posixtimer_exit_group() does not exist.


> + */
> +void posix_cpu_timers_exit_group(void)
>  {
> -	cleanup_timers(&tsk->signal->posix_cputimers);
> +	posix_cpu_timers_exit_task();
> +
> +	guard(spinlock_irq)(&current->sighand->siglock);
> +	cleanup_timers(&current->signal->posix_cputimers);
>  }
>  
>  /*

  Andrea