Messages in this thread | | | From | Anna-Maria Behnsen <> | Subject | Re: [patch V2 31/50] posix-timers: Add proper state tracking | Date | Wed, 17 Apr 2024 15:40:36 +0200 |
| |
Thomas Gleixner <tglx@linutronix.de> writes:
> Right now the state tracking is done by two struct members: > > - it_active: > A boolean which tracks armed/disarmed state > > - it_signal_seq: > A sequence counter which is used to invalidate settings > and prevent rearming > > Replace it_active with it_status and keep properly track about the states > in one place. > > This allows to reuse it_signal_seq to track reprogramming, disarm and > delete operations in order to drop signals which are related to the state > previous of those operations. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[...]
> --- a/kernel/time/posix-cpu-timers.c > +++ b/kernel/time/posix-cpu-timers.c > @@ -453,7 +453,6 @@ static void disarm_timer(struct k_itimer > struct cpu_timer *ctmr = &timer->it.cpu; > struct posix_cputimer_base *base; > > - timer->it_active = 0; > if (!cpu_timer_dequeue(ctmr)) > return; > > @@ -494,11 +493,12 @@ static int posix_cpu_timer_del(struct k_ > */ > WARN_ON_ONCE(ctmr->head || timerqueue_node_queued(&ctmr->node)); > } else { > - if (timer->it.cpu.firing) > + if (timer->it.cpu.firing) { > ret = TIMER_RETRY; > - else > + } else { > disarm_timer(timer, p); > - > + timer->it_status = POSIX_TIMER_DISARMED; > + } > unlock_task_sighand(p, &flags); > }
Why do you move the update of the it_status here and do not reuse the place where you added the it_active in patch 21 "posix-cpu-timers: Make k_itimer::it_active consistent"? Then the update of the state would still be next to cpu_timer_dequeue().
[...]
> @@ -647,10 +650,10 @@ void common_timer_get(struct k_itimer *t > /* interval timer ? */ > if (iv) { > cur_setting->it_interval = ktime_to_timespec64(iv); > - } else if (!timr->it_active) { > + } else if (timr->it_status == POSIX_TIMER_DISARMED) { > /* > * SIGEV_NONE oneshot timers are never queued and therefore > - * timr->it_active is always false. The check below > + * timr->it_status is always DISARMED. The check below
s/DISARMED/POSIX_TIMER_DISARMED/
This change would help when using grep.
> * vs. remaining time will handle this case. > * > * For all other timers there is nothing to update here, so
| |