Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 15:59:07 -0700
Andrew Morton <[EMAIL PROTECTED]> wrote:

> And lost the changelog ;)

Good point.

The current kswapd (and try_to_free_pages) code has an oddity where the
code will wait on IO, even if there is no IO in flight.  This problem is
notable especially when the system scans through many unfreeable pages,
causing unnecessary stalls in the VM.

Additionally, tasks without __GFP_FS or __GFP_IO in the direct reclaim
path will sleep if a significant number of pages are encountered that
should be written out.  This gives kswapd a chance to write out those
pages, while the direct reclaim task sleeps.

Signed-off-by: Rik van Riel <[EMAIL PROTECTED]>

diff -up linux-2.6.22/mm/vmscan.c.wait linux-2.6.22/mm/vmscan.c
--- linux-2.6.22/mm/vmscan.c.wait   2007-09-27 18:45:57.0 -0400
+++ linux-2.6.22/mm/vmscan.c2007-09-27 18:48:43.0 -0400
@@ -68,6 +68,13 @@ struct scan_control {
int all_unreclaimable;
 
int order;
+
+   /*
+* Pages that have (or should have) IO pending.  If we run into
+* a lot of these, we're better off waiting a little for IO to
+* finish rather than scanning more pages in the VM.
+*/
+   int nr_io_pages;
 };
 
 #define lru_to_page(_head) (list_entry((_head)->prev, struct page, lru))
@@ -489,8 +496,10 @@ static unsigned long shrink_page_list(st
 */
if (sync_writeback == PAGEOUT_IO_SYNC && may_enter_fs)
wait_on_page_writeback(page);
-   else
+   else {
+   sc->nr_io_pages++;
goto keep_locked;
+   }
}
 
