Skip to contentSkip to conversation

Firewire node manager causes up to ~3.25s delay in freezing tasks.

PATCH In review Nigel Cunningham 2 participants 3 messages Dec 9, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Firewire node manager causes up to ~3.25s delay in freezing tasks.

Hi all.

(Okay, having learnt that ieee1394 != bluetooth, I'll try again :>)

The firewire nodemanager function "nodemgr_host_thread" contains a loop that calls try_to_freeze near the top of the loop, but then delays for up to 3.25 seconds (plus time to do work) before getting back to the top of the loop. When starting a cycle post-boot, this doesn't seem to bite, but it is causing a noticeable delay at boot time, when freezing processes prior to starting to read the image.

The following patch adds invocation of try_to_freeze to the subloops that are used in the body of this function. With these additions, the time to freeze when starting to resume at boot time is virtually zero. I'm no expert on bluetooth, and so don't know that we shouldn't check the return value and jump back to the top of the loop or such like after being frozen, but I submit it for your consideration.

 nodemgr.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/ieee1394/nodemgr.c +2 -0
diff -ruNp 710-nodemanager-freezing.patch-old/drivers/ieee1394/nodemgr.c 710-nodemanager-freezing.patch-new/drivers/ieee1394/nodemgr.c
@@ -1685,6 +1685,7 @@ static int nodemgr_host_thread(void *dat
 		g = get_hpsb_generation(host);
 		for (i = 0; i < 4 ; i++) {
 			msleep_interruptible(63);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 
@@ -1725,6 +1726,7 @@ static int nodemgr_host_thread(void *dat
 		/* Sleep 3 seconds */
 		for (i = 3000/200; i; i--) {
 			msleep_interruptible(200);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 
#1 lore raw
Quoted 10 lines · Nigel Cunningham

Hi all.

(Okay, having learnt that ieee1394 != bluetooth, I'll try again :>)

The firewire nodemanager function "nodemgr_host_thread" contains a loop that calls try_to_freeze near the top of the loop, but then delays for up to 3.25 seconds (plus time to do work) before getting back to the top of the loop. When starting a cycle post-boot, this doesn't seem to bite, but it is causing a noticeable delay at boot time, when freezing processes prior to starting to read the image.

In 2.6.27 and less, these were only 0.25 seconds. The 3 additional seconds in 2.6.28-rc are my doing and sort of a regression. It occurs to me that I should send this patch to mainline before release.

Quoted 8 lines

The following patch adds invocation of try_to_freeze to the subloops that are used in the body of this function. With these additions, the time to freeze when starting to resume at boot time is virtually zero. I'm no expert on bluetooth, and so don't know that we shouldn't check the return value and jump back to the top of the loop or such like after being frozen, but I submit it for your consideration.

As far as I can tell, try_to_freeze() without any jump is correct.

drivers/ieee1394/nodemgr.c static int nodemgr_host_thread(void *dat · line 1691

Quoted 13 lines
 nodemgr.c |    2 ++
 1 file changed, 2 insertions(+)

diff -ruNp 710-nodemanager-freezing.patch-old/drivers/ieee1394/nodemgr.c 710-nodemanager-freezing.patch-new/drivers/ieee1394/nodemgr.c

drivers/ieee1394/nodemgr.c +1 -0
@@ -1685,6 +1685,7 @@ static int nodemgr_host_thread(void *dat
 		g = get_hpsb_generation(host);
 		for (i = 0; i < 4 ; i++) {
 			msleep_interruptible(63);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 

Here we have the goal to proceed only >= 0.25s after the last bus reset happened. While the new try_to_freeze() held up nodemgr, another bus reset may happen. But we check for this later in the loop and handle it, so this is OK.

drivers/ieee1394/nodemgr.c static int nodemgr_host_thread(void *dat · line 1732

Quoted 8 lines
drivers/ieee1394/nodemgr.c +1 -0
@@ -1725,6 +1726,7 @@ static int nodemgr_host_thread(void *dat
 		/* Sleep 3 seconds */
 		for (i = 3000/200; i; i--) {
 			msleep_interruptible(200);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 

Ditto, the rest of the loop catches the case that another bus reset occurred during the sleep or freeze.

Thanks for what appears to be a regression fix,

Signature · 3 lines
Stefan Richter
-=====-==--- ==-- -=--=
http://arcgraph.de/sr/
#2 lore raw
In reply to Nigel Cunningham
Quoted 10 lines · Nigel Cunningham

Hi all.

(Okay, having learnt that ieee1394 != bluetooth, I'll try again :>)

The firewire nodemanager function "nodemgr_host_thread" contains a loop that calls try_to_freeze near the top of the loop, but then delays for up to 3.25 seconds (plus time to do work) before getting back to the top of the loop. When starting a cycle post-boot, this doesn't seem to bite, but it is causing a noticeable delay at boot time, when freezing processes prior to starting to read the image.

In 2.6.27 and less, these were only 0.25 seconds. The 3 additional seconds in 2.6.28-rc are my doing and sort of a regression. It occurs to me that I should send this patch to mainline before release.

Quoted 8 lines

The following patch adds invocation of try_to_freeze to the subloops that are used in the body of this function. With these additions, the time to freeze when starting to resume at boot time is virtually zero. I'm no expert on bluetooth, and so don't know that we shouldn't check the return value and jump back to the top of the loop or such like after being frozen, but I submit it for your consideration.

As far as I can tell, try_to_freeze() without any jump is correct.

drivers/ieee1394/nodemgr.c static int nodemgr_host_thread(void *dat · line 1691

Quoted 13 lines
 nodemgr.c |    2 ++
 1 file changed, 2 insertions(+)

diff -ruNp 710-nodemanager-freezing.patch-old/drivers/ieee1394/nodemgr.c 710-nodemanager-freezing.patch-new/drivers/ieee1394/nodemgr.c

drivers/ieee1394/nodemgr.c +1 -0
@@ -1685,6 +1685,7 @@ static int nodemgr_host_thread(void *dat
 		g = get_hpsb_generation(host);
 		for (i = 0; i < 4 ; i++) {
 			msleep_interruptible(63);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 

Here we have the goal to proceed only >= 0.25s after the last bus reset happened. While the new try_to_freeze() held up nodemgr, another bus reset may happen. But we check for this later in the loop and handle it, so this is OK.

drivers/ieee1394/nodemgr.c static int nodemgr_host_thread(void *dat · line 1732

Quoted 8 lines
drivers/ieee1394/nodemgr.c +1 -0
@@ -1725,6 +1726,7 @@ static int nodemgr_host_thread(void *dat
 		/* Sleep 3 seconds */
 		for (i = 3000/200; i; i--) {
 			msleep_interruptible(200);
+			try_to_freeze();
 			if (kthread_should_stop())
 				goto exit;
 

Ditto, the rest of the loop catches the case that another bus reset occurred during the sleep or freeze.

Thanks for what appears to be a regression fix,

Signature · 3 lines
Stefan Richter
-=====-==--- ==-- -=--=
http://arcgraph.de/sr/