Thread (38 messages) 38 messages, 6 authors, 2010-09-02

Re: [PATCH 3/3] writeback: Do not congestion sleep when there are no congested BDIs

From: Minchan Kim <hidden>
Date: 2010-08-26 17:38:51
Also in: linux-fsdevel, lkml

On Thu, Aug 26, 2010 at 04:14:16PM +0100, Mel Gorman wrote:
quoted hunk ↗ jump to hunk
If congestion_wait() is called with no BDIs congested, the caller will
sleep for the full timeout and this is an unnecessary sleep. This patch
checks if there are BDIs congested. If so, it goes to sleep as normal.
If not, it calls cond_resched() to ensure the caller is not hogging the
CPU longer than its quota but otherwise will not sleep.

This is aimed at reducing some of the major desktop stalls reported during
IO. For example, while kswapd is operating, it calls congestion_wait()
but it could just have been reclaiming clean page cache pages with no
congestion. Without this patch, it would sleep for a full timeout but after
this patch, it'll just call schedule() if it has been on the CPU too long.
Similar logic applies to direct reclaimers that are not making enough
progress.

Signed-off-by: Mel Gorman <redacted>
---
 mm/backing-dev.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index a49167f..6abe860 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
Function's decripton should be changed since we don't wait next write any more. 
quoted hunk ↗ jump to hunk
@@ -767,13 +767,21 @@ long congestion_wait(int sync, long timeout)
 	DEFINE_WAIT(wait);
 	wait_queue_head_t *wqh = &congestion_wqh[sync];
 
-	/* Check if this call to congestion_wait was necessary */
-	if (atomic_read(&nr_bdi_congested[sync]) == 0)
+	/*
+	 * If there is no congestion, there is no point sleeping on the queue.
+	 * This call was unecessary but in case we are spinning due to a bad
+	 * caller, at least call cond_reched() and sleep if our CPU quota
+	 * has expired
+	 */
+	if (atomic_read(&nr_bdi_congested[sync]) == 0) {
 		unnecessary = true;
-
-	prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
-	ret = io_schedule_timeout(timeout);
-	finish_wait(wqh, &wait);
+		cond_resched();
+		ret = 0;
"ret = timeout" is more proper as considering io_schedule_timeout's return value.
+	} else {
+		prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
+		ret = io_schedule_timeout(timeout);
+		finish_wait(wqh, &wait);
+	}
 
 	trace_writeback_congest_waited(jiffies_to_usecs(jiffies - start),
 			unnecessary);
-- 
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help