timers: simplify locking

This is the last one, I promise.
On top of "[PATCH rc1-mm3] timers: kill timer_list->lock", see
http://marc.theaimsgroup.com/?l=linux-kernel&m=111193319932543

This patch adds lock_timer() helper, which locks timer's base, and checks it is still the same and != NULL.

After the previous patch the timer's base is never NULL. With this patch __mod_timer() temporaly sets ->_base = __TIMER_PENDING after deletion of timer, so the timer is still pending and can't be locked by lock_timer().

The result is that __mod_timer() does not need to lock both bases. Now it can do:

	old_base = lock_timer()
	list_del(timer)
	__set_base(timer, NULL, 1)
	unlock(old_base)
		// The timer can't be seen by __run_timers(old_base).
		// Still pending, del_timer() will call lock_timer().
		// Nobody can modify it, lock_timer() will wait.
	lock_new(new_base)
	add timer
	unlock(new_base)

This slightly speedups __mod_timer(), and in my opinion simplifies the code. I hope it may also improve scalability of timers.

@@ -33,11 +33,6 @@ struct timer_list {
 
 #define	__TIMER_PENDING	1
 
-static inline int __timer_pending(struct tvec_t_base_s *base)
-{
-	return ((unsigned long)base & __TIMER_PENDING) != 0;
-}
-
 #define TIMER_MAGIC	0x4b87ad6e
 
 extern struct fake_timer_base fake_timer_base;

@@ -64,7 +59,7 @@ void fastcall init_timer(struct timer_li
  */
 static inline int timer_pending(const struct timer_list * timer)
 {
-	return __timer_pending(timer->_base);
+	return ((unsigned long)timer->_base & __TIMER_PENDING) != 0;
 }
 
 extern void add_timer_on(struct timer_list *timer, int cpu);

Ok. Testing with your latest and greatest patches. Is there any clarity about what caused the hangs?

Many thanks.

No, I still hope these hangs are unrelated to these patches. I am trying to find the bug, but I can't. May be it is because I do not want to believe that these patches are buggy :)

I tried to stress them with various small tests I wrote and I have not found any problems.

And I don't know what kernel you are using, there is additional timer patch in mm2.

Oleg.

I thought that earlier patch was a bit weird and I think it would be better to get to the bottom of these problems which people have been reporting in the 2.6.12-rc1-mm3 timer code before adding more things, don't you?

A bit weird, or too weird to be acceptable?

I am very much interested in your and others opinion. You do not like this fake_timer_base or you don't like the idea that the timer is always locked through timer_base() ?

These 2 patches should be cleanuped of course.

struct XXX {
	spinlock_t lock;
	timer_list *running_timer;
};

struct XXX fake_timer_base;

struct tvec_t_base_s {
	struct XXX xxx;

	long timer_jiffies;
	tvec_t tv1;
	...
}

struct timer_list {
	...
	struct XXX *_base;
}

So tvec_t_base_s is used only in __mod_timer() for new_base. This cleanup would be trivial and without changes in timer.o.

However this global fake_timer_base is really neccessary for this patch and it is really wierd.

But I like the fact that __mod_timer() takes 2 locks sequentially instead of 3 at once.

I just can't imagine how this "del_timer_sync instead of del_singleshot_timer in schedule_timeout" can reveal any bug in these patches. del_singleshot_timer calls del_timer_sync anyway when the timer is inactive. The only difference is that now schedule_timeout()->del_timer_sync() actually deletes this timer when the caller was waken by a signal/event. And that deletion is very simple, it just "can't be wrong", and it adds LIST_POISON to timer->entry.

But you are right of course. It is better to forget about these new patches for a while.

Oleg.