Messages in this thread Patch in this message | | | From | "Eric W. Biederman" <> | Date | Fri, 12 Apr 2024 13:25:01 -0500 | Subject | Re: [patch V2 07/50] posix-cpu-timers: Split up posix_cpu_timer_get() |
| |
Thomas Gleixner <tglx@linutronix.de> writes:
> In preparation for addressing issues in the timer_get() and timer_set() > functions of posix CPU timers.
To see that this was safe I had to lookup and see that cpu_timer_getexpires is a truly trivial function.
static inline u64 cpu_timer_getexpires(struct cpu_timer *ctmr) { return ctmr->node.expires; }
I am a bit confused by the purpose of this function in posix-cpu-timers.c. In some places this helper is used (like below), and in other places like bump_cpu_timer the expires member is accessed directly.
It isn't really a problem, but it is something that might be worth making consistent in the code to make it easier to read.
Eric
> No functional change. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > V2: Split out into new patch to make review simpler - Frederic > --- > kernel/time/posix-cpu-timers.c | 51 +++++++++++++++++++---------------------- > 1 file changed, 24 insertions(+), 27 deletions(-) > > --- a/kernel/time/posix-cpu-timers.c > +++ b/kernel/time/posix-cpu-timers.c > @@ -785,33 +785,9 @@ static int posix_cpu_timer_set(struct k_ > return ret; > } > > -static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp) > +static void __posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp, u64 now) > { > - clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock); > - struct cpu_timer *ctmr = &timer->it.cpu; > - u64 now, expires = cpu_timer_getexpires(ctmr); > - struct task_struct *p; > - > - rcu_read_lock(); > - p = cpu_timer_task_rcu(timer); > - if (!p) > - goto out; > - > - /* > - * Easy part: convert the reload time. > - */ > - itp->it_interval = ktime_to_timespec64(timer->it_interval); > - > - if (!expires) > - goto out; > - > - /* > - * Sample the clock to take the difference with the expiry time. > - */ > - if (CPUCLOCK_PERTHREAD(timer->it_clock)) > - now = cpu_clock_sample(clkid, p); > - else > - now = cpu_clock_sample_group(clkid, p, false); > + u64 expires = cpu_timer_getexpires(&timer->it.cpu); > > if (now < expires) { > itp->it_value = ns_to_timespec64(expires - now); > @@ -823,7 +799,28 @@ static void posix_cpu_timer_get(struct k > itp->it_value.tv_nsec = 1; > itp->it_value.tv_sec = 0; > } > -out: > +} > + > +static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp) > +{ > + clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock); > + struct task_struct *p; > + u64 now; > + > + rcu_read_lock(); > + p = cpu_timer_task_rcu(timer); > + if (p) { > + itp->it_interval = ktime_to_timespec64(timer->it_interval); > + > + if (cpu_timer_getexpires(&timer->it.cpu)) { > + if (CPUCLOCK_PERTHREAD(timer->it_clock)) > + now = cpu_clock_sample(clkid, p); > + else > + now = cpu_clock_sample_group(clkid, p, false); > + > + __posix_cpu_timer_get(timer, itp, now); > + } > + } > rcu_read_unlock(); > } >
| |