Re: [PATCH] Bugfix for CLOCK_REALTIME absolute timer.

[Andrew Morton <akpm@osdl.org>]

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.