Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Jun 2023 13:32:24 +0200
From:   Frederic Weisbecker <frederic@kernel.org>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Anna-Maria Behnsen <anna-maria@linutronix.de>,
        John Stultz <jstultz@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Eric Biederman <ebiederm@xmission.com>,
        Oleg Nesterov <oleg@redhat.com>
Subject: Re: [patch 13/45] posix-cpu-timers: Replace old expiry retrieval in
 posix_cpu_timer_set()
Message-ID: <ZJrIyGxdDYy3Ppy1@lothringen>
References: <20230606132949.068951363@linutronix.de>
 <20230606142031.761271959@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230606142031.761271959@linutronix.de>
Precedence: bulk

On Tue, Jun 06, 2023 at 04:37:39PM +0200, Thomas Gleixner wrote:
> Reuse the split out __posix_cpu_timer_get() function which does already the
> right thing.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/time/posix-cpu-timers.c |   28 +++++-----------------------
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -609,6 +609,8 @@ static void cpu_timer_fire(struct k_itim
>  	}
>  }
>  
> +static void __posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64
> *itp, u64 now);

Perhaps you can move __posix_cpu_timer_get() above posix_cpu_timer_set()?

> +
>  /*
>   * Guts of sys_timer_settime for CPU timers.
>   * This is called with the timer locked and interrupts disabled.
> @@ -680,29 +682,9 @@ static int posix_cpu_timer_set(struct k_
>  	else
>  		now = cpu_clock_sample_group(clkid, p, !sigev_none);
>  
> -	if (old) {
> -		if (old_expires == 0) {
> -			old->it_value.tv_sec = 0;
> -			old->it_value.tv_nsec = 0;
> -		} else {
> -			/*
> -			 * Update the timer in case it has overrun already.
> -			 * If it has, we'll report it as having overrun and
> -			 * with the next reloaded timer already ticking,
> -			 * though we are swallowing that pending
> -			 * notification here to install the new setting.
> -			 */
> -			u64 exp = bump_cpu_timer(timer, now);
> -
> -			if (now < exp) {
> -				old_expires = exp - now;
> -				old->it_value = ns_to_timespec64(old_expires);
> -			} else {
> -				old->it_value.tv_nsec = 1;
> -				old->it_value.tv_sec = 0;
> -			}
> -		}
> -	}
> +	/* Retrieve the previous expiry value if requested. */
> +	if (old && old_expires)
> +		__posix_cpu_timer_get(timer, old, now);

The changelog should probably mention the desired side effect that sigev_none
will return a zero old value if expired.

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

Thanks.

>  
>  	/* Retry if the timer expiry is running concurrently */
>  	if (unlikely(ret)) {
>