All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Nick Bowler <nbowler@elliptictech.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-nfs@vger.kernel.org
Subject: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir
Date: Wed,  1 Dec 2010 10:36:44 -0500	[thread overview]
Message-ID: <1291217804-11257-4-git-send-email-Trond.Myklebust@netapp.com> (raw)
In-Reply-To: <1291217804-11257-3-git-send-email-Trond.Myklebust@netapp.com>
In-Reply-To: <20101201150428.GA2879@elliptictech.com>

We need to ensure that the entries in the nfs_cache_array get cleared
when the page is removed from the page cache. To do so, we use the
releasepage address_space operation (which also requires us to set
the Pg_private flag).

Change nfs_readdir_clear_array to use kmap_atomic(), so that the
function can be safely called from all direct reclaim contexts.

Finally, modify the cache_page_release helper to call
nfs_readdir_clear_array directly, when dealing with an anonymous
page from 'uncached_readdir'.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/dir.c           |   22 +++++++++++++++++-----
 fs/nfs/inode.c         |    1 +
 include/linux/nfs_fs.h |    1 +
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 3ec3f1c..4c6319e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -58,6 +58,7 @@ static int nfs_rename(struct inode *, struct dentry *,
 static int nfs_fsync_dir(struct file *, int);
 static loff_t nfs_llseek_dir(struct file *, loff_t, int);
 static int nfs_readdir_clear_array(struct page*, gfp_t);
+static void nfs_readdir_invalidatepage(struct page*, unsigned long);
 
 const struct file_operations nfs_dir_operations = {
 	.llseek		= nfs_llseek_dir,
@@ -85,6 +86,7 @@ const struct inode_operations nfs_dir_inode_operations = {
 
 const struct address_space_operations nfs_dir_addr_space_ops = {
 	.releasepage = nfs_readdir_clear_array,
+	.invalidatepage = nfs_readdir_invalidatepage,
 };
 
 #ifdef CONFIG_NFS_V3
@@ -216,15 +218,22 @@ void nfs_readdir_release_array(struct page *page)
 static
 int nfs_readdir_clear_array(struct page *page, gfp_t mask)
 {
-	struct nfs_cache_array *array = nfs_readdir_get_array(page);
+	struct nfs_cache_array *array;
 	int i;
 
-	if (IS_ERR(array))
-		return PTR_ERR(array);
+	array = kmap_atomic(page, KM_USER0);
 	for (i = 0; i < array->size; i++)
 		kfree(array->array[i].string.name);
-	nfs_readdir_release_array(page);
-	return 0;
+	kunmap_atomic(array, KM_USER0);
+	ClearPageUptodate(page);
+	ClearPagePrivate(page);
+	return 1;
+}
+
+static
+void nfs_readdir_invalidatepage(struct page *page, unsigned long offset)
+{
+	nfs_readdir_clear_array(page, 0);
 }
 
 /*
@@ -624,6 +633,7 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page* page)
 	if (ret < 0)
 		goto error;
 	SetPageUptodate(page);
+	SetPagePrivate(page);
 
 	if (invalidate_inode_pages2_range(inode->i_mapping, page->index + 1, -1) < 0) {
 		/* Should never happen */
@@ -639,6 +649,8 @@ int nfs_readdir_filler(nfs_readdir_descriptor_t *desc, struct page* page)
 static
 void cache_page_release(nfs_readdir_descriptor_t *desc)
 {
+	if (!desc->page->mapping)
+		nfs_readdir_clear_array(desc->page, GFP_KERNEL);
 	unlock_page(desc->page);
 	page_cache_release(desc->page);
 	desc->page = NULL;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 314f571..0018e07 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -289,6 +289,7 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr)
 		} else if (S_ISDIR(inode->i_mode)) {
 			inode->i_op = NFS_SB(sb)->nfs_client->rpc_ops->dir_inode_ops;
 			inode->i_fop = &nfs_dir_operations;
+			inode->i_data.a_ops = &nfs_dir_addr_space_ops;
 			if (nfs_server_capable(inode, NFS_CAP_READDIRPLUS))
 				set_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(inode)->flags);
 			/* Deal with crossing mountpoints */
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index c66fdb7..b5d3ab0 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -401,6 +401,7 @@ extern const struct inode_operations nfs3_file_inode_operations;
 #endif /* CONFIG_NFS_V3 */
 extern const struct file_operations nfs_file_operations;
 extern const struct address_space_operations nfs_file_aops;
+extern const struct address_space_operations nfs_dir_addr_space_ops;
 
 static inline struct nfs_open_context *nfs_file_open_context(struct file *filp)
 {
-- 
1.7.3.2


  parent reply	other threads:[~2010-12-01 15:36 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-30 17:42 [PATCH] NFS: Fix a readdirplus bug Trond Myklebust
2010-11-30 22:10 ` Linus Torvalds
2010-11-30 22:13   ` Trond Myklebust
2010-12-01  3:47   ` [PATCH 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-01  3:47   ` [PATCH 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-01 15:04     ` Nick Bowler
2010-12-01 15:36       ` [PATCH v2 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-01 15:36       ` [PATCH v2 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-01 15:36       ` [PATCH v2 2/3] NFS: lock the readdir page while it is in use Trond Myklebust
2010-12-01 15:36       ` Trond Myklebust [this message]
2010-12-01 16:17         ` [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir Linus Torvalds
2010-12-01 16:35           ` Rik van Riel
2010-12-01 16:45             ` Benny Halevy
2010-12-01 16:47             ` Linus Torvalds
2010-12-01 17:02               ` Rik van Riel
2010-12-01 17:58           ` Trond Myklebust
2010-12-01 18:29           ` Miklos Szeredi
2010-12-01 18:54           ` Trond Myklebust
2010-12-01 19:23             ` Hugh Dickins
2010-12-01 19:52               ` Linus Torvalds
2010-12-01 20:05               ` Trond Myklebust
2010-12-01 20:39                 ` Andrew Morton
2010-12-01 21:29                   ` Neil Brown
2010-12-01 22:43                     ` Andrew Morton
2010-12-01 23:01                       ` Neil Brown
2010-12-01 19:47             ` Linus Torvalds
2010-12-01 20:10               ` Trond Myklebust
2010-12-01 20:10                 ` Trond Myklebust
2010-12-01 20:18                 ` Linus Torvalds
2010-12-01 20:31                 ` Hugh Dickins
2010-12-01 20:33                 ` Andrew Morton
2010-12-01 21:02                   ` Hugh Dickins
2010-12-01 21:15                     ` Hugh Dickins
2010-12-01 21:38                       ` Andrew Morton
2010-12-01 21:51                         ` Trond Myklebust
2010-12-01 22:13                           ` Andrew Morton
2010-12-01 22:24                             ` Linus Torvalds
2010-12-01 22:38                               ` Andrew Morton
2010-12-01 22:47                                 ` Trond Myklebust
2010-12-01 23:21                                   ` Trond Myklebust
2010-12-01 23:21                                     ` Trond Myklebust
2010-12-01 23:46                                     ` Andrew Morton
2010-12-01 23:56                                       ` Trond Myklebust
2010-12-01 23:31                                 ` Linus Torvalds
2010-12-01 23:36                                   ` Andrew Morton
2010-12-02  1:05                                     ` Linus Torvalds
2010-12-02  1:22                                       ` Andrew Morton
2010-12-02  1:42                                         ` Linus Torvalds
2010-12-02  2:05                                           ` Andrew Morton
2010-12-02  3:08                                           ` [PATCH v3 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-02  3:08                                           ` [PATCH v3 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-02  3:08                                           ` [PATCH v3 2/3] Call the filesystem back whenever a page is removed from the page cache Trond Myklebust
2010-12-02  3:34                                             ` Hugh Dickins
2010-12-02  3:53                                               ` Trond Myklebust
2010-12-02  3:58                                                 ` Linus Torvalds
2010-12-06 16:59                                                   ` [PATCH v4 0/3] Fix more NFS readdir regressions Trond Myklebust
2010-12-06 16:59                                                   ` [PATCH v4 1/3] NFS: Ensure we use the correct cookie in nfs_readdir_xdr_filler Trond Myklebust
2010-12-06 16:59                                                   ` [PATCH v4 2/3] Call the filesystem back whenever a page is removed from the page cache Trond Myklebust
2010-12-07  7:08                                                     ` Nick Piggin
2010-12-06 16:59                                                   ` [PATCH v4 3/3] NFS: Fix a memory leak in nfs_readdir Trond Myklebust
2010-12-02  3:08                                           ` [PATCH v3 " Trond Myklebust
2010-12-03  9:12                                           ` [PATCH v2 " Nick Piggin
2010-12-01 23:43                                   ` Trond Myklebust
2010-12-01 22:43                               ` Hugh Dickins
2010-12-01  3:47   ` [PATCH 2/3] NFS: lock the readdir page while it is in use Trond Myklebust
2010-12-01  4:10     ` Linus Torvalds
2010-12-01  4:29       ` Trond Myklebust
2010-12-01  5:06         ` Linus Torvalds
2010-12-01 14:49           ` Trond Myklebust
2010-12-01 13:14         ` Rik van Riel
2010-12-01 14:55           ` Trond Myklebust
2010-12-01  3:47   ` [PATCH 3/3] NFS: Fix a memory leak in nfs_readdir Trond Myklebust

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1291217804-11257-4-git-send-email-Trond.Myklebust@netapp.com \
    --to=trond.myklebust@netapp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=nbowler@elliptictech.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.