referenced = page_referenced(page, 1);
@@ -529,8 +538,10 @@ static unsigned long shrink_page_list(st
if (PageDirty(page)) {
if (sc->order <= PAGE_ALLOC_COSTLY_ORDER && referenced)
goto keep_locked;
-   if (!may_enter_fs)
+   if (!may_enter_fs) {
+   sc->nr_io_pages++;
goto keep_locked;
+   }
if (!sc->may_writepage)
goto keep_locked;
 
@@ -541,8 +552,10 @@ static unsigned long shrink_page_list(st
case PAGE_ACTIVATE:
goto activate_locked;
case PAGE_SUCCESS:
-   if (PageWriteback(page) || PageDirty(page))
+   if (PageWriteback(page) || PageDirty(page)) {
+   sc->nr_io_pages++;
goto keep;
+   }
/*
 * A synchronous write - probably a ramdisk.  Go
 * ahead and try to reclaim the page.
@@ -1201,6 +1214,7 @@ unsigned long try_to_free_pages(struct z
 
for (priority = DEF_PRIORITY; priority >= 0; priority--) {
sc.nr_scanned = 0;
+   sc.nr_io_pages = 0;
if (!priority)
disable_swap_token();
nr_reclaimed += shrink_zones(priority, zones, );
@@ -1229,7 +1243,8 @@ unsigned long try_to_free_pages(struct z
}
 
/* Take a nap, wait for some writeback to complete */
-   if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
+   if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
+   sc.nr_io_pages > sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
}
/* top priority shrink_caches still had more to do? don't OOM, then */
@@ -1315,6 +1330,7 @@ loop_again:
if (!priority)
disable_swap_token();
 
+   sc.nr_io_pages = 0;
all_zones_ok = 1;
 
/*
@@ -1398,7 +1414,8 @@ loop_again:
 * OK, kswapd is getting into trouble.  Take a nap, then take
 * another pass across the zones.
 */
-   if (total_scanned && priority < DEF_PRIORITY - 2)
+   if (total_scanned && priority < DEF_PRIORITY - 2 &&
+   sc.nr_io_pages > sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
 
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 18:50:27 -0400
Rik van Riel <[EMAIL PROTECTED]> wrote:

> On Thu, 27 Sep 2007 15:21:21 -0700
> Andrew Morton <[EMAIL PROTECTED]> wrote:
> 
> > > Nope, sc.nr_io_pages will also be incremented when the code runs into
> > > pages that are already PageWriteback.
> > 
> > yup, I didn't think of that.  Hopefully someone else will be in there
> > working on that zone too.  If this caller yields and defers to kswapd
> > then that's very likely.  Except we just took away the ability to do that..
> 
> if (PageDirty(page)) {
> if (sc->order <= PAGE_ALLOC_COSTLY_ORDER && 
> referenced)
> goto keep_locked;
> if (!may_enter_fs)
> goto keep_locked;
> 
> I think we can fix that problem by adding a sc->nr_io_pages++
> between the last if and the goto keep_locked in shrink_page_list.
> 
> That way !GFP_IO or !GFP_FS tasks will cause themselves to sleep
> if there are pages that need to be written out, even if those
> pages are not in flight to disk yet.

yeah, that's prudent I guess.

> I have also added the comment you wanted.

And lost the changelog ;)

> - if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
> + if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
> + sc.nr_io_pages > sc.swap_cluster_max)

I do think this design decision needs a bit of explanation too.

>   congestion_wait(WRITE, HZ/10);
>   }
>   /* top priority shrink_caches still had more to do? don't OOM, then */
> @@ -1315,6 +1330,7 @@ loop_again:
>   if (!priority)
>   disable_swap_token();
>  
> + sc.nr_io_pages = 0;
>   all_zones_ok = 1;
>  
>   /*
> @@ -1398,7 +1414,8 @@ loop_again:
>* OK, kswapd is getting into trouble.  Take a nap, then take
>* another pass across the zones.
>*/
> - if (total_scanned && priority < DEF_PRIORITY - 2)
> + if (total_scanned && priority < DEF_PRIORITY - 2 &&

As did that one.  Ho hum :(  Maybe it's in the git history somewhere.

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


Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 15:21:21 -0700
Andrew Morton <[EMAIL PROTECTED]> wrote:

> > Nope, sc.nr_io_pages will also be incremented when the code runs into
> > pages that are already PageWriteback.
> 
> yup, I didn't think of that.  Hopefully someone else will be in there
> working on that zone too.  If this caller yields and defers to kswapd
> then that's very likely.  Except we just took away the ability to do that..

if (PageDirty(page)) {
if (sc->order <= PAGE_ALLOC_COSTLY_ORDER && referenced)
goto keep_locked;
if (!may_enter_fs)
goto keep_locked;

I think we can fix that problem by adding a sc->nr_io_pages++
between the last if and the goto keep_locked in shrink_page_list.

That way !GFP_IO or !GFP_FS tasks will cause themselves to sleep
if there are pages that need to be written out, even if those
pages are not in flight to disk yet.

I have also added the comment you wanted.

Signed-off-by: Rik van Riel <[EMAIL PROTECTED]>

diff -up linux-2.6.23-rc7/mm/vmscan.c.wait linux-2.6.22/mm/vmscan.c
--- linux-2.6.23-rc7/mm/vmscan.c.wait   2007-09-27 18:45:57.0 -0400
+++ linux-2.6.23-rc7/mm/vmscan.c2007-09-27 18:48:43.0 -0400
@@ -68,6 +68,13 @@ struct scan_control {
int all_unreclaimable;
 
int order;
+
+   /*
+* Pages that have (or should have) IO pending.  If we run into
+* a lot of these, we're better off waiting a little for IO to
+* finish rather than scanning more pages in the VM.
+*/
+   int nr_io_pages;
 };
 
 #define lru_to_page(_head) (list_entry((_head)->prev, struct page, lru))
@@ -489,8 +496,10 @@ static unsigned long shrink_page_list(st
 */
if (sync_writeback == PAGEOUT_IO_SYNC && may_enter_fs)
wait_on_page_writeback(page);
-   else
+   else {
+   sc->nr_io_pages++;
goto keep_locked;
+   }
}
 
referenced = page_referenced(page, 1);
@@ -529,8 +538,10 @@ static unsigned long shrink_page_list(st
if (PageDirty(page)) {
if (sc->order <= PAGE_ALLOC_COSTLY_ORDER && referenced)
goto keep_locked;
-   if (!may_enter_fs)
+   if (!may_enter_fs) {
+   sc->nr_io_pages++;
goto keep_locked;
+   }
if (!sc->may_writepage)
goto keep_locked;
 
@@ -541,8 +552,10 @@ static unsigned long shrink_page_list(st
case PAGE_ACTIVATE:
goto activate_locked;
case PAGE_SUCCESS:
-   if (PageWriteback(page) || PageDirty(page))
+   if (PageWriteback(page) || PageDirty(page)) {
+   sc->nr_io_pages++;
goto keep;
+   }
/*
 * A synchronous write - probably a ramdisk.  Go
 * ahead and try to reclaim the page.
@@ -1201,6 +1214,7 @@ unsigned long try_to_free_pages(struct z
 
for (priority = DEF_PRIORITY; priority >= 0; priority--) {
sc.nr_scanned = 0;
+   sc.nr_io_pages = 0;
if (!priority)
disable_swap_token();
nr_reclaimed += shrink_zones(priority, zones, );
@@ -1229,7 +1243,8 @@ unsigned long try_to_free_pages(struct z
}
 
/* Take a nap, wait for some writeback to complete */
-   if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
+   if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
+   sc.nr_io_pages > sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
}
/* top priority shrink_caches still had more to do? don't OOM, then */
@@ -1315,6 +1330,7 @@ loop_again:
if (!priority)
disable_swap_token();
 
+   sc.nr_io_pages = 0;
all_zones_ok = 1;
 
/*
@@ -1398,7 +1414,8 @@ loop_again:
 * OK, kswapd is getting into trouble.  Take a nap, then take
 * another pass across the zones.
 */
-   if (total_scanned && priority < DEF_PRIORITY - 2)
+   if (total_scanned && priority < DEF_PRIORITY - 2 &&
+   sc.nr_io_pages > sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
 
/*
-
To unsubscribe from this list: send 

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 18:13:25 -0400
Rik van Riel <[EMAIL PROTECTED]> wrote:

> On Thu, 27 Sep 2007 14:47:02 -0700
> Andrew Morton <[EMAIL PROTECTED]> wrote:
> 
> > On Thu, 27 Sep 2007 17:08:16 -0400
> > Rik van Riel <[EMAIL PROTECTED]> wrote:
> > 
> > > The current kswapd (and try_to_free_pages) code has an oddity where the
> > > code will wait on IO, even if there is no IO in flight.  This problem is
> > > notable especially when the system scans through many unfreeable pages,
> > > causing unnecessary stalls in the VM.
> > > 
> > 
> > What effect did this change have?
> 
> Kswapd was no longer sitting in "D" state as often and pages got
> freed more promptly.  The test was done on a RHEL kernel with
> this change though - I guess I should redo it with a current upstream
> kernel.

OK.  Yes, it should help quite a bit in the common cases.

> > >  
> > >   /* Take a nap, wait for some writeback to complete */
> > > - if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
> > > + if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
> > > + sc.nr_io_pages > sc.swap_cluster_max)
> > 
> > The comparison with swap_cluster_max is unobvious, and merits a
> > comment.  What is the thinking here?  
> 
> If the number of pages undergoing IO is really small, waiting
> for them may be a waste of time.
> 
> Maybe my thinking is wrong, not sure...

The thinking sounds good to me, but I'm looking for weirdo side-effects in
corner cases.  And I'm trying to work out what actual design we want to
have behind these various magic numbers and thresholds.

> > Also, we now have this:
> > 
> > if (total_scanned > sc.swap_cluster_max +
> > sc.swap_cluster_max / 2) {
> > wakeup_pdflush(laptop_mode ? 0 : total_scanned);
> > sc.may_writepage = 1;
> > }
> > 
> > /* Take a nap, wait for some writeback to complete */
> > if (sc.nr_scanned && priority < DEF_PRIORITY - 2 && 
> > sc.nr_io_pages > sc.swap_cluster_max)
> > congestion_wait(WRITE, HZ/10);
> > 
> > 
> > So in the case where total_scanned has not yet reached
> > swap_cluster_max, this process isn't initiating writeout and it isn't
> > sleeping, either.  Nor is it incrementing nr_io_pages.
> 
> Actually, nr_io_pages is also incremented when we run into pages that
> are already PageWriteback - even if we did not start the IO ourselves.

OK, that'll help a lot in this scenario.

> > In the range (swap_cluster_max < nr_io_pages < 1.5*swap_cluster_max) this
> > process still isn't incrementing nr_io_pages, but it _is_ running
> > congestion_wait().
> 
> It is incrementing sc.nr_io_pages and will wait on IO to complete if
> the amount of pages in flight to disk that it scanned over is larger
> than the number of pages that it is trying to free.
> 
> > Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
> > initiating IO and is throttling on writeback completion events.
> > 
> > This all seems a bit weird and arbitrary - what is the reason for
> > throttling-but-not-writing in that 1.0->1.5 window?
> 
> Good question.  Note that the throttling-but-not-writing window in
> the current code is 0.0->1.5, so this patch does reduce the throttling
> window compared to the current code.
> 
> What is the reason that the current code does IO throttling even if
> there is no IO at all in flight?

Buggered if I know ;)

It may have the accidental effect that it opens a window in which some
may_enter_fs-capable process can get scheduled and do some writeout,
perhaps.

> > If there _is_ a reason and it's all been carefully thought out and
> > designed, then can we please capture a description of that design in the
> > changelog or in the code?
> 
> I'll add a description for the sc.nr_io_pages > sc.swap_cluster_max
> test.

OK, thanks.  Perhaps a few words tacked onto the nr_io_pages definition
site would be the place to capture this.

> > Also, I wonder about what this change will do to the dynamic behaviour of
> > GFP_NOFS direct-reclaimers.  Previously they would throttle if they
> > encounter dirty pages which they can't write out.  Hopefully someone else
> > (kswapd or a __GFP_FS direct-reclaimer) will write some of those pages
> > and this caller will be woken when that writeout completes and will go off
> > and scoop them off the tail of the LRU.
> > 
> > But after this change, such a GFP_NOFS caller will, afacit, burn its way
> > through potentially the entire inactive list and will then declare oom. 
> 
> Nope, sc.nr_io_pages will also be incremented when the code runs into
> pages that are already PageWriteback.

yup, I didn't think of that.  Hopefully someone else will be in there
working on that zone too.  If this caller yields and defers to kswapd
then that's very likely.  Except we just took away the ability to do that..

-
To unsubscribe from this 

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 14:47:02 -0700
Andrew Morton <[EMAIL PROTECTED]> wrote:

> On Thu, 27 Sep 2007 17:08:16 -0400
> Rik van Riel <[EMAIL PROTECTED]> wrote:
> 
> > The current kswapd (and try_to_free_pages) code has an oddity where the
> > code will wait on IO, even if there is no IO in flight.  This problem is
> > notable especially when the system scans through many unfreeable pages,
> > causing unnecessary stalls in the VM.
> > 
> 
> What effect did this change have?

Kswapd was no longer sitting in "D" state as often and pages got
freed more promptly.  The test was done on a RHEL kernel with
this change though - I guess I should redo it with a current upstream
kernel.

> > diff -up linux-2.6.22.x86_64/mm/vmscan.c.wait 
> > linux-2.6.22.x86_64/mm/vmscan.c
> > --- linux-2.6.22.x86_64/mm/vmscan.c.wait2007-09-25 11:33:30.0 
> > -0400
> > +++ linux-2.6.22.x86_64/mm/vmscan.c 2007-09-25 21:27:08.0 -0400
> > @@ -68,6 +68,8 @@ struct scan_control {
> > int all_unreclaimable;
> >  
> > int order;
> > +
> > +   int nr_io_pages;
> >  };
> >  
> >  #define lru_to_page(_head) (list_entry((_head)->prev, struct page, lru))
> > @@ -489,8 +491,10 @@ static unsigned long shrink_page_list(st
> >  */
> > if (sync_writeback == PAGEOUT_IO_SYNC && may_enter_fs)
> > wait_on_page_writeback(page);
> > -   else
> > +   else {
> > +   sc->nr_io_pages++;
> > goto keep_locked;
> > +   }
> > }
> >  
> > referenced = page_referenced(page, 1);
> > @@ -541,8 +545,10 @@ static unsigned long shrink_page_list(st
> > case PAGE_ACTIVATE:
> > goto activate_locked;
> > case PAGE_SUCCESS:
> > -   if (PageWriteback(page) || PageDirty(page))
> > +   if (PageWriteback(page) || PageDirty(page)) {
> > +   sc->nr_io_pages++;
> > goto keep;
> > +   }
> > /*
> >  * A synchronous write - probably a ramdisk.  Go
> >  * ahead and try to reclaim the page.
> > @@ -1201,6 +1207,7 @@ unsigned long try_to_free_pages(struct z
> >  
> > for (priority = DEF_PRIORITY; priority >= 0; priority--) {
> > sc.nr_scanned = 0;
> > +   sc.nr_io_pages = 0;
> > if (!priority)
> > disable_swap_token();
> > nr_reclaimed += shrink_zones(priority, zones, );
> > @@ -1229,7 +1236,8 @@ unsigned long try_to_free_pages(struct z
> > }
> >  
> > /* Take a nap, wait for some writeback to complete */
> > -   if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
> > +   if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
> > +   sc.nr_io_pages > sc.swap_cluster_max)
> 
> The comparison with swap_cluster_max is unobvious, and merits a
> comment.  What is the thinking here?  

If the number of pages undergoing IO is really small, waiting
for them may be a waste of time.

Maybe my thinking is wrong, not sure...

> Also, we now have this:
> 
>   if (total_scanned > sc.swap_cluster_max +
>   sc.swap_cluster_max / 2) {
>   wakeup_pdflush(laptop_mode ? 0 : total_scanned);
>   sc.may_writepage = 1;
>   }
> 
>   /* Take a nap, wait for some writeback to complete */
>   if (sc.nr_scanned && priority < DEF_PRIORITY - 2 && 
>   sc.nr_io_pages > sc.swap_cluster_max)
>   congestion_wait(WRITE, HZ/10);
> 
> 
> So in the case where total_scanned has not yet reached
> swap_cluster_max, this process isn't initiating writeout and it isn't
> sleeping, either.  Nor is it incrementing nr_io_pages.

Actually, nr_io_pages is also incremented when we run into pages that
are already PageWriteback - even if we did not start the IO ourselves.

> In the range (swap_cluster_max < nr_io_pages < 1.5*swap_cluster_max) this
> process still isn't incrementing nr_io_pages, but it _is_ running
> congestion_wait().

It is incrementing sc.nr_io_pages and will wait on IO to complete if
the amount of pages in flight to disk that it scanned over is larger
than the number of pages that it is trying to free.

> Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
> initiating IO and is throttling on writeback completion events.
> 
> This all seems a bit weird and arbitrary - what is the reason for
> throttling-but-not-writing in that 1.0->1.5 window?

Good question.  Note that the throttling-but-not-writing window in
the current code is 0.0->1.5, so this patch does reduce the throttling
window compared to the 

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 17:08:16 -0400
Rik van Riel <[EMAIL PROTECTED]> wrote:

> The current kswapd (and try_to_free_pages) code has an oddity where the
> code will wait on IO, even if there is no IO in flight.  This problem is
> notable especially when the system scans through many unfreeable pages,
> causing unnecessary stalls in the VM.
> 

What effect did this change have?

> 
> diff -up linux-2.6.22.x86_64/mm/vmscan.c.wait linux-2.6.22.x86_64/mm/vmscan.c
> --- linux-2.6.22.x86_64/mm/vmscan.c.wait  2007-09-25 11:33:30.0 
> -0400
> +++ linux-2.6.22.x86_64/mm/vmscan.c   2007-09-25 21:27:08.0 -0400
> @@ -68,6 +68,8 @@ struct scan_control {
>   int all_unreclaimable;
>  
>   int order;
> +
> + int nr_io_pages;
>  };
>  
>  #define lru_to_page(_head) (list_entry((_head)->prev, struct page, lru))
> @@ -489,8 +491,10 @@ static unsigned long shrink_page_list(st
>*/
>   if (sync_writeback == PAGEOUT_IO_SYNC && may_enter_fs)
>   wait_on_page_writeback(page);
> - else
> + else {
> + sc->nr_io_pages++;
>   goto keep_locked;
> + }
>   }
>  
>   referenced = page_referenced(page, 1);
> @@ -541,8 +545,10 @@ static unsigned long shrink_page_list(st
>   case PAGE_ACTIVATE:
>   goto activate_locked;
>   case PAGE_SUCCESS:
> - if (PageWriteback(page) || PageDirty(page))
> + if (PageWriteback(page) || PageDirty(page)) {
> + sc->nr_io_pages++;
>   goto keep;
> + }
>   /*
>* A synchronous write - probably a ramdisk.  Go
>* ahead and try to reclaim the page.
> @@ -1201,6 +1207,7 @@ unsigned long try_to_free_pages(struct z
>  
>   for (priority = DEF_PRIORITY; priority >= 0; priority--) {
>   sc.nr_scanned = 0;
> + sc.nr_io_pages = 0;
>   if (!priority)
>   disable_swap_token();
>   nr_reclaimed += shrink_zones(priority, zones, );
> @@ -1229,7 +1236,8 @@ unsigned long try_to_free_pages(struct z
>   }
>  
>   /* Take a nap, wait for some writeback to complete */
> - if (sc.nr_scanned && priority < DEF_PRIORITY - 2)
> + if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
> + sc.nr_io_pages > sc.swap_cluster_max)

The comparison with swap_cluster_max is unobvious, and merits a
comment.  What is the thinking here?  


Also, we now have this:

if (total_scanned > sc.swap_cluster_max +
sc.swap_cluster_max / 2) {
wakeup_pdflush(laptop_mode ? 0 : total_scanned);
sc.may_writepage = 1;
}

/* Take a nap, wait for some writeback to complete */
if (sc.nr_scanned && priority < DEF_PRIORITY - 2 &&
sc.nr_io_pages > sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);


So in the case where total_scanned has not yet reached
swap_cluster_max, this process isn't initiating writeout and it isn't
sleeping, either.  Nor is it incrementing nr_io_pages.

In the range (swap_cluster_max < nr_io_pages < 1.5*swap_cluster_max) this
process still isn't incrementing nr_io_pages, but it _is_ running
congestion_wait().

Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
initiating IO and is throttling on writeback completion events.

This all seems a bit weird and arbitrary - what is the reason for
throttling-but-not-writing in that 1.0->1.5 window?

If there _is_ a reason and it's all been carefully thought out and
designed, then can we please capture a description of that design in the
changelog or in the code?



Also, I wonder about what this change will do to the dynamic behaviour of
GFP_NOFS direct-reclaimers.  Previously they would throttle if they
encounter dirty pages which they can't write out.  Hopefully someone else
(kswapd or a __GFP_FS direct-reclaimer) will write some of those pages
and this caller will be woken when that writeout completes and will go off
and scoop them off the tail of the LRU.

But after this change, such a GFP_NOFS caller will, afacit, burn its way
through potentially the entire inactive list and will then declare oom. 
Non-preemtible uniprocessor kernels would be most at risk from this.


>   congestion_wait(WRITE, HZ/10);
>   }
>   /* top priority shrink_caches still had more to do? don't OOM, then */
> @@ -1315,6 +1323,7 @@ loop_again:
>   if (!priority)
>   

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 17:08:16 -0400
Rik van Riel [EMAIL PROTECTED] wrote:

 The current kswapd (and try_to_free_pages) code has an oddity where the
 code will wait on IO, even if there is no IO in flight.  This problem is
 notable especially when the system scans through many unfreeable pages,
 causing unnecessary stalls in the VM.
 

What effect did this change have?

 
 diff -up linux-2.6.22.x86_64/mm/vmscan.c.wait linux-2.6.22.x86_64/mm/vmscan.c
 --- linux-2.6.22.x86_64/mm/vmscan.c.wait  2007-09-25 11:33:30.0 
 -0400
 +++ linux-2.6.22.x86_64/mm/vmscan.c   2007-09-25 21:27:08.0 -0400
 @@ -68,6 +68,8 @@ struct scan_control {
   int all_unreclaimable;
  
   int order;
 +
 + int nr_io_pages;
  };
  
  #define lru_to_page(_head) (list_entry((_head)-prev, struct page, lru))
 @@ -489,8 +491,10 @@ static unsigned long shrink_page_list(st
*/
   if (sync_writeback == PAGEOUT_IO_SYNC  may_enter_fs)
   wait_on_page_writeback(page);
 - else
 + else {
 + sc-nr_io_pages++;
   goto keep_locked;
 + }
   }
  
   referenced = page_referenced(page, 1);
 @@ -541,8 +545,10 @@ static unsigned long shrink_page_list(st
   case PAGE_ACTIVATE:
   goto activate_locked;
   case PAGE_SUCCESS:
 - if (PageWriteback(page) || PageDirty(page))
 + if (PageWriteback(page) || PageDirty(page)) {
 + sc-nr_io_pages++;
   goto keep;
 + }
   /*
* A synchronous write - probably a ramdisk.  Go
* ahead and try to reclaim the page.
 @@ -1201,6 +1207,7 @@ unsigned long try_to_free_pages(struct z
  
   for (priority = DEF_PRIORITY; priority = 0; priority--) {
   sc.nr_scanned = 0;
 + sc.nr_io_pages = 0;
   if (!priority)
   disable_swap_token();
   nr_reclaimed += shrink_zones(priority, zones, sc);
 @@ -1229,7 +1236,8 @@ unsigned long try_to_free_pages(struct z
   }
  
   /* Take a nap, wait for some writeback to complete */
 - if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
 + if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
 + sc.nr_io_pages  sc.swap_cluster_max)

The comparison with swap_cluster_max is unobvious, and merits a
comment.  What is the thinking here?  


Also, we now have this:

if (total_scanned  sc.swap_cluster_max +
sc.swap_cluster_max / 2) {
wakeup_pdflush(laptop_mode ? 0 : total_scanned);
sc.may_writepage = 1;
}

/* Take a nap, wait for some writeback to complete */
if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
sc.nr_io_pages  sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);


So in the case where total_scanned has not yet reached
swap_cluster_max, this process isn't initiating writeout and it isn't
sleeping, either.  Nor is it incrementing nr_io_pages.

In the range (swap_cluster_max  nr_io_pages  1.5*swap_cluster_max) this
process still isn't incrementing nr_io_pages, but it _is_ running
congestion_wait().

Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
initiating IO and is throttling on writeback completion events.

This all seems a bit weird and arbitrary - what is the reason for
throttling-but-not-writing in that 1.0-1.5 window?

If there _is_ a reason and it's all been carefully thought out and
designed, then can we please capture a description of that design in the
changelog or in the code?



Also, I wonder about what this change will do to the dynamic behaviour of
GFP_NOFS direct-reclaimers.  Previously they would throttle if they
encounter dirty pages which they can't write out.  Hopefully someone else
(kswapd or a __GFP_FS direct-reclaimer) will write some of those pages
and this caller will be woken when that writeout completes and will go off
and scoop them off the tail of the LRU.

But after this change, such a GFP_NOFS caller will, afacit, burn its way
through potentially the entire inactive list and will then declare oom. 
Non-preemtible uniprocessor kernels would be most at risk from this.


   congestion_wait(WRITE, HZ/10);
   }
   /* top priority shrink_caches still had more to do? don't OOM, then */
 @@ -1315,6 +1323,7 @@ loop_again:
   if (!priority)
   disable_swap_token();
  
 + sc.nr_io_pages = 0;
   

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 14:47:02 -0700
Andrew Morton [EMAIL PROTECTED] wrote:

 On Thu, 27 Sep 2007 17:08:16 -0400
 Rik van Riel [EMAIL PROTECTED] wrote:
 
  The current kswapd (and try_to_free_pages) code has an oddity where the
  code will wait on IO, even if there is no IO in flight.  This problem is
  notable especially when the system scans through many unfreeable pages,
  causing unnecessary stalls in the VM.
  
 
 What effect did this change have?

Kswapd was no longer sitting in D state as often and pages got
freed more promptly.  The test was done on a RHEL kernel with
this change though - I guess I should redo it with a current upstream
kernel.

  diff -up linux-2.6.22.x86_64/mm/vmscan.c.wait 
  linux-2.6.22.x86_64/mm/vmscan.c
  --- linux-2.6.22.x86_64/mm/vmscan.c.wait2007-09-25 11:33:30.0 
  -0400
  +++ linux-2.6.22.x86_64/mm/vmscan.c 2007-09-25 21:27:08.0 -0400
  @@ -68,6 +68,8 @@ struct scan_control {
  int all_unreclaimable;
   
  int order;
  +
  +   int nr_io_pages;
   };
   
   #define lru_to_page(_head) (list_entry((_head)-prev, struct page, lru))
  @@ -489,8 +491,10 @@ static unsigned long shrink_page_list(st
   */
  if (sync_writeback == PAGEOUT_IO_SYNC  may_enter_fs)
  wait_on_page_writeback(page);
  -   else
  +   else {
  +   sc-nr_io_pages++;
  goto keep_locked;
  +   }
  }
   
  referenced = page_referenced(page, 1);
  @@ -541,8 +545,10 @@ static unsigned long shrink_page_list(st
  case PAGE_ACTIVATE:
  goto activate_locked;
  case PAGE_SUCCESS:
  -   if (PageWriteback(page) || PageDirty(page))
  +   if (PageWriteback(page) || PageDirty(page)) {
  +   sc-nr_io_pages++;
  goto keep;
  +   }
  /*
   * A synchronous write - probably a ramdisk.  Go
   * ahead and try to reclaim the page.
  @@ -1201,6 +1207,7 @@ unsigned long try_to_free_pages(struct z
   
  for (priority = DEF_PRIORITY; priority = 0; priority--) {
  sc.nr_scanned = 0;
  +   sc.nr_io_pages = 0;
  if (!priority)
  disable_swap_token();
  nr_reclaimed += shrink_zones(priority, zones, sc);
  @@ -1229,7 +1236,8 @@ unsigned long try_to_free_pages(struct z
  }
   
  /* Take a nap, wait for some writeback to complete */
  -   if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
  +   if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
  +   sc.nr_io_pages  sc.swap_cluster_max)
 
 The comparison with swap_cluster_max is unobvious, and merits a
 comment.  What is the thinking here?  

If the number of pages undergoing IO is really small, waiting
for them may be a waste of time.

Maybe my thinking is wrong, not sure...

 Also, we now have this:
 
   if (total_scanned  sc.swap_cluster_max +
   sc.swap_cluster_max / 2) {
   wakeup_pdflush(laptop_mode ? 0 : total_scanned);
   sc.may_writepage = 1;
   }
 
   /* Take a nap, wait for some writeback to complete */
   if (sc.nr_scanned  priority  DEF_PRIORITY - 2  
   sc.nr_io_pages  sc.swap_cluster_max)
   congestion_wait(WRITE, HZ/10);
 
 
 So in the case where total_scanned has not yet reached
 swap_cluster_max, this process isn't initiating writeout and it isn't
 sleeping, either.  Nor is it incrementing nr_io_pages.

Actually, nr_io_pages is also incremented when we run into pages that
are already PageWriteback - even if we did not start the IO ourselves.

 In the range (swap_cluster_max  nr_io_pages  1.5*swap_cluster_max) this
 process still isn't incrementing nr_io_pages, but it _is_ running
 congestion_wait().

It is incrementing sc.nr_io_pages and will wait on IO to complete if
the amount of pages in flight to disk that it scanned over is larger
than the number of pages that it is trying to free.

 Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
 initiating IO and is throttling on writeback completion events.
 
 This all seems a bit weird and arbitrary - what is the reason for
 throttling-but-not-writing in that 1.0-1.5 window?

Good question.  Note that the throttling-but-not-writing window in
the current code is 0.0-1.5, so this patch does reduce the throttling
window compared to the current code.

What is the reason that the current code does IO throttling even if
there is no IO at all in flight?

 If there _is_ a reason and it's all been carefully thought out 

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 18:13:25 -0400
Rik van Riel [EMAIL PROTECTED] wrote:

 On Thu, 27 Sep 2007 14:47:02 -0700
 Andrew Morton [EMAIL PROTECTED] wrote:
 
  On Thu, 27 Sep 2007 17:08:16 -0400
  Rik van Riel [EMAIL PROTECTED] wrote:
  
   The current kswapd (and try_to_free_pages) code has an oddity where the
   code will wait on IO, even if there is no IO in flight.  This problem is
   notable especially when the system scans through many unfreeable pages,
   causing unnecessary stalls in the VM.
   
  
  What effect did this change have?
 
 Kswapd was no longer sitting in D state as often and pages got
 freed more promptly.  The test was done on a RHEL kernel with
 this change though - I guess I should redo it with a current upstream
 kernel.

OK.  Yes, it should help quite a bit in the common cases.


 /* Take a nap, wait for some writeback to complete */
   - if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
   + if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
   + sc.nr_io_pages  sc.swap_cluster_max)
  
  The comparison with swap_cluster_max is unobvious, and merits a
  comment.  What is the thinking here?  
 
 If the number of pages undergoing IO is really small, waiting
 for them may be a waste of time.
 
 Maybe my thinking is wrong, not sure...

The thinking sounds good to me, but I'm looking for weirdo side-effects in
corner cases.  And I'm trying to work out what actual design we want to
have behind these various magic numbers and thresholds.

  Also, we now have this:
  
  if (total_scanned  sc.swap_cluster_max +
  sc.swap_cluster_max / 2) {
  wakeup_pdflush(laptop_mode ? 0 : total_scanned);
  sc.may_writepage = 1;
  }
  
  /* Take a nap, wait for some writeback to complete */
  if (sc.nr_scanned  priority  DEF_PRIORITY - 2  
  sc.nr_io_pages  sc.swap_cluster_max)
  congestion_wait(WRITE, HZ/10);
  
  
  So in the case where total_scanned has not yet reached
  swap_cluster_max, this process isn't initiating writeout and it isn't
  sleeping, either.  Nor is it incrementing nr_io_pages.
 
 Actually, nr_io_pages is also incremented when we run into pages that
 are already PageWriteback - even if we did not start the IO ourselves.

OK, that'll help a lot in this scenario.

  In the range (swap_cluster_max  nr_io_pages  1.5*swap_cluster_max) this
  process still isn't incrementing nr_io_pages, but it _is_ running
  congestion_wait().
 
 It is incrementing sc.nr_io_pages and will wait on IO to complete if
 the amount of pages in flight to disk that it scanned over is larger
 than the number of pages that it is trying to free.
 
  Once nr_io_pages exceeds 1.5*swap_cluster_max, this process is both
  initiating IO and is throttling on writeback completion events.
  
  This all seems a bit weird and arbitrary - what is the reason for
  throttling-but-not-writing in that 1.0-1.5 window?
 
 Good question.  Note that the throttling-but-not-writing window in
 the current code is 0.0-1.5, so this patch does reduce the throttling
 window compared to the current code.
 
 What is the reason that the current code does IO throttling even if
 there is no IO at all in flight?

Buggered if I know ;)

