Patch: Make linkwatch more robust against rtnl holders

Patch: Make linkwatch more robust against rtnl holders

[Stefan Rompf]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

Hi David,

the attached patch updates the linkwatch code so that it backs off and retries 
whenever it cannot get the rtnl semaphore.

This makes it more robust against unrelated processes calling 
flush_scheduled_work() while holding the rtnl lock. This problem was reported 
on netdev for the sungem driver.

I preferred rescheduling every 1/10 second against creating a kernel thread as 
the code is shorter, and the situation should not happen anyway.

Stefan

[-- Attachment #2: patch-linkwatch-2.6.1 --]
[-- Type: text/x-diff, Size: 737 bytes --]

--- net/core/link_watch.c.old	2003-12-26 20:38:25.000000000 +0100
+++ net/core/link_watch.c	2004-01-30 20:07:57.000000000 +0100
@@ -84,6 +84,8 @@
 
 static void linkwatch_event(void *dummy)
 {
+	if (rtnl_shlock_nowait()) goto out_err1;
+	if (rtnl_exlock_nowait()) goto out_err2;
 	/* Limit the number of linkwatch events to one
 	 * per second so that a runaway driver does not
 	 * cause a storm of messages on the netlink
@@ -92,11 +94,16 @@
 	linkwatch_nextevent = jiffies + HZ;
 	clear_bit(LW_RUNNING, &linkwatch_flags);
 
-	rtnl_shlock();
-	rtnl_exlock();
 	linkwatch_run_queue();
 	rtnl_exunlock();
 	rtnl_shunlock();
+
+	return;
+
+out_err2:
+	rtnl_shunlock();
+out_err1:
+	schedule_delayed_work(&linkwatch_work, HZ / 10);
 }
 
 

Re: Patch: Make linkwatch more robust against rtnl holders

[David S. Miller]

On Sun, 15 Feb 2004 15:11:47 +0100
Stefan Rompf <srompf@isg.de> wrote:

> the attached patch updates the linkwatch code so that it backs off and retries 
> whenever it cannot get the rtnl semaphore.

I think we should fix locking conflicts created by things like what
the sungem driver is doing instead.

Re: Patch: Make linkwatch more robust against rtnl holders

[Jeff Garzik]

David S. Miller wrote:
> On Sun, 15 Feb 2004 15:11:47 +0100
> Stefan Rompf <srompf@isg.de> wrote:
> 
> 
>>the attached patch updates the linkwatch code so that it backs off and retries 
>>whenever it cannot get the rtnl semaphore.
> 
> 
> I think we should fix locking conflicts created by things like what
> the sungem driver is doing instead.


Agreed.

	Jeff

Re: Patch: Make linkwatch more robust against rtnl holders

[Stefan Rompf]

Hi David,

> > the attached patch updates the linkwatch code so that it backs off and
> > retries whenever it cannot get the rtnl semaphore.
>
> I think we should fix locking conflicts created by things like what
> the sungem driver is doing instead.

On the other hand, I'm not aware of any convention when flush_scheduled_work() 
is allowed to be called. As long as anyone inside kernel space is can 
schedule work and anyone else can wait for completion, it does not hurt to 
make a schedule task less vulnerable to deadlocks.

Stefan