linux-kernel - Re: [PATCH 3/6] posix-cpu-timers: Force next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YMnBUybUcUP3Ll/P@hirez.programming.kicks-ass.net>
Date:   Wed, 16 Jun 2021 11:16:03 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        Oleg Nesterov <oleg@...hat.com>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH 3/6] posix-cpu-timers: Force next_expiration recalc after
 timer deletion

On Fri, Jun 04, 2021 at 01:31:56PM +0200, Frederic Weisbecker wrote:
> A timer deletion only dequeues the timer but it doesn't shutdown
> the related costly process wide cputimer counter and the tick dependency.
> 
> The following code snippet keeps this overhead around for one week after
> the timer deletion:
> 
> 	void trigger_process_counter(void)
> 	{
> 		timer_t id;
> 		struct itimerspec val = { };
> 
> 		val.it_value.tv_sec = 604800;
> 		timer_create(CLOCK_PROCESS_CPUTIME_ID, NULL, &id);
> 		timer_settime(id, 0, &val, NULL);
> 		timer_delete(id);
> 	}
> 
> Make sure the next target's tick recalculates the nearest expiration and
> clears the process wide counter and tick dependency if necessary.

> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index 132fd56fb1cd..bb1f862c785e 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -405,6 +405,33 @@ static int posix_cpu_timer_create(struct k_itimer *new_timer)
>  	return 0;
>  }
>  
> +/*
> + * Dequeue the timer and reset the base if it was its earliest expiration.
> + * It makes sure the next tick recalculates the base next expiration so we
> + * don't keep the costly process wide cputime counter around for a random
> + * amount of time, along with the tick dependency.
> + */
> +static void disarm_timer(struct k_itimer *timer, struct task_struct *p)
> +{
> +	struct cpu_timer *ctmr = &timer->it.cpu;
> +	struct posix_cputimer_base *base;
> +	int clkidx;
> +
> +	if (!cpu_timer_dequeue(ctmr))
> +		return;
> +
> +	clkidx = CPUCLOCK_WHICH(timer->it_clock);
> +
> +	if (CPUCLOCK_PERTHREAD(timer->it_clock))
> +		base = p->posix_cputimers.bases + clkidx;
> +	else
> +		base = p->signal->posix_cputimers.bases + clkidx;
> +
> +	if (cpu_timer_getexpires(ctmr) == base->nextevt)
> +		base->nextevt = 0;
> +}

OK, so check_process_timers() unconditionally recomputes ->nextevt in
collect_posix_cputimers() provided ->timers_active. It also clears
->timers_active if it finds none are left. This recompute is before all
actual consumers of ->nextevt, with one exception.

This will loose the update of ->nextevt in arm_timer(), if one were to
happen between this and check_process_timers(), but afaict that has no
ill effect. Still that might warrant a comment somewhere.

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>