Re: [patch V2 3/5] posix-cpu-timers: Provide mechanisms to defer timer handling to task_work

Oleg Nesterov
2020-07-17T17:26:40Z

Looks correct to me, but I forgot everything about posix-timers.c

this obviously means that the expired timer won't fire until the
task returns to user-mode but probably we don't care.

One cosmetic nit below,

On 07/16, Thomas Gleixner wrote:
>
> +#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
> +void posix_cpu_timers_work(struct callback_head *work);
> +
> +static inline void posix_cputimer_init_work(struct posix_cputimers *pct)
> +{
> + pct->task_work.func = posix_cpu_timers_work;

init_task_work() ?

> +}
> +#else
> +static inline void posix_cputimer_init_work(struct posix_cputimers *pct) { }
> +#endif
> +
> static inline void posix_cputimers_init(struct posix_cputimers *pct)
> {
> memset(pct, 0, sizeof(*pct));
> pct->bases[0].nextevt = U64_MAX;
> pct->bases[1].nextevt = U64_MAX;
> pct->bases[2].nextevt = U64_MAX;
> + posix_cputimer_init_work(pct);
> }

And I can't resist. I know this is a common practice, please ignore, but to me

static inline void posix_cputimers_init(struct posix_cputimers *pct)
{
memset(pct, 0, sizeof(*pct));
pct->bases[0].nextevt = U64_MAX;
pct->bases[1].nextevt = U64_MAX;
pct->bases[2].nextevt = U64_MAX;
#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK
init_task_work(&pct->task_work, posix_cpu_timers_work);
#endif
}

looks better than 2 posix_cputimer_init_work() definitions above.

Note also that signal_struct->posix_cputimers.task_work is never used, perhaps
it would be better to move this task_work into task_struct? This way we do not
even need to change posix_cputimers_init(), we call simply initialize
init_task.posix_task_work.

Oleg.