[ANNOUNCE] Kernel Janitor's TODO list

[ANNOUNCE] Kernel Janitor's TODO list

[Arnaldo Carvalho de Melo]

Hi,

	The kernel Janitor's TODO list is updated at
http://bazar.conectiva.com.br/~acme/TODO, lots of things to do to get rid
of old cruft, make sure that resources are properly used, etc, please take
a look and help! Please send additions and corrections to me and I'll try
to keep it updated.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Arnaldo Carvalho de Melo]

Em Sun, Jan 28, 2001 at 03:20:18PM +0000, David Woodhouse escreveu:
> 
> acme@conectiva.com.br said:
> >  Please send additions and corrections to me and I'll try to keep it
> > updated.
> 
> Anything which uses sleep_on() has a 90% chance of being broken. Fix
> them all, because we want to remove sleep_on() and friends in 2.5.

TODO updated, availabe at http://bazar.conectiva.com.br/~acme/TODO

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Arnaldo Carvalho de Melo]

Em Mon, Jan 29, 2001 at 03:13:19AM +1100, Andrew Morton escreveu:
> Arnaldo Carvalho de Melo wrote:
> > 
> > Please send additions and corrections to me and I'll try
> > to keep it updated.
> 
> Here - have about 300 bugs:
> 
> 	http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> 
> A lot of the timer deletion races are hard to fix because of
> the deadlock problem.

added, please keep sending, and as somebody pointed out: it is good to have
explanations about what have to be done, so interested people can
contribute, specially people that are looking to start helping and don't
know where to start, now we can say "hey, pick one of these 300 bugs and
start doing something!" ;)

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Arnaldo Carvalho de Melo]

Em Sun, Jan 28, 2001 at 12:28:50PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 29, 2001 at 03:13:19AM +1100, Andrew Morton escreveu:
> > Arnaldo Carvalho de Melo wrote:
> > > 
> > > Please send additions and corrections to me and I'll try
> > > to keep it updated.
> > 
> > Here - have about 300 bugs:
> > 
> > 	http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> > 
> > A lot of the timer deletion races are hard to fix because of
> > the deadlock problem.
> 
> added, please keep sending, and as somebody pointed out: it is good to have
> explanations about what have to be done, so interested people can
> contribute, specially people that are looking to start helping and don't
> know where to start, now we can say "hey, pick one of these 300 bugs and
> start doing something!" ;)

I forgot to add: "Like Andrew did in the above URL" 8)

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[David Woodhouse]

acme@conectiva.com.br said:
>  Please send additions and corrections to me and I'll try to keep it
> updated.

Anything which uses sleep_on() has a 90% chance of being broken. Fix
them all, because we want to remove sleep_on() and friends in 2.5.

--
dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Michael H. Warfield]

On Sun, Jan 28, 2001 at 03:20:18PM +0000, David Woodhouse wrote:

> acme@conectiva.com.br said:
> >  Please send additions and corrections to me and I'll try to keep it
> > updated.

> Anything which uses sleep_on() has a 90% chance of being broken. Fix
> them all, because we want to remove sleep_on() and friends in 2.5.

	And friends meaning "interruptible_sleep_on"?  Great...  I've
got a driver with about a half a dozen of them.  Point me at the Doco
to fix please?

> --
> dwmw2

	Mike
-- 
 Michael H. Warfield    |  (770) 985-6132   |  mhw@WittsEnd.com
  (The Mad Wizard)      |  (678) 463-0932   |  http://www.wittsend.com/mhw/
  NIC whois:  MHW9      |  An optimist believes we live in the best of all
 PGP Key: 0xDF1DD471    |  possible worlds.  A pessimist is sure of it!

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Andrew Morton]

Arnaldo Carvalho de Melo wrote:
> 
> Please send additions and corrections to me and I'll try
> to keep it updated.

Here - have about 300 bugs:

	http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html

A lot of the timer deletion races are hard to fix because of
the deadlock problem.

-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Rusty Russell]

In message <3A74451F.DA29FD17@uow.edu.au> you write:
> 	http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> 
> A lot of the timer deletion races are hard to fix because of
> the deadlock problem.

Hmmm...

	For 2.5, changing the timer interface to disallow mod_timer or
add_timer (equivalent) on self, and making the timerfn return num
jiffies to next run (0 = don't rerun) would solve this, right?
I don't see a maintainable way of solving this otherwise,

	Of course, kfree'ing the timer struct and returning non-zero
would be a *bug*...

Rusty.
--
Premature optmztion is rt of all evl. --DK
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Daniel Phillips]

