Messages in this thread | | | Subject | Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir | From | Trond Myklebust <> | Date | Wed, 01 Dec 2010 18:56:16 -0500 |
| |
On Wed, 2010-12-01 at 15:46 -0800, Andrew Morton wrote: > On Wed, 01 Dec 2010 18:21:16 -0500 > Trond Myklebust <Trond.Myklebust@netapp.com> wrote: > > > --- a/mm/truncate.c > > +++ b/mm/truncate.c > > @@ -108,6 +108,10 @@ truncate_complete_page(struct address_space *mapping, struct page *page) > > clear_page_mlock(page); > > remove_from_page_cache(page); > > ClearPageMappedToDisk(page); > > + > > + if (mapping->a_ops->freepage) > > + mapping->a_ops->freepage(page); > > + > > page_cache_release(page); /* pagecache ref */ > > return 0; > > } > > So here we're assuming that `mapping' was pinned by other means. > > Fair enough, although subtle. Even drop_pagecache_sb() got it right ;) > > > @@ -390,6 +394,10 @@ invalidate_complete_page2(struct address_space *mapping, struct page *page) > > __remove_from_page_cache(page); > > spin_unlock_irq(&mapping->tree_lock); > > mem_cgroup_uncharge_cache_page(page); > > + > > + if (mapping->a_ops->freepage) > > + mapping->a_ops->freepage(page); > > + > > page_cache_release(page); /* pagecache ref */ > > return 1; > > failed: > > And here.
Yes. Both these functions are static, and their callers are assuming that something is already pinning the underlying inode, so the above should be quite safe.
-- Trond Myklebust Linux NFS client maintainer
NetApp Trond.Myklebust@netapp.com www.netapp.com
| |