El mar, 17 dic 2024 a las 23:33, Zhaoming Luo (<zhming...@163.com>) escribió: > On 12/18/24 10:02 AM, Diego Nieto Cid wrote: > > El mar, 17 dic 2024 a las 22:21, Zhaoming Luo (<zhming...@163.com>) escribió: > > > > `read_elapsed_ticks` is only used to calculate elapsed_usec and thus > > may be inlined, removing the otherwise unused variable > Do you mean something like the following? > ``` > elapsed_usec = elapsed_ticks * tick; > elapsed_time->seconds = elapsed_usec / MICROSECONDS_IN_ONE_SECOND; > elapsed_time->microseconds = elapsed_usec % MICROSECONDS_IN_ONE_SECOND; > ``` > `read_elapsed_ticks` is read first because I hope it can reduce the > possibility of data races. The `elapsed_tick` may be incremented in the > timer interrupt[0].
Yes, like that.Storing it in a variable doesn't make it less racy. At some point the read has to be done, it doesn't matter if it is being stored in a temporary location (register) or in the stack. > > > > Or you could also use the lock used to increment `elapsed_ticks`: > > > > s = simple_lock_irq(&timer_lock); > > elapsed_usec = elapsed_tikcs * tick; > > simple_unlock_irq(s, &timer_lock); > > > > I'm not sure which works best. > I don't think the latter will work. The timer interrupt also calls > simple_lock_irq()[2]. It may cause deadlock. > > Hmm, yeah you are right, the timer interrupt may be triggered while the ticks variable is read, I'm not sure what will happen then :/ But there are some usages here and there, so it somehow should work already. Below, the uses I'm referring to softclock : https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n305 set_timeout : https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n337 reset_timeout: https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n362 timeout: https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n618 untimeout: https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n641 However, in its declaration[1]... def_simple_lock_irq_data (static, timer_lock) /* lock for ... */ timer_elt_data_t timer_head; /* ordered list of timeouts */ /* (doubles as end-of-list) */ ...the comment implies it's used for protecting the list of timeouts, not the elapsed_tick. -- [1] https://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/kern/mach_clock.c#n122