[PATCH] Do not mark being-truncated-pages as cache hot

[PATCH] Do not mark being-truncated-pages as cache hot

[Marcelo Tosatti]

Hi, 

The truncate VM functions use pagevec's for operation batching, but they mark
the pagevec used to hold being-truncated-pages as "cache hot". 

There is nothing which indicates such pages are likely to be "cache hot" - the
following patch marks being-truncated-pages as cold instead. 

Please apply.


BTW Martin, I'm wondering on a few performance points about the per_cpu_page lists, 
as we talked on chat before. Here they are:

- I wonder if the size of the lists are optimal. They might be too big to fit into the caches.

- Making the allocation policy FIFO should drastically increase the chances "hot" pages
are handed to the allocator. AFAIK the policy now is LIFO.

- When we we hit the high per_cpu_pages watermark, which can easily happen,
further hot pages being freed are send down to the SLAB manager, until 
the pcp count goes below the high watermark. Meaning that during this period 
the hot/cold logic goes down the drain.

But the main point of the pcp lists, which is to avoid locking AFAIK, is not affected
by the issues I describe.

Comments?

--- linux-2.6.9-rc1-mm5/mm/truncate.c.orig	2004-09-13 19:43:08.454659904 -0300
+++ linux-2.6.9-rc1-mm5/mm/truncate.c	2004-09-13 19:45:00.765586048 -0300
@@ -133,7 +133,7 @@ void truncate_inode_pages_range(struct a
 	BUG_ON((lend & (PAGE_CACHE_SIZE - 1)) != (PAGE_CACHE_SIZE - 1));
 	end = (lend >> PAGE_CACHE_SHIFT);
 
-	pagevec_init(&pvec, 0);
+	pagevec_init(&pvec, 1);
 	next = start;
 	while (next <= end &&
 	       pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {
@@ -237,7 +237,7 @@ unsigned long invalidate_mapping_pages(s
 	unsigned long ret = 0;
 	int i;
 
-	pagevec_init(&pvec, 0);
+	pagevec_init(&pvec, 1);
 	while (next <= end &&
 			pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {
 		for (i = 0; i < pagevec_count(&pvec); i++) {
@@ -293,7 +293,7 @@ void invalidate_inode_pages2(struct addr
 	pgoff_t next = 0;
 	int i;
 
-	pagevec_init(&pvec, 0);
+	pagevec_init(&pvec, 1);
 	while (pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {
 		for (i = 0; i < pagevec_count(&pvec); i++) {
 			struct page *page = pvec.pages[i];
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Andrew Morton]

Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote:
>
> The truncate VM functions use pagevec's for operation batching, but they mark
>  the pagevec used to hold being-truncated-pages as "cache hot". 
> 
>  There is nothing which indicates such pages are likely to be "cache hot" - the
>  following patch marks being-truncated-pages as cold instead. 

Disagree.

	blah > /tmp/foo
	rm /tmp/foo

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Marcelo Tosatti]

On Mon, Sep 13, 2004 at 04:45:10PM -0700, Andrew Morton wrote:
> Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote:
> >
> > The truncate VM functions use pagevec's for operation batching, but they mark
> >  the pagevec used to hold being-truncated-pages as "cache hot". 
> > 
> >  There is nothing which indicates such pages are likely to be "cache hot" - the
> >  following patch marks being-truncated-pages as cold instead. 
> 
> Disagree.
> 
> 	blah > /tmp/foo
> 	rm /tmp/foo

Well thats sys_unlink(). It does truncate?

Anyway, I think thats not the common case at all. 
Hum, it depends on the workload, but still.

But yeah, you have a point. 
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Martin J. Bligh]

> The truncate VM functions use pagevec's for operation batching, but they mark
> the pagevec used to hold being-truncated-pages as "cache hot". 
> 
> There is nothing which indicates such pages are likely to be "cache hot" - the
> following patch marks being-truncated-pages as cold instead. 

Are they coming from the reclaim path? looks at a glance like they are,
in which case cold would definitely be correct. 

> BTW Martin, I'm wondering on a few performance points about the per_cpu_page lists, 
> as we talked on chat before. Here they are:
> 
> - I wonder if the size of the lists are optimal. They might be too big to fit into the caches.

Doesn't really matter that much if they are over-sized, it doesn't do all
that much harm, but it would be better if we sized it off the CPUs actual
cache size. Does anyone know a consistent way to get that across arches?
 
> - Making the allocation policy FIFO should drastically increase the chances "hot" pages
> are handed to the allocator. AFAIK the policy now is LIFO.

It should definitely have been FIFO to start with ... at least that was
the intent. free_hot_cold_page is doing list_add between head and head->next, buffered_rmqueue is doing list_del from the head, AFAICS, so it should work.

> - When we we hit the high per_cpu_pages watermark, which can easily happen,
> further hot pages being freed are send down to the SLAB manager, until 
> the pcp count goes below the high watermark. Meaning that during this period 
> the hot/cold logic goes down the drain.

Well, we should be freeing off the BACK end of the FIFO stack into the page
allocator - I haven't checked it but that was the intent.

> But the main point of the pcp lists, which is to avoid locking AFAIK, 
> is not affected by the issues I describe.

Well, it's both - they both had a fairly significant effect, IIRC.

M.
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Marcelo Tosatti]

On Mon, Sep 13, 2004 at 05:03:25PM -0700, Martin J. Bligh wrote:
> > The truncate VM functions use pagevec's for operation batching, but they mark
> > the pagevec used to hold being-truncated-pages as "cache hot". 
> > 
> > There is nothing which indicates such pages are likely to be "cache hot" - the
> > following patch marks being-truncated-pages as cold instead. 
> 
> Are they coming from the reclaim path? looks at a glance like they are,
> in which case cold would definitely be correct. 

No - its from the truncate() path? They might have come from the reclaim path when 
they were allocated and mapped.

> > BTW Martin, I'm wondering on a few performance points about the per_cpu_page lists, 
> > as we talked on chat before. Here they are:
> > 
> > - I wonder if the size of the lists are optimal. They might be too big to fit into the caches.
> 
> Doesn't really matter that much if they are over-sized, it doesn't do all
> that much harm, but it would be better if we sized it off the CPUs actual
> cache size. Does anyone know a consistent way to get that across arches?
>  
> > - Making the allocation policy FIFO should drastically increase the chances "hot" pages
> > are handed to the allocator. AFAIK the policy now is LIFO.
> 
> It should definitely have been FIFO to start with ... at least that was
> the intent. free_hot_cold_page is doing list_add between head and head->next, buffered_rmqueue is doing list_del from the head, AFAICS, so it should work.

Oh yes correct.

                        page = list_entry(pcp->list.next, struct page, lru);

I missed that "next".

> > - When we we hit the high per_cpu_pages watermark, which can easily happen,
> > further hot pages being freed are send down to the SLAB manager, until 
> > the pcp count goes below the high watermark. Meaning that during this period 
> > the hot/cold logic goes down the drain.
> 
> Well, we should be freeing off the BACK end of the FIFO stack into the page
> allocator - I haven't checked it but that was the intent.

static void fastcall free_hot_cold_page(struct page *page, int cold)
{
...
        if (pcp->count >= pcp->high)
                pcp->count -= free_pages_bulk(zone, pcp->batch, &pcp->list, 0);

So when we hit the high watermark, "hotter" pages are sent back to SLAB. 
I dont think this is optimal, wonder if moving pages at the end of the pcp
to SLAB and moving the being-freed-page to start of pcp (so to be processed 
as "hottest") will make a difference.

AFAICS its the best thing to do wrt better cache usage.

> > But the main point of the pcp lists, which is to avoid locking AFAIK, 
> > is not affected by the issues I describe.
> 
> Well, it's both - they both had a fairly significant effect, IIRC.

Do you have the numbers at the time you wrote the it?

Thanks for your comments guys!
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Andrew Morton]

Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote:
>
> So when we hit the high watermark, "hotter" pages are sent back to SLAB. 

That would be a bug.

free_hot_cold_page() sticks the being-freed page at ->next, while
free_pages_bulk() frees pages at ->prev.   Looks OK? 
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Marcelo Tosatti]

On Mon, Sep 13, 2004 at 06:21:48PM -0700, Andrew Morton wrote:
> Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote:
> >
> > So when we hit the high watermark, "hotter" pages are sent back to SLAB. 
> 
> That would be a bug.
> 
> free_hot_cold_page() sticks the being-freed page at ->next, while
> free_pages_bulk() frees pages at ->prev.   Looks OK? 

Yes looks OK, I misinterpreted.
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Andrew Morton]

"Martin J. Bligh" <mbligh@aracnet.com> wrote:
>
> > - Making the allocation policy FIFO should drastically increase the chances "hot" pages
>  > are handed to the allocator. AFAIK the policy now is LIFO.
> 
>  It should definitely have been FIFO to start with

I always intended that it be LIFO.  Take the hottest page, for heavens
sake.  As the oldest page is the one which is most likely to have fallen
out of cache then why a-priori choose it?
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Martin J. Bligh]

> "Martin J. Bligh" <mbligh@aracnet.com> wrote:
>> 
>> > - Making the allocation policy FIFO should drastically increase the chances "hot" pages
>>  > are handed to the allocator. AFAIK the policy now is LIFO.
>> 
>>  It should definitely have been FIFO to start with
> 
> I always intended that it be LIFO.  Take the hottest page, for heavens
> sake.  As the oldest page is the one which is most likely to have fallen
> out of cache then why a-priori choose it?

Bah. I just had my acronyms backwards ... I meant LIFO, sorry ;-)

M.

--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Do not mark being-truncated-pages as cache hot

[Marcelo Tosatti]

On Mon, Sep 13, 2004 at 05:24:32PM -0700, Martin J. Bligh wrote:
> > "Martin J. Bligh" <mbligh@aracnet.com> wrote:
> >> 
> >> > - Making the allocation policy FIFO should drastically increase the chances "hot" pages
> >>  > are handed to the allocator. AFAIK the policy now is LIFO.
> >> 
> >>  It should definitely have been FIFO to start with
> > 
> > I always intended that it be LIFO.  Take the hottest page, for heavens
> > sake.  As the oldest page is the one which is most likely to have fallen
> > out of cache then why a-priori choose it?
> 
> Bah. I just had my acronyms backwards ... I meant LIFO, sorry ;-)

I inverted both in the first email, I meant the other way around. Sorry :)
--
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:"aart@kvack.org"> aart@kvack.org </a>