El mar, 17 dic 2024 a las 23:33, Zhaoming Luo (<
zhmingluo@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 (<
zhmingluo@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#n641However, 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.