[PATCH] ramfs: fix memleak on no-mmu arch

[PATCH] ramfs: fix memleak on no-mmu arch

[Bob Liu]

On no-mmu arch, there is a memleak duirng shmem test.
The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
refcount to 2 which makes iput() can't free that pages.

The simple test file is like this:
int main(void)
{
	int i;
	key_t k = ftok("/etc", 42);

	for ( i=0; i<100; ++i) {
		int id = shmget(k, 10000, 0644|IPC_CREAT);
		if (id == -1) {
			printf("shmget error\n");
		}
		if(shmctl(id, IPC_RMID, NULL ) == -1) {
			printf("shm  rm error\n");
			return -1;
		}
	}
	printf("run ok...\n");
	return 0;
}

And the result:
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16644        43676            0            0
-/+ buffers:              16644        43676
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        17912        42408            0            0
-/+ buffers:              17912        42408
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        19096        41224            0            0
-/+ buffers:              19096        41224
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        20296        40024            0            0
-/+ buffers:              20296        40024
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        21496        38824            0            0
-/+ buffers:              21496        38824
root:/> shmem 
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        22692        37628            0            0
-/+ buffers:              22692        37628
root:/> 

After this patch the test result is:(no memleak anymore)
root:/> 
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16580        43740            0            0
-/+ buffers:              16580        43740
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> 

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 fs/ramfs/file-nommu.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 9eead2c..fbb0b47 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
 		SetPageDirty(page);
 
 		unlock_page(page);
+		put_page(page);
 	}
 
 	return 0;
-- 
1.6.3.3

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Andrew Morton]

On Mon, 28 Mar 2011 13:32:35 +0800
Bob Liu <lliubbo@gmail.com> wrote:

> On no-mmu arch, there is a memleak duirng shmem test.
> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
> refcount to 2 which makes iput() can't free that pages.
> 
> The simple test file is like this:
> int main(void)
> {
> 	int i;
> 	key_t k = ftok("/etc", 42);
> 
> 	for ( i=0; i<100; ++i) {
> 		int id = shmget(k, 10000, 0644|IPC_CREAT);
> 		if (id == -1) {
> 			printf("shmget error\n");
> 		}
> 		if(shmctl(id, IPC_RMID, NULL ) == -1) {
> 			printf("shm  rm error\n");
> 			return -1;
> 		}
> 	}
> 	printf("run ok...\n");
> 	return 0;
> }
> 
> ...
> 
> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
> index 9eead2c..fbb0b47 100644
> --- a/fs/ramfs/file-nommu.c
> +++ b/fs/ramfs/file-nommu.c
> @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
>  		SetPageDirty(page);
>  
>  		unlock_page(page);
> +		put_page(page);
>  	}
>  
>  	return 0;

Something is still wrong here.

A live, in-use page should have a refcount of three.  One for the
existence of the page, one for its presence on the page LRU and one for
its existence in the pagecache radix tree.

So allocation should do:

	alloc_pages()
	add_to_page_cache()
	add_to_lru()

and deallocation should do

	remove_from_lru()
	remove_from_page_cache()
	put_page()

If this protocol is followed correctly, there is no need to do a
put_page() during the allocation/setup phase!

I suspect that the problem in nommu really lies in the
deallocation/teardown phase.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Bob Liu]

On Tue, Mar 29, 2011 at 8:02 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Mon, 28 Mar 2011 13:32:35 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> On no-mmu arch, there is a memleak duirng shmem test.
>> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
>> refcount to 2 which makes iput() can't free that pages.
>>
>> The simple test file is like this:
>> int main(void)
>> {
>>       int i;
>>       key_t k = ftok("/etc", 42);
>>
>>       for ( i=0; i<100; ++i) {
>>               int id = shmget(k, 10000, 0644|IPC_CREAT);
>>               if (id == -1) {
>>                       printf("shmget error\n");
>>               }
>>               if(shmctl(id, IPC_RMID, NULL ) == -1) {
>>                       printf("shm  rm error\n");
>>                       return -1;
>>               }
>>       }
>>       printf("run ok...\n");
>>       return 0;
>> }
>>
>> ...
>>
>> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
>> index 9eead2c..fbb0b47 100644
>> --- a/fs/ramfs/file-nommu.c
>> +++ b/fs/ramfs/file-nommu.c
>> @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
>>               SetPageDirty(page);
>>
>>               unlock_page(page);
>> +             put_page(page);
>>       }
>>
>>       return 0;
>
> Something is still wrong here.
>
> A live, in-use page should have a refcount of three.  One for the
> existence of the page, one for its presence on the page LRU and one for
> its existence in the pagecache radix tree.
>
> So allocation should do:
>
>        alloc_pages()
>        add_to_page_cache()
>        add_to_lru()
>
> and deallocation should do
>
>        remove_from_lru()
>        remove_from_page_cache()
>        put_page()
>
> If this protocol is followed correctly, there is no need to do a
> put_page() during the allocation/setup phase!
>
> I suspect that the problem in nommu really lies in the
> deallocation/teardown phase.
>
>

