Messages in this thread | | | Date | Mon, 28 Jun 2004 15:17:25 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] Bugfix for CLOCK_REALTIME absolute timer. |
| |
George Anzinger <george@mvista.com> wrote: > > Andrew, > > Boris and I have kicked this around enough. It think it is ready for prime time. >
> static void schedule_next_timer(struct k_itimer *timr) > { > ... > + do { > + seq = read_seqbegin(&xtime_lock); > + new_wall_to = wall_to_monotonic; > + posix_get_now(&now); > + } while (read_seqretry(&xtime_lock, seq)); > + > + if (!list_empty(&timr->abs_timer_entry)) { > + spin_lock(&abs_list.lock); > + add_clockset_delta(timr, &new_wall_to); > + } > + > do { > posix_bump_timer(timr); > }while (posix_time_before(&timr->it_timer, &now)); > > + if (!list_empty(&timr->abs_timer_entry)) > + spin_unlock(&abs_list.lock);
The locking in here is a bit ugly. Does the lock actually need to be held while the timer is being bumped?
And what is the upper bound on that while loop?
> tmr->it_id = (timer_t)-1; > + INIT_LIST_HEAD(&tmr->abs_timer_entry); > if (unlikely(!(tmr->sigq = sigqueue_alloc()))) {
The cat ate your tab key? ;)
> + if (!list_empty(&timr->abs_timer_entry)) { > + spin_lock(&abs_list.lock); > + list_del_init(&timr->abs_timer_entry); > + spin_unlock(&abs_list.lock); > + }
This is repeated often. Does it merit its own function?
> +static DECLARE_MUTEX(clock_was_set_lock); > +#define mutex_enter(x) down(x) > +#define mutex_enter_interruptable(x) down_interruptible(x) > +#define mutex_exit(x) up(x)
Please open-code these operations.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |