[patch 2/2] ipvs deadlock fix

[patch 2/2] ipvs deadlock fix

[akpm]

update_defense_level() is calling si_meminfo() from timer context.  But
si_meminfo takes non-irq-safe locks.

Move it all to keventd context.  

Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 25-akpm/net/ipv4/ipvs/ip_vs_ctl.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff -puN net/ipv4/ipvs/ip_vs_ctl.c~ipvs-deadlock-fix net/ipv4/ipvs/ip_vs_ctl.c
--- 25/net/ipv4/ipvs/ip_vs_ctl.c~ipvs-deadlock-fix	2005-01-10 21:15:28.220163464 -0800
+++ 25-akpm/net/ipv4/ipvs/ip_vs_ctl.c	2005-01-10 21:15:28.226162552 -0800
@@ -26,7 +26,7 @@
 #include <linux/fs.h>
 #include <linux/sysctl.h>
 #include <linux/proc_fs.h>
-#include <linux/timer.h>
+#include <linux/workqueue.h>
 #include <linux/swap.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
@@ -90,7 +90,7 @@ int ip_vs_get_debug_level(void)
 #endif
 
 /*
- *	update_defense_level is called from timer bh and from sysctl.
+ *	update_defense_level is called from keventd and from sysctl.
  */
 static void update_defense_level(void)
 {
@@ -212,19 +212,19 @@ static void update_defense_level(void)
 /*
  *	Timer for checking the defense
  */
-static struct timer_list defense_timer;
 #define DEFENSE_TIMER_PERIOD	1*HZ
+static void defense_work_handler(void *data);
+static DECLARE_WORK(defense_work, defense_work_handler, NULL);
 
-static void defense_timer_handler(unsigned long data)
+static void defense_work_handler(void *data)
 {
 	update_defense_level();
 	if (atomic_read(&ip_vs_dropentry))
 		ip_vs_random_dropentry();
 
-	mod_timer(&defense_timer, jiffies + DEFENSE_TIMER_PERIOD);
+	schedule_delayed_work(&defense_work, DEFENSE_TIMER_PERIOD);
 }
 
-
 int
 ip_vs_use_count_inc(void)
 {
@@ -2370,10 +2370,7 @@ int ip_vs_control_init(void)
 	ip_vs_new_estimator(&ip_vs_stats);
 
 	/* Hook the defense timer */
-	init_timer(&defense_timer);
-	defense_timer.function = defense_timer_handler;
-	defense_timer.expires = jiffies + DEFENSE_TIMER_PERIOD;
-	add_timer(&defense_timer);
+	schedule_delayed_work(&defense_work, DEFENSE_TIMER_PERIOD);
 
 	LeaveFunction(2);
 	return 0;
@@ -2384,7 +2381,7 @@ void ip_vs_control_cleanup(void)
 {
 	EnterFunction(2);
 	ip_vs_trash_cleanup();
-	del_timer_sync(&defense_timer);
+	cancel_rearming_delayed_work(&defense_work);
 	ip_vs_kill_estimator(&ip_vs_stats);
 	unregister_sysctl_table(sysctl_header);
 	proc_net_remove("ip_vs_stats");
_

Re: [patch 2/2] ipvs deadlock fix

[Horms]

On Sun, Jan 30, 2005 at 10:33:02PM -0800, akpm@osdl.org wrote:
> 
> 
> update_defense_level() is calling si_meminfo() from timer context.  But
> si_meminfo takes non-irq-safe locks.
> 
> Move it all to keventd context.  

Would I be right in thinking that the offending lock is bdev_lock which
is grabbed in nr_blockdev_pages() and thus that this is not an issue
for 2.4 whose si_meminfo() does not have such a call?

-- 
Horms

Re: [patch 2/2] ipvs deadlock fix

[Andrew Morton]

Horms <horms@verge.net.au> wrote:
>
> On Sun, Jan 30, 2005 at 10:33:02PM -0800, akpm@osdl.org wrote:
>  > 
>  > 
>  > update_defense_level() is calling si_meminfo() from timer context.  But
>  > si_meminfo takes non-irq-safe locks.
>  > 
>  > Move it all to keventd context.  
> 
>  Would I be right in thinking that the offending lock is bdev_lock which
>  is grabbed in nr_blockdev_pages() and thus that this is not an issue
>  for 2.4 whose si_meminfo() does not have such a call?

Yes, 2.4's si_meminfo() seems to be OK from interrupt context.

On x86 - I didn't check the other architectures.

Re: [patch 2/2] ipvs deadlock fix

[Horms]

On Mon, Jan 31, 2005 at 01:47:22AM -0800, Andrew Morton wrote:
> Horms <horms@verge.net.au> wrote:
> >
> > On Sun, Jan 30, 2005 at 10:33:02PM -0800, akpm@osdl.org wrote:
> >  > 
> >  > 
> >  > update_defense_level() is calling si_meminfo() from timer context.  But
> >  > si_meminfo takes non-irq-safe locks.
> >  > 
> >  > Move it all to keventd context.  
> > 
> >  Would I be right in thinking that the offending lock is bdev_lock which
> >  is grabbed in nr_blockdev_pages() and thus that this is not an issue
> >  for 2.4 whose si_meminfo() does not have such a call?
> 
> Yes, 2.4's si_meminfo() seems to be OK from interrupt context.

Thanks.

> On x86 - I didn't check the other architectures.

Me neither.

-- 
Horms

Re: [patch 2/2] ipvs deadlock fix

[David S. Miller]

On Sun, 30 Jan 2005 22:33:02 -0800
akpm@osdl.org wrote:

> update_defense_level() is calling si_meminfo() from timer context.  But
> si_meminfo takes non-irq-safe locks.
> 
> Move it all to keventd context.  
> 
> Signed-off-by: Andrew Morton <akpm@osdl.org>

Ok, both patches applied.  I'll push it upstream in 2.6.12