Yes,
And in my understanding in nommu deallocation phase:

1. iput() call default generate_drop_inode().
2. and then in evict() call truncate_inode_pages() which just remove
pages from_lru and pagecache.

There is no pace call put_page() so pages can't be freed at last.

Maybe we need to implement evict_inode() or drop_inode() in ramfs.
I will try it soon but I am not familiar with fs, any ideas is welcome.


Thanks
-- 
Regards,
--Bob

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Bob Liu]

Hi, Andrew

cc'd some folks working on nommu.

On Tue, Mar 29, 2011 at 8:02 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Mon, 28 Mar 2011 13:32:35 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> On no-mmu arch, there is a memleak duirng shmem test.
>> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
>> refcount to 2 which makes iput() can't free that pages.
>>
>> The simple test file is like this:
>> int main(void)
>> {
>>       int i;
>>       key_t k = ftok("/etc", 42);
>>
>>       for ( i=0; i<100; ++i) {
>>               int id = shmget(k, 10000, 0644|IPC_CREAT);
>>               if (id == -1) {
>>                       printf("shmget error\n");
>>               }
>>               if(shmctl(id, IPC_RMID, NULL ) == -1) {
>>                       printf("shm  rm error\n");
>>                       return -1;
>>               }
>>       }
>>       printf("run ok...\n");
>>       return 0;
>> }
>>
>> ...
>>
>> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
>> index 9eead2c..fbb0b47 100644
>> --- a/fs/ramfs/file-nommu.c
>> +++ b/fs/ramfs/file-nommu.c
>> @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
>>               SetPageDirty(page);
>>
>>               unlock_page(page);
>> +             put_page(page);
>>       }
>>
>>       return 0;
>
> Something is still wrong here.
>
> A live, in-use page should have a refcount of three.  One for the
> existence of the page, one for its presence on the page LRU and one for
> its existence in the pagecache radix tree.
>
> So allocation should do:
>
>        alloc_pages()
>        add_to_page_cache()
>        add_to_lru()
>
> and deallocation should do
>
>        remove_from_lru()
>        remove_from_page_cache()
>        put_page()
>
> If this protocol is followed correctly, there is no need to do a
> put_page() during the allocation/setup phase!
>
> I suspect that the problem in nommu really lies in the
> deallocation/teardown phase.
>

What about below patch ?

BTW: It seems that in MMU cases shmem pages are freed during memory reclaim,
since I didn't find the direct free place.
I am not sure maybe I got something wrong ?

Signed-off-by: Bob Liu <lliubbo@gmail.com>
---
 fs/ramfs/file-nommu.c |    1 +
 fs/ramfs/inode.c      |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index fbb0b47..11f48eb 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -114,6 +114,7 @@ int ramfs_nommu_expand_for_mapping(struct inode
*inode, size_t newsize)
 		unlock_page(page);
 		put_page(page);
 	}
+	inode->i_private = pages;

 	return 0;

diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index eacb166..e446d9f 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -151,9 +151,31 @@ static const struct inode_operations
ramfs_dir_inode_operations = {
 	.rename		= simple_rename,
 };

+#ifndef CONFIG_MMU
+static void ramfs_evict_inode(struct inode *inode)
+{
+	int i;
+	struct page *free_pages = (struct page *)inode->i_private;
+
+	/*
+	 * for nommu arch, need an extra put_page so that pages gotten
+	 * by ramfs_nommu_expand_for_mapping() can be freed
+	 */
+	for (i = 0; i < inode->i_data.nrpages; i++)
+		put_page(free_pages + i);
+
+	if (inode->i_data.nrpages)
+		truncate_inode_pages(&inode->i_data, 0);
+	end_writeback(inode);
+}
+#endif
+
 static const struct super_operations ramfs_ops = {
 	.statfs		= simple_statfs,
 	.drop_inode	= generic_delete_inode,
+#ifndef CONFIG_MMU
+	.evict_inode    = ramfs_evict_inode,
+#endif
 	.show_options	= generic_show_options,
 };

-- 
1.6.3.3

-- 
Regards,
--Bob

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[David Howells]

Bob Liu <lliubbo@gmail.com> wrote:

> On no-mmu arch, there is a memleak duirng shmem test.
> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
> refcount to 2 which makes iput() can't free that pages.

Sorry I haven't got around to looking at this yet; it's going to have to wait
till I get back from the US in just over a week.

David

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Hugh Dickins]

On Mon, 28 Mar 2011, Bob Liu wrote:
> On no-mmu arch, there is a memleak duirng shmem test.
> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
> refcount to 2 which makes iput() can't free that pages.
> 
> The simple test file is like this:
> int main(void)
> {
> 	int i;
> 	key_t k = ftok("/etc", 42);
> 
> 	for ( i=0; i<100; ++i) {
> 		int id = shmget(k, 10000, 0644|IPC_CREAT);
> 		if (id == -1) {
> 			printf("shmget error\n");
> 		}
> 		if(shmctl(id, IPC_RMID, NULL ) == -1) {
> 			printf("shm  rm error\n");
> 			return -1;
> 		}
> 	}
> 	printf("run ok...\n");
> 	return 0;
> }
> 
> And the result:
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16644        43676            0            0
> -/+ buffers:              16644        43676
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        17912        42408            0            0
> -/+ buffers:              17912        42408
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        19096        41224            0            0
> -/+ buffers:              19096        41224
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        20296        40024            0            0
> -/+ buffers:              20296        40024
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        21496        38824            0            0
> -/+ buffers:              21496        38824
> root:/> shmem 
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        22692        37628            0            0
> -/+ buffers:              22692        37628
> root:/> 
> 
> After this patch the test result is:(no memleak anymore)
> root:/> 
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16580        43740            0            0
> -/+ buffers:              16580        43740
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16668        43652            0            0
> -/+ buffers:              16668        43652
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16668        43652            0            0
> -/+ buffers:              16668        43652
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16668        43652            0            0
> -/+ buffers:              16668        43652
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16668        43652            0            0
> -/+ buffers:              16668        43652
> root:/> shmem
> run ok...
> root:/> free
>              total         used         free       shared      buffers
> Mem:         60320        16668        43652            0            0
> -/+ buffers:              16668        43652
> root:/> 
> 
> Signed-off-by: Bob Liu <lliubbo@gmail.com>

Acked-by: Hugh Dickins <hughd@google.com>
Cc: stable@kernel.org

Sorry for being so slow to get back to this, Bob.

And I'm sorry that in my original patch for it, I just couldn't
resist a little tidying up while I was there: which led Paul to
observe correctly that the function would be better off using
alloc_pages_exact(); which made me pause before responding,
seeing a proposal to rename that to get_free_pages_exact().

Aaaaaah, let's go with your patch below, which fixes the bug in
the simplest fashion; and do any tidying up at leisure later on.

I see Andrew is questioning the correctness of this patch:
let me answer his mail separately.

Hugh

> ---
>  fs/ramfs/file-nommu.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
> index 9eead2c..fbb0b47 100644
> --- a/fs/ramfs/file-nommu.c
> +++ b/fs/ramfs/file-nommu.c
> @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
>  		SetPageDirty(page);
>  
>  		unlock_page(page);
> +		put_page(page);
>  	}
>  
>  	return 0;
> -- 
> 1.6.3.3

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Hugh Dickins]

On Mon, 28 Mar 2011, Andrew Morton wrote:
> On Mon, 28 Mar 2011 13:32:35 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
> 
> > On no-mmu arch, there is a memleak duirng shmem test.
> > The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
> > refcount to 2 which makes iput() can't free that pages.
> > ...
> > 
> > diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
> > index 9eead2c..fbb0b47 100644
> > --- a/fs/ramfs/file-nommu.c
> > +++ b/fs/ramfs/file-nommu.c
> > @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
> >  		SetPageDirty(page);
> >  
> >  		unlock_page(page);
> > +		put_page(page);
> >  	}
> >  
> >  	return 0;
> 
> Something is still wrong here.

I don't think so.

> 
> A live, in-use page should have a refcount of three.  One for the
> existence of the page, one for its presence on the page LRU and one for
> its existence in the pagecache radix tree.

No, we don't count 1 for the LRU: it always seems a little odd that
we don't, but that's how it is.  I did dive into the debugger to
check that is really still the case.  And it doesn't really matter
here, since of course we don't count -1 when taking off LRU either.

The pages here are not "in-use" as such: we're just priming the
page cache with them, so they will be found shortly afterwards
when they do come into use, when inserted into the address space.

What if memory pressure comes in and frees them before then?
Er, er, that gave me a nasty turn.  But there's a comment
just above the SetPageDirty visible in Bob's patch, saying
/* prevent the page from being discarded on memory pressure */

> 
> So allocation should do:
> 
> 	alloc_pages()

Yes, it did that (along with a split_page we can ignore here).

> 	add_to_page_cache()
> 	add_to_lru()

And those it did in the combined function add_to_page_cache_lru().

> 
> and deallocation should do
> 
> 	remove_from_lru()
> 	remove_from_page_cache()

Nowadays delete_from_page_cache(), which decrements the reference
acquired in add_to_page_cache().

> 	put_page()
> 
> If this protocol is followed correctly, there is no need to do a
> put_page() during the allocation/setup phase!

There is a get_page() when each page is mapped into the address
space, which then matches the final put_page() you show above.

> 
> I suspect that the problem in nommu really lies in the
> deallocation/teardown phase.

Hugh

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] ramfs: fix memleak on no-mmu arch

[Hugh Dickins]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4382 bytes --]

On Fri, 1 Apr 2011, Bob Liu wrote:
> Hi, Andrew
> 
> cc'd some folks working on nommu.
> 
> On Tue, Mar 29, 2011 at 8:02 AM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Mon, 28 Mar 2011 13:32:35 +0800
> > Bob Liu <lliubbo@gmail.com> wrote:
> >
> >> On no-mmu arch, there is a memleak duirng shmem test.
> >> The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
> >> refcount to 2 which makes iput() can't free that pages.
> >>
> >> The simple test file is like this:
> >> int main(void)
> >> {
> >>       int i;
> >>       key_t k = ftok("/etc", 42);
> >>
> >>       for ( i=0; i<100; ++i) {
> >>               int id = shmget(k, 10000, 0644|IPC_CREAT);
> >>               if (id == -1) {
> >>                       printf("shmget error\n");
> >>               }
> >>               if(shmctl(id, IPC_RMID, NULL ) == -1) {
> >>                       printf("shm  rm error\n");
> >>                       return -1;
> >>               }
> >>       }
> >>       printf("run ok...\n");
> >>       return 0;
> >> }
> >>
> >> ...
> >>
> >> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
> >> index 9eead2c..fbb0b47 100644
> >> --- a/fs/ramfs/file-nommu.c
> >> +++ b/fs/ramfs/file-nommu.c
> >> @@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
> >>               SetPageDirty(page);
> >>
> >>               unlock_page(page);
> >> +             put_page(page);
> >>       }
> >>
> >>       return 0;
> >
> > Something is still wrong here.
> >
> > A live, in-use page should have a refcount of three.  One for the
> > existence of the page, one for its presence on the page LRU and one for
> > its existence in the pagecache radix tree.
> >
> > So allocation should do:
> >
> >        alloc_pages()
> >        add_to_page_cache()
> >        add_to_lru()
> >
> > and deallocation should do
> >
> >        remove_from_lru()
> >        remove_from_page_cache()
> >        put_page()
> >
> > If this protocol is followed correctly, there is no need to do a
> > put_page() during the allocation/setup phase!
> >
> > I suspect that the problem in nommu really lies in the
> > deallocation/teardown phase.
> >
> 
> What about below patch ?
> 
> BTW: It seems that in MMU cases shmem pages are freed during memory reclaim,
> since I didn't find the direct free place.
> I am not sure maybe I got something wrong ?
> 
> Signed-off-by: Bob Liu <lliubbo@gmail.com>