It may have the accidental effect that it opens a window in which some
may_enter_fs-capable process can get scheduled and do some writeout,
perhaps.

  If there _is_ a reason and it's all been carefully thought out and
  designed, then can we please capture a description of that design in the
  changelog or in the code?
 
 I'll add a description for the sc.nr_io_pages  sc.swap_cluster_max
 test.

OK, thanks.  Perhaps a few words tacked onto the nr_io_pages definition
site would be the place to capture this.

  Also, I wonder about what this change will do to the dynamic behaviour of
  GFP_NOFS direct-reclaimers.  Previously they would throttle if they
  encounter dirty pages which they can't write out.  Hopefully someone else
  (kswapd or a __GFP_FS direct-reclaimer) will write some of those pages
  and this caller will be woken when that writeout completes and will go off
  and scoop them off the tail of the LRU.
  
  But after this change, such a GFP_NOFS caller will, afacit, burn its way
  through potentially the entire inactive list and will then declare oom. 
 
 Nope, sc.nr_io_pages will also be incremented when the code runs into
 pages that are already PageWriteback.

yup, I didn't think of that.  Hopefully someone else will be in there
working on that zone too.  If this caller yields and defers to kswapd
then that's very likely.  Except we just took away the ability to do that..

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 15:21:21 -0700
Andrew Morton [EMAIL PROTECTED] wrote:

  Nope, sc.nr_io_pages will also be incremented when the code runs into
  pages that are already PageWriteback.
 
 yup, I didn't think of that.  Hopefully someone else will be in there
 working on that zone too.  If this caller yields and defers to kswapd
 then that's very likely.  Except we just took away the ability to do that..