Rusty Russell wrote:
> 
> In message <3A74451F.DA29FD17@uow.edu.au> you write:
> >       http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> >
> > A lot of the timer deletion races are hard to fix because of
> > the deadlock problem.
> 
> Hmmm...
> 
>         For 2.5, changing the timer interface to disallow mod_timer or
> add_timer (equivalent) on self, and making the timerfn return num
> jiffies to next run (0 = don't rerun) would solve this, right?
> I don't see a maintainable way of solving this otherwise,

It seems silly not to provide direct support for such a simple, useful
mechanism as a periodic timer.  This can be accomplished easily by
adding a field 'periodic' to struct timer_list.  If 'periodic' is
non-zero then run_timer_list uses it to set the 'expires' field and
re-inserts the timer.

For what it's worth, this is backward compatible with the existing
strategy.  The timer_list->function is still in complete control of
things if it wants to be, but forbidding it from re-adding itself sounds
like an awfully good idea.

--
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [ANNOUNCE] Kernel Janitor's TODO list

[Daniel Phillips]

Daniel Phillips wrote:
> 
> Rusty Russell wrote:
> >
> > In message <3A74451F.DA29FD17@uow.edu.au> you write:
> > >       http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> > >
> > > A lot of the timer deletion races are hard to fix because of
> > > the deadlock problem.
> >
> > Hmmm...
> >
> >         For 2.5, changing the timer interface to disallow mod_timer or
> > add_timer (equivalent) on self, and making the timerfn return num
> > jiffies to next run (0 = don't rerun) would solve this, right?
> > I don't see a maintainable way of solving this otherwise,
> 
> It seems silly not to provide direct support for such a simple, useful
> mechanism as a periodic timer.  This can be accomplished easily by
> adding a field 'periodic' to struct timer_list.  If 'periodic' is
> non-zero then run_timer_list uses it to set the 'expires' field and
> re-inserts the timer.
> 
> For what it's worth, this is backward compatible with the existing
> strategy.  The timer_list->function is still in complete control of
> things if it wants to be, but forbidding it from re-adding itself sounds
> like an awfully good idea.

Whoops, this post from Alexy makes it quite clear why I can't do that:

	http://www.wcug.wwu.edu/lists/netdev/200005/msg00050.html
	Timers are self-destructable as rule. See? Normal usage
	for timer is to have it allocated inside an object and
	timer event detroys the object together with timer.

I did a quick scan through timer usage, and sure enough, I found
self-destructive behaviour as Alexy describes, for example, in
ax25_std_heartbeat_expiry.  Your suggestion is good and simple, but
requires every timer_list->function to be changed, a couple of hundred
places to check.

It would be nice to have a nice easy transition instead of a
jump-off-the-cliff and change all usage approach.  Hmm, a hack is
coming... I'll add a new, improved function field beside the old one,
call it ->timer_event, and it can force rescheduling as you suggested. 
If ->timer_event is non-null it gets called instead of ->function, and
the timer may be requeued.  For good measure, I'll leave my ->period
field in there because it just makes sense.  Then I can write a generic
->timer_event that just returns the ->period.

/me: hack, hack, hack

--
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

[RFC] New Improved Stronger Whiter Timers (was: Kernel Janitor)

[Daniel Phillips]

On Tue, 30 Jan 2001, Rusty Russell wrote:
> In message <3A74451F.DA29FD17@uow.edu.au> you write:
> > 	http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.3/0269.html
> > 
> > A lot of the timer deletion races are hard to fix because of
> > the deadlock problem.
> 
> Hmmm...
> 
> 	For 2.5, changing the timer interface to disallow mod_timer or
> add_timer (equivalent) on self, and making the timerfn return num
> jiffies to next run (0 = don't rerun) would solve this, right?
> I don't see a maintainable way of solving this otherwise,
> 
> 	Of course, kfree'ing the timer struct and returning non-zero
> would be a *bug*...

Here's a patch that implements your idea together with a hack by me to
make it work with existing code: ->event is non-null for the new
behaviour, null for the broken behavior.  The new behaviour is that
timers don't re-add themselves anymore, run_timer_list does, so when
you kill a timer it stays dead.

--- 2.4.1.clean/include/linux/timer.h	Tue Jan 30 08:24:55 2001
+++ 2.4.1/include/linux/timer.h	Tue Jan 30 20:59:01 2001
@@ -22,6 +22,7 @@
 	unsigned long expires;
 	unsigned long data;
 	void (*function)(unsigned long);
+	unsigned long (*event)(unsigned long data);
 };
 
 extern void add_timer(struct timer_list * timer);
@@ -49,6 +50,9 @@
 static inline void init_timer(struct timer_list * timer)
 {
 	timer->list.next = timer->list.prev = NULL;
+	timer->function = NULL;
+	timer->event = NULL;
+	timer->data = 0;
 }
 
 static inline int timer_pending (const struct timer_list * timer)
--- 2.4.1.clean/kernel/timer.c	Sun Dec 10 18:53:19 2000
+++ 2.4.1/kernel/timer.c	Tue Jan 30 20:59:01 2001
@@ -301,18 +301,25 @@
 		curr = head->next;
 		if (curr != head) {
 			struct timer_list *timer;
-			void (*fn)(unsigned long);
-			unsigned long data;
+			unsigned long data, requeue;
 
 			timer = list_entry(curr, struct timer_list, list);
- 			fn = timer->function;
  			data= timer->data;
 
 			detach_timer(timer);
 			timer->list.next = timer->list.prev = NULL;
 			timer_enter(timer);
 			spin_unlock_irq(&timerlist_lock);
-			fn(data);
+			if (timer->event)
+			{
+				if ((requeue = timer->event(data)))
+				{
+					timer->expires += requeue;
+					internal_add_timer(timer);
+				}
+			}
+			else
+				timer->function(data); /* bad old way */
 			spin_lock_irq(&timerlist_lock);
 			timer_exit();
 			goto repeat;

-------------------
Here's some test code.  It kprintS a 10-step countdown during init:

--- 2.4.1.clean/fs/buffer.c	Mon Jan 15 21:42:32 2001
+++ 2.4.1/fs/buffer.c	Tue Jan 30 20:59:01 2001
@@ -2792,9 +2792,22 @@
 	}
 }
 
+struct timer_list foo_timer;
+
+unsigned long foo_event (unsigned long data)
+{
+	printk ("Foo %i\n", foo_timer.data);
+	return --foo_timer.data? HZ: 0;
+}
+
 static int __init bdflush_init(void)
 {
 	DECLARE_MUTEX_LOCKED(sem);
+	init_timer (&foo_timer);
+	foo_timer.event = foo_event;
+	foo_timer.data = 10;
+	add_timer (&foo_timer);
+
 	kernel_thread(bdflush, &sem, CLONE_FS | CLONE_FILES | CLONE_SIGNAL);
 	down(&sem);
 	kernel_thread(kupdate, &sem, CLONE_FS | CLONE_FILES | CLONE_SIGNAL);

-- 
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [RFC] New Improved Stronger Whiter Timers (was: Kernel Janitor)

[Daniel Phillips]

On Tue, 30 Jan 2001, Manfred Spraul wrote:
> This one is an UP and SMP race:
> 
>   spin_unlock_irq(&timerlist_lock); 
> + if (timer->event) 
> + { 
> + if ((requeue = timer->event(data))) 
> + { 
> + timer->expires += requeue; 
> + internal_add_timer(timer); 
> + } 
> + } 
> + else 
> + timer->function(data); /* bad old way */ 
>   spin_lock_irq(&timerlist_lock); 
> 
> internal_add_timer assumes that the timerlist_lock is acquired.

Yes, oops, here is the fix.

--- ../2.4.1.clean/include/linux/timer.h	Tue Jan 30 08:24:55 2001
+++ ./include/linux/timer.h	Tue Jan 30 21:52:58 2001
@@ -22,6 +22,7 @@
 	unsigned long expires;
 	unsigned long data;
 	void (*function)(unsigned long);
+	unsigned long (*event)(unsigned long data);
 };
 
 extern void add_timer(struct timer_list * timer);
@@ -49,6 +50,9 @@
 static inline void init_timer(struct timer_list * timer)
 {
 	timer->list.next = timer->list.prev = NULL;
+	timer->function = NULL;
+	timer->event = NULL;
+	timer->data = 0;
 }
 
 static inline int timer_pending (const struct timer_list * timer)
--- ../2.4.1.clean/kernel/timer.c	Sun Dec 10 18:53:19 2000
+++ ./kernel/timer.c	Tue Jan 30 22:09:09 2001
@@ -301,19 +301,30 @@
 		curr = head->next;
 		if (curr != head) {
 			struct timer_list *timer;
-			void (*fn)(unsigned long);
-			unsigned long data;
+			unsigned long data, requeue;
 
 			timer = list_entry(curr, struct timer_list, list);
- 			fn = timer->function;
  			data= timer->data;
 
 			detach_timer(timer);
 			timer->list.next = timer->list.prev = NULL;
 			timer_enter(timer);
 			spin_unlock_irq(&timerlist_lock);
-			fn(data);
-			spin_lock_irq(&timerlist_lock);
+			if (timer->event)
+			{
+				requeue = timer->event(data);
+				spin_lock_irq(&timerlist_lock);
+				if (requeue)
+				{
+					timer->expires += requeue;
+					internal_add_timer(timer);
+				}
+			}
+			else
+			{
+				timer->function(data); /* bad old way */
+				spin_lock_irq(&timerlist_lock);
+			}
 			timer_exit();
 			goto repeat;
 		}

-- 
Daniel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/