* [PATCH] ramfs: fix memleak on no-mmu arch
@ 2011-03-28 5:32 Bob Liu
2011-03-29 0:02 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Bob Liu @ 2011-03-28 5:32 UTC (permalink / raw)
To: akpm
Cc: linux-mm, hughd, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm, 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>
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-28 5:32 [PATCH] ramfs: fix memleak on no-mmu arch Bob Liu
@ 2011-03-29 0:02 ` Andrew Morton
2011-03-29 11:06 ` Bob Liu
` (2 more replies)
2011-04-01 15:19 ` David Howells
2011-04-02 2:52 ` Hugh Dickins
2 siblings, 3 replies; 9+ messages in thread
From: Andrew Morton @ 2011-03-29 0:02 UTC (permalink / raw)
To: Bob Liu
Cc: linux-mm, hughd, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm
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>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-29 0:02 ` Andrew Morton
@ 2011-03-29 11:06 ` Bob Liu
2011-04-01 8:25 ` Bob Liu
2011-04-02 3:35 ` Hugh Dickins
2 siblings, 0 replies; 9+ messages in thread
From: Bob Liu @ 2011-03-29 11:06 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, hughd, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm
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>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-29 0:02 ` Andrew Morton
2011-03-29 11:06 ` Bob Liu
@ 2011-04-01 8:25 ` Bob Liu
2011-04-02 3:39 ` Hugh Dickins
2011-04-02 3:35 ` Hugh Dickins
2 siblings, 1 reply; 9+ messages in thread
From: Bob Liu @ 2011-04-01 8:25 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, hughd, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm, Mike Frysinger, horms, gerg, ithamar.adema
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>
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-04-01 8:25 ` Bob Liu
@ 2011-04-02 3:39 ` Hugh Dickins
0 siblings, 0 replies; 9+ messages in thread
From: Hugh Dickins @ 2011-04-02 3:39 UTC (permalink / raw)
To: Bob Liu
Cc: Andrew Morton, linux-mm, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm, Mike Frysinger, horms, gerg, ithamar.adema
[-- 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-29 0:02 ` Andrew Morton
2011-03-29 11:06 ` Bob Liu
2011-04-01 8:25 ` Bob Liu
@ 2011-04-02 3:35 ` Hugh Dickins
2 siblings, 0 replies; 9+ messages in thread
From: Hugh Dickins @ 2011-04-02 3:35 UTC (permalink / raw)
To: Andrew Morton
Cc: Bob Liu, linux-mm, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-28 5:32 [PATCH] ramfs: fix memleak on no-mmu arch Bob Liu
2011-03-29 0:02 ` Andrew Morton
@ 2011-04-01 15:19 ` David Howells
2011-04-02 2:52 ` Hugh Dickins
2 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2011-04-01 15:19 UTC (permalink / raw)
To: Bob Liu
Cc: dhowells, akpm, linux-mm, hughd, viro, hch, npiggin, tj, lethal,
magnus.damm
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ramfs: fix memleak on no-mmu arch
2011-03-28 5:32 [PATCH] ramfs: fix memleak on no-mmu arch Bob Liu
2011-03-29 0:02 ` Andrew Morton
2011-04-01 15:19 ` David Howells
@ 2011-04-02 2:52 ` Hugh Dickins
2 siblings, 0 replies; 9+ messages in thread
From: Hugh Dickins @ 2011-04-02 2:52 UTC (permalink / raw)
To: Bob Liu
Cc: akpm, linux-mm, hughd, viro, hch, npiggin, tj, dhowells, lethal,
magnus.damm
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ramfs: fix memleak on no-mmu arch
@ 2011-04-13 16:45 David Howells
0 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2011-04-13 16:45 UTC (permalink / raw)
To: torvalds, akpm
Cc: linux-mm, uclinux-dev, linux-kernel, Bob Liu, Hugh Dickins,
stable, 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>
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-04-13 16:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-28 5:32 [PATCH] ramfs: fix memleak on no-mmu arch Bob Liu
2011-03-29 0:02 ` Andrew Morton
2011-03-29 11:06 ` Bob Liu
2011-04-01 8:25 ` Bob Liu
2011-04-02 3:39 ` Hugh Dickins
2011-04-02 3:35 ` Hugh Dickins
2011-04-01 15:19 ` David Howells
2011-04-02 2:52 ` Hugh Dickins
-- strict thread matches above, loose matches on Subject: below --
2011-04-13 16:45 David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).