The attached patch does what I described in the other thread, it
makes the pageout code free swap space when swap is getting full,
by taking away the swap space from pages that get moved onto or
back onto the active list.
In some tests on a system with 2GB RAM and 1GB swap, it kept the
free swap at 500MB for a 2.3GB qsbench, while without the patch
over 950MB of swap was in use all of the time.
This should give kswapd more flexibility in what to swap out.
What do you think?
Signed-off-by: Rik van Riel <[email protected]>
--
Politics is the struggle between those who want to make their country
the best in the world, and those who believe it already is. Each group
calls the other unpatriotic.
On Fri, 16 Feb 2007, Rik van Riel wrote:
> What do you think?
Looks good apart from one passage (which just vanished when I tried to
reply, please post patches as inline text).
It was the portion that modifies shrink_active_list. Why operate
on the pagevec there? The pagevec only contains the leftovers to be
released from scanning over the temporary inactive list.
Christoph Lameter wrote:
> On Fri, 16 Feb 2007, Rik van Riel wrote:
>
>> What do you think?
>
> Looks good apart from one passage (which just vanished when I tried to
> reply, please post patches as inline text).
>
> It was the portion that modifies shrink_active_list. Why operate
> on the pagevec there? The pagevec only contains the leftovers to be
> released from scanning over the temporary inactive list.
Why? Because the pages that were not referenced will be
going onto the inactive list and are now a candidate for
swapping out. I don't see why we would want to reclaim
the swap space for pages that area about to be swapped
out again.
--
Politics is the struggle between those who want to make their country
the best in the world, and those who believe it already is. Each group
calls the other unpatriotic.
On Tue, 20 Feb 2007, Rik van Riel wrote:
> > It was the portion that modifies shrink_active_list. Why operate
> > on the pagevec there? The pagevec only contains the leftovers to be released
> > from scanning over the temporary inactive list.
>
> Why? Because the pages that were not referenced will be
> going onto the inactive list and are now a candidate for
> swapping out. I don't see why we would want to reclaim
> the swap space for pages that area about to be swapped
> out again.
Sounds sane. Then drop that piece. Again, you were only operating on the
pages left over in the pagevec after the move of the pages to the
inactive list. If you really wanted to do something there then the
processing should have covered all pages that go to the inactive list.
Christoph Lameter wrote:
> On Tue, 20 Feb 2007, Rik van Riel wrote:
>
>>> It was the portion that modifies shrink_active_list. Why operate
>>> on the pagevec there? The pagevec only contains the leftovers to be released
>>> from scanning over the temporary inactive list.
>> Why? Because the pages that were not referenced will be
>> going onto the inactive list and are now a candidate for
>> swapping out. I don't see why we would want to reclaim
>> the swap space for pages that area about to be swapped
>> out again.
>
> Sounds sane. Then drop that piece. Again, you were only operating on the
> pages left over in the pagevec after the move of the pages to the
> inactive list. If you really wanted to do something there then the
> processing should have covered all pages that go to the inactive list.
Nono, I try to remove the swap space occupied by pages that
go back onto the active list. Regardless of whether they
were already there, or whether they started out on the
inactive list.
Stripping the swap space of the pages that are going to
the inactive list makes less sense IMHO, because those
pages are candidates for swapping out - meaning those
should keep the space.
--
All Rights Reversed
On Tue, 20 Feb 2007, Rik van Riel wrote:
> Nono, I try to remove the swap space occupied by pages that
> go back onto the active list. Regardless of whether they
> were already there, or whether they started out on the
> inactive list.
Ok then do it for all pages that go back not just for those leftover from
the moving of pages to the inactive list (why would you move those???)
> Stripping the swap space of the pages that are going to
> the inactive list makes less sense IMHO, because those
> pages are candidates for swapping out - meaning those
> should keep the space.
Just trying to figure out what your patch does there and it does not make
much sense to me so far.
Maybe the hunk does apply in a different location than I thought. If you
do that in the loop over the pages on active list then it would make
sense. But in that case you need another piece of it doing the same to the
pages that are released at the end of shrink_active_list().
On Fri, Feb 16, 2007 at 05:46:29PM -0500, Rik van Riel wrote:
> The attached patch does what I described in the other thread, it
> makes the pageout code free swap space when swap is getting full,
> by taking away the swap space from pages that get moved onto or
> back onto the active list.
> In some tests on a system with 2GB RAM and 1GB swap, it kept the
> free swap at 500MB for a 2.3GB qsbench, while without the patch
> over 950MB of swap was in use all of the time.
> This should give kswapd more flexibility in what to swap out.
> What do you think?
> Signed-off-by: Rik van Riel <[email protected]>
I would call this a bugfix, not an optimization.
-- wli
On Tue, 20 Feb 2007, Rik van Riel wrote:
> > Maybe the hunk does apply in a different location than I thought.
>
> I suspect that's the case ...
No that is not the case:
@@ -875,6 +878,11 @@ force_reclaim_mapped:
pagevec_strip(&pvec);
spin_lock_irq(&zone->lru_lock);
}
+ if (vm_swap_full()) {
+ spin_unlock_irq(&zone->lru_lock);
+ pagevec_swap_free(&pvec);
+ spin_lock_irq(&zone->lru_lock);
+ }
pgmoved = 0;
while (!list_empty(&l_active)) {
So you do the swap free on the pages leftover from moving to the inactive
list and not to the pages that will be moved to the active list. It does
not do what you wanted to be done.
You need to
1. Do the pagevec_swap_free in the loop over the active pages
2. At end of the loop over the active pages do something like the above.
Christoph Lameter wrote:
> On Tue, 20 Feb 2007, Rik van Riel wrote:
>
>> Nono, I try to remove the swap space occupied by pages that
>> go back onto the active list. Regardless of whether they
>> were already there, or whether they started out on the
>> inactive list.
>
> Ok then do it for all pages that go back not just for those leftover from
> the moving of pages to the inactive list (why would you move those???)
I do. The only pages that are exempt are the pages that move
from the active list to the inactive list, because those will
probably be evicted soon enough.
> Maybe the hunk does apply in a different location than I thought.
I suspect that's the case ...
> If you
> do that in the loop over the pages on active list then it would make
> sense. But in that case you need another piece of it doing the same to the
> pages that are released at the end of shrink_active_list().
... because I think this is what my patch does :)
--
All Rights Reversed
Rik van Riel wrote:
> ... because I think this is what my patch does :)
Never mind, I see it now.
The attached patch should be correct.
Btw, why do we not call pagevec_strip on the pages on l_active?
I assume we want to reclaim their buffer heads, too...
--
All Rights Reversed
On Tue, 20 Feb 2007, Rik van Riel wrote:
> The attached patch should be correct.
Oh. It vanished again when I replied to your mail.
> Btw, why do we not call pagevec_strip on the pages on l_active?
> I assume we want to reclaim their buffer heads, too...
Yes we want to reduce buffer heads if we are over the limit.
On Tue, 20 Feb 2007, Rik van Riel wrote:
> Btw, why do we not call pagevec_strip on the pages on l_active?
> I assume we want to reclaim their buffer heads, too...
But those buffer heads may be used soon. So its better to leave them
alone.
Rik van Riel wrote:
> Rik van Riel wrote:
> > ... because I think this is what my patch does :)
>
> Never mind, I see it now.
>
> The attached patch should be correct.
Your patch seems to improve the situation a little bit, but the numbers still
look weird, especially for swap-in, which gets progressively slower.
RAM 512mb , SWAP 1G
#mount -t tmpfs -o size=1G none /dev/shm
#time cat /dev/full > /dev/shm/x.dmp
15sec
#time cat /dev/shm/x.dmp > /dev/null
58sec
#time cat /dev/shm/x.dmp > /dev/null
72sec
#time cat /dev/shm/x.dmp > /dev/null
85sec
#time cat /dev/shm/x.dmp > /dev/null
93sec
#time cat /dev/shm/x.dmp > /dev/null
99sec
Thanks!
--
Al
Rik van Riel <[email protected]> wrote:
+++ linux-2.6.20.noarch/mm/swap.c????????2007-02-20 06:44:17.000000000 -0500
@@ -420,6 +420,26 @@ void pagevec_strip(struct pagevec *pvec)
> +????????????????????????if (printk_ratelimit())
> +????????????????????????????????printk("kswapd freed a swap space\n");
>
NOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO!!!!!!!!!!!
1) This message is a debug message! You forgot to set the printk level.
2) The message text is bad, the average log reader only knows swap files
and pages in swapfiles. The first reaction will be like "WTF happened
to my swap???". Thousands of admins will cry out in anguish trying to
get the meaning of this message, and again cry out in wrath after they
found out.
3) What should I do if I see this message? It's neither good, nor bad for
me, nor is it in any way informative even if it were changed to be
meaningfull. It's utterly useless! Let it be!
--
We are all born ignorant, but one must work hard to remain stupid.
-- Benjamin Franklin
Fri?, Spammer: [email protected]
Bodo Eggert wrote:
> Rik van Riel <[email protected]> wrote:
>
> +++ linux-2.6.20.noarch/mm/swap.c 2007-02-20 06:44:17.000000000 -0500
> @@ -420,6 +420,26 @@ void pagevec_strip(struct pagevec *pvec)
>
>> + if (printk_ratelimit())
>> + printk("kswapd freed a swap space\n");
>>
>
> NOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO!!!!!!!!!!!
>
> 1) This message is a debug message! You forgot to set the printk level.
Doh, I forgot to cut it out of the patch when I fixed it
according to Christoph's hint.
This chunk should be removed from the patch...
Andrew, I'll send you a new one tomorrow morning.
--
Politics is the struggle between those who want to make their country
the best in the world, and those who believe it already is. Each group
calls the other unpatriotic.