From: Benjamin Coddington <bcodding@redhat.com>
To: Trond Myklebust <trond.myklebust@primarydata.com>,
Anna Schumaker <anna.schumaker@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH] NFSv4: Fix a dentry leak on alias use
Date: Wed, 17 Feb 2016 09:44:48 -0500 [thread overview]
Message-ID: <a65c14e6e3e051fd2367732ba4b237d3605920a6.1455718518.git.bcodding@redhat.com> (raw)
We've had some users hitting what used to be a BUG() in
shink_dcache_for_umount() but is now a printk:
BUG: Dentry ffff880066676000{i=20007,n=foobar} still in use (1) [unmount of nfs4 0:39]
VFS: Busy inodes after unmount of 0:39. Self-destruct in 5 seconds. Have a nice day...
The users don't like that very much.
I've spotted a dentry leak in the rare case where d_add_unique() finds an
alias on open and we swap the open context's dentry. I'm pretty sure we
shouldn't be doing another dget() there. I can reliably reproduce it with
this bit of bash:
mkdir -p /exports/dir{1,2}
exportfs -o rw localhost:/exports
mount -t nfs -ov4.1 localhost:/ /mnt/localhost
(sleep 2) > /mnt/localhost/dir1/foobar &
waitpid="$!"
sleep 1
mv /exports/dir{1,2}/foobar
echo A > /mnt/localhost/dir2/foobar
stat /mnt/localhost/dir1/foobar 2> /dev/null
mv /exports/dir{2,1}/foobar
echo A > /mnt/localhost/dir1/foobar
wait $waitpid
umount /mnt/localhost
Ben
8<---------------------------------------------------------------------
In the case where d_add_unique() finds an appropriate alias to use it will
have already incremented the reference count. An additional dget() to swap
the open context's dentry is unnecessary and will leak a reference.
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
fs/nfs/nfs4proc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 4bfc33a..f4401ed 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2468,7 +2468,7 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
dentry = opendata->dentry;
} else if (dentry != ctx->dentry) {
dput(ctx->dentry);
- ctx->dentry = dget(dentry);
+ ctx->dentry = dentry;
}
nfs_set_verifier(dentry,
nfs_save_change_attribute(d_inode(opendata->dir)));
--
1.7.1
next reply other threads:[~2016-02-17 14:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-17 14:44 Benjamin Coddington [this message]
2016-02-17 15:41 ` [PATCH v2] NFSv4: Fix a dentry leak on alias use Benjamin Coddington
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=a65c14e6e3e051fd2367732ba4b237d3605920a6.1455718518.git.bcodding@redhat.com \
--to=bcodding@redhat.com \
--cc=anna.schumaker@netapp.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@primarydata.com \
/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.