No, I really think this ramfs_evict_inode() thing is a quite
unnecessary complication: your little put_page() patch much nicer.

Hugh

> ---
>  fs/ramfs/file-nommu.c |    1 +
>  fs/ramfs/inode.c      |   22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
> index fbb0b47..11f48eb 100644
> --- a/fs/ramfs/file-nommu.c
> +++ b/fs/ramfs/file-nommu.c
> @@ -114,6 +114,7 @@ int ramfs_nommu_expand_for_mapping(struct inode
> *inode, size_t newsize)
>  		unlock_page(page);
>  		put_page(page);
>  	}
> +	inode->i_private = pages;
> 
>  	return 0;
> 
> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index eacb166..e446d9f 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -151,9 +151,31 @@ static const struct inode_operations
> ramfs_dir_inode_operations = {
>  	.rename		= simple_rename,
>  };
> 
> +#ifndef CONFIG_MMU
> +static void ramfs_evict_inode(struct inode *inode)
> +{
> +	int i;
> +	struct page *free_pages = (struct page *)inode->i_private;
> +
> +	/*
> +	 * for nommu arch, need an extra put_page so that pages gotten
> +	 * by ramfs_nommu_expand_for_mapping() can be freed
> +	 */
> +	for (i = 0; i < inode->i_data.nrpages; i++)
> +		put_page(free_pages + i);
> +
> +	if (inode->i_data.nrpages)
> +		truncate_inode_pages(&inode->i_data, 0);
> +	end_writeback(inode);
> +}
> +#endif
> +
>  static const struct super_operations ramfs_ops = {
>  	.statfs		= simple_statfs,
>  	.drop_inode	= generic_delete_inode,
> +#ifndef CONFIG_MMU
> +	.evict_inode    = ramfs_evict_inode,
> +#endif
>  	.show_options	= generic_show_options,
>  };
> 
> -- 
> 1.6.3.3

[PATCH] ramfs: fix memleak on no-mmu arch

[David Howells]

From: Bob Liu <lliubbo@gmail.com>

On no-mmu arch, there is a memleak duirng shmem test.
The cause of this memleak is ramfs_nommu_expand_for_mapping() added page
refcount to 2 which makes iput() can't free that pages.

The simple test file is like this:
int main(void)
{
	int i;
	key_t k = ftok("/etc", 42);

	for ( i=0; i<100; ++i) {
		int id = shmget(k, 10000, 0644|IPC_CREAT);
		if (id == -1) {
			printf("shmget error\n");
		}
		if(shmctl(id, IPC_RMID, NULL ) == -1) {
			printf("shm  rm error\n");
			return -1;
		}
	}
	printf("run ok...\n");
	return 0;
}

And the result:
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16644        43676            0            0
-/+ buffers:              16644        43676
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        17912        42408            0            0
-/+ buffers:              17912        42408
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        19096        41224            0            0
-/+ buffers:              19096        41224
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        20296        40024            0            0
-/+ buffers:              20296        40024
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        21496        38824            0            0
-/+ buffers:              21496        38824
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        22692        37628            0            0
-/+ buffers:              22692        37628
root:/>

After this patch the test result is:(no memleak anymore)
root:/>
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16580        43740            0            0
-/+ buffers:              16580        43740
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/> shmem
run ok...
root:/> free
             total         used         free       shared      buffers
Mem:         60320        16668        43652            0            0
-/+ buffers:              16668        43652
root:/>

Signed-off-by: Bob Liu <lliubbo@gmail.com>
Acked-by: Hugh Dickins <hughd@google.com>
Cc: stable@kernel.org
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/ramfs/file-nommu.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 9eead2c..fbb0b47 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -112,6 +112,7 @@ int ramfs_nommu_expand_for_mapping(struct inode *inode, size_t newsize)
 		SetPageDirty(page);
 
 		unlock_page(page);
+		put_page(page);
 	}
 
 	return 0;

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>