if (PageDirty(page)) {
if (sc-order = PAGE_ALLOC_COSTLY_ORDER  referenced)
goto keep_locked;
if (!may_enter_fs)
goto keep_locked;

I think we can fix that problem by adding a sc-nr_io_pages++
between the last if and the goto keep_locked in shrink_page_list.

That way !GFP_IO or !GFP_FS tasks will cause themselves to sleep
if there are pages that need to be written out, even if those
pages are not in flight to disk yet.

I have also added the comment you wanted.

Signed-off-by: Rik van Riel [EMAIL PROTECTED]

diff -up linux-2.6.23-rc7/mm/vmscan.c.wait linux-2.6.22/mm/vmscan.c
--- linux-2.6.23-rc7/mm/vmscan.c.wait   2007-09-27 18:45:57.0 -0400
+++ linux-2.6.23-rc7/mm/vmscan.c2007-09-27 18:48:43.0 -0400
@@ -68,6 +68,13 @@ struct scan_control {
int all_unreclaimable;
 
int order;
+
+   /*
+* Pages that have (or should have) IO pending.  If we run into
+* a lot of these, we're better off waiting a little for IO to
+* finish rather than scanning more pages in the VM.
+*/
+   int nr_io_pages;
 };
 
 #define lru_to_page(_head) (list_entry((_head)-prev, struct page, lru))
@@ -489,8 +496,10 @@ static unsigned long shrink_page_list(st
 */
if (sync_writeback == PAGEOUT_IO_SYNC  may_enter_fs)
wait_on_page_writeback(page);
-   else
+   else {
+   sc-nr_io_pages++;
goto keep_locked;
+   }
}
 
referenced = page_referenced(page, 1);
@@ -529,8 +538,10 @@ static unsigned long shrink_page_list(st
if (PageDirty(page)) {
if (sc-order = PAGE_ALLOC_COSTLY_ORDER  referenced)
goto keep_locked;
-   if (!may_enter_fs)
+   if (!may_enter_fs) {
+   sc-nr_io_pages++;
goto keep_locked;
+   }
if (!sc-may_writepage)
goto keep_locked;
 
@@ -541,8 +552,10 @@ static unsigned long shrink_page_list(st
case PAGE_ACTIVATE:
goto activate_locked;
case PAGE_SUCCESS:
-   if (PageWriteback(page) || PageDirty(page))
+   if (PageWriteback(page) || PageDirty(page)) {
+   sc-nr_io_pages++;
goto keep;
+   }
/*
 * A synchronous write - probably a ramdisk.  Go
 * ahead and try to reclaim the page.
@@ -1201,6 +1214,7 @@ unsigned long try_to_free_pages(struct z
 
for (priority = DEF_PRIORITY; priority = 0; priority--) {
sc.nr_scanned = 0;
+   sc.nr_io_pages = 0;
if (!priority)
disable_swap_token();
nr_reclaimed += shrink_zones(priority, zones, sc);
@@ -1229,7 +1243,8 @@ unsigned long try_to_free_pages(struct z
}
 
/* Take a nap, wait for some writeback to complete */
-   if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
+   if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
+   sc.nr_io_pages  sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
}
/* top priority shrink_caches still had more to do? don't OOM, then */
@@ -1315,6 +1330,7 @@ loop_again:
if (!priority)
disable_swap_token();
 
+   sc.nr_io_pages = 0;
all_zones_ok = 1;
 
/*
@@ -1398,7 +1414,8 @@ loop_again:
 * OK, kswapd is getting into trouble.  Take a nap, then take
 * another pass across the zones.
 */
-   if (total_scanned  priority  DEF_PRIORITY - 2)
+   if (total_scanned  priority  DEF_PRIORITY - 2 
+   sc.nr_io_pages  sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
 
/*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the 

Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Andrew Morton
On Thu, 27 Sep 2007 18:50:27 -0400
Rik van Riel [EMAIL PROTECTED] wrote:

 On Thu, 27 Sep 2007 15:21:21 -0700
 Andrew Morton [EMAIL PROTECTED] wrote:
 
   Nope, sc.nr_io_pages will also be incremented when the code runs into
   pages that are already PageWriteback.
  
  yup, I didn't think of that.  Hopefully someone else will be in there
  working on that zone too.  If this caller yields and defers to kswapd
  then that's very likely.  Except we just took away the ability to do that..
 
 if (PageDirty(page)) {
 if (sc-order = PAGE_ALLOC_COSTLY_ORDER  
 referenced)
 goto keep_locked;
 if (!may_enter_fs)
 goto keep_locked;
 
 I think we can fix that problem by adding a sc-nr_io_pages++
 between the last if and the goto keep_locked in shrink_page_list.
 
 That way !GFP_IO or !GFP_FS tasks will cause themselves to sleep
 if there are pages that need to be written out, even if those
 pages are not in flight to disk yet.

yeah, that's prudent I guess.

 I have also added the comment you wanted.

And lost the changelog ;)

 - if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
 + if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
 + sc.nr_io_pages  sc.swap_cluster_max)

I do think this design decision needs a bit of explanation too.

   congestion_wait(WRITE, HZ/10);
   }
   /* top priority shrink_caches still had more to do? don't OOM, then */
 @@ -1315,6 +1330,7 @@ loop_again:
   if (!priority)
   disable_swap_token();
  
 + sc.nr_io_pages = 0;
   all_zones_ok = 1;
  
   /*
 @@ -1398,7 +1414,8 @@ loop_again:
* OK, kswapd is getting into trouble.  Take a nap, then take
* another pass across the zones.
*/
 - if (total_scanned  priority  DEF_PRIORITY - 2)
 + if (total_scanned  priority  DEF_PRIORITY - 2 

As did that one.  Ho hum :(  Maybe it's in the git history somewhere.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] kswapd should only wait on IO if there is IO

2007-09-27 Thread Rik van Riel
On Thu, 27 Sep 2007 15:59:07 -0700
Andrew Morton [EMAIL PROTECTED] wrote:

 And lost the changelog ;)

Good point.

The current kswapd (and try_to_free_pages) code has an oddity where the
code will wait on IO, even if there is no IO in flight.  This problem is
notable especially when the system scans through many unfreeable pages,
causing unnecessary stalls in the VM.

Additionally, tasks without __GFP_FS or __GFP_IO in the direct reclaim
path will sleep if a significant number of pages are encountered that
should be written out.  This gives kswapd a chance to write out those
pages, while the direct reclaim task sleeps.

Signed-off-by: Rik van Riel [EMAIL PROTECTED]

diff -up linux-2.6.22/mm/vmscan.c.wait linux-2.6.22/mm/vmscan.c
--- linux-2.6.22/mm/vmscan.c.wait   2007-09-27 18:45:57.0 -0400
+++ linux-2.6.22/mm/vmscan.c2007-09-27 18:48:43.0 -0400
@@ -68,6 +68,13 @@ struct scan_control {
int all_unreclaimable;
 
int order;
+
+   /*
+* Pages that have (or should have) IO pending.  If we run into
+* a lot of these, we're better off waiting a little for IO to
+* finish rather than scanning more pages in the VM.
+*/
+   int nr_io_pages;
 };
 
 #define lru_to_page(_head) (list_entry((_head)-prev, struct page, lru))
@@ -489,8 +496,10 @@ static unsigned long shrink_page_list(st
 */
if (sync_writeback == PAGEOUT_IO_SYNC  may_enter_fs)
wait_on_page_writeback(page);
-   else
+   else {
+   sc-nr_io_pages++;
goto keep_locked;
+   }
}
 
referenced = page_referenced(page, 1);
@@ -529,8 +538,10 @@ static unsigned long shrink_page_list(st
if (PageDirty(page)) {
if (sc-order = PAGE_ALLOC_COSTLY_ORDER  referenced)
goto keep_locked;
-   if (!may_enter_fs)
+   if (!may_enter_fs) {
+   sc-nr_io_pages++;
goto keep_locked;
+   }
if (!sc-may_writepage)
goto keep_locked;
 
@@ -541,8 +552,10 @@ static unsigned long shrink_page_list(st
case PAGE_ACTIVATE:
goto activate_locked;
case PAGE_SUCCESS:
-   if (PageWriteback(page) || PageDirty(page))
+   if (PageWriteback(page) || PageDirty(page)) {
+   sc-nr_io_pages++;
goto keep;
+   }
/*
 * A synchronous write - probably a ramdisk.  Go
 * ahead and try to reclaim the page.
@@ -1201,6 +1214,7 @@ unsigned long try_to_free_pages(struct z
 
for (priority = DEF_PRIORITY; priority = 0; priority--) {
sc.nr_scanned = 0;
+   sc.nr_io_pages = 0;
if (!priority)
disable_swap_token();
nr_reclaimed += shrink_zones(priority, zones, sc);
@@ -1229,7 +1243,8 @@ unsigned long try_to_free_pages(struct z
}
 
/* Take a nap, wait for some writeback to complete */
-   if (sc.nr_scanned  priority  DEF_PRIORITY - 2)
+   if (sc.nr_scanned  priority  DEF_PRIORITY - 2 
+   sc.nr_io_pages  sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
}
/* top priority shrink_caches still had more to do? don't OOM, then */
@@ -1315,6 +1330,7 @@ loop_again:
if (!priority)
disable_swap_token();
 
+   sc.nr_io_pages = 0;
all_zones_ok = 1;
 
/*
@@ -1398,7 +1414,8 @@ loop_again:
 * OK, kswapd is getting into trouble.  Take a nap, then take
 * another pass across the zones.
 */
-   if (total_scanned  priority  DEF_PRIORITY - 2)
+   if (total_scanned  priority  DEF_PRIORITY - 2 
+   sc.nr_io_pages  sc.swap_cluster_max)
congestion_wait(WRITE, HZ/10);
 
/*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/