summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnand Avati <avati@gluster.com>2010-11-15 05:34:00 +0000
committerAnand V. Avati <avati@dev.gluster.com>2010-11-15 04:03:50 -0800
commit78654d270cde028c5d7f9da29b2790b90c19e11f (patch)
tree9c19e7ad52a8e3f20e72315b14d6eb00e08dad33
parent7d48999c13b885d034528cfca61a8b63ade8a365 (diff)
nfs: opendir/closedir for every readdir
Revert "nfs3: Unref & unbind dir fd with inode lock on EOF" This reverts commit 4e6fb304ce41acbaf7c9ba67c06bf443e65082e8. The above commit (which unbinds fds at EOF) does not fix the original bug (1619) because a readdir from a second app could have already started before the readdir_cbk of the first app's readdir reaches NFS code. Hence the race still exists. Performing extra unrefs when EOF is received is not a reliable way of detecting that a client has performed a closedir (and to close the fd ourselves). Neither is interpreting a 0 cookies a new opendir. Clients can always use telldir/seekdir and hit EOFs twice. Due to the way NFS3 protocol is designed, it is just not possible for the server to reliably detect opendirs/closedirs performed by the client and map the corresponding readdirs to the same dir fd on the server side. The only reliable way of fixing this is to perform opendir/closedir at the cost of performance. Any optimization towards keeping dir fds open attempting to map them with application's opendir/closedir will either result in fd leaks or extra fd unrefs. Signed-off-by: Anand V. Avati <avati@blackhole.gluster.com> Signed-off-by: Anand V. Avati <avati@dev.gluster.com> BUG: 2061 (NFS server crashes in readdir_fstat_cbk due to extra fd unref) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2061
-rw-r--r--libglusterfs/src/fd.c23
-rw-r--r--libglusterfs/src/fd.h2
-rw-r--r--xlators/nfs/server/src/nfs3-helpers.c2
-rw-r--r--xlators/nfs/server/src/nfs3.c17
4 files changed, 5 insertions, 39 deletions
diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c
index e4e75d804..def152557 100644
--- a/libglusterfs/src/fd.c
+++ b/libglusterfs/src/fd.c
@@ -500,29 +500,6 @@ fd_bind (fd_t *fd)
}
-void
-fd_unref_unbind (fd_t *fd)
-{
- GF_ASSERT (fd->refcount);
-
- LOCK (&fd->inode->lock);
- {
- --fd->refcount;
- /* Better know what you're doing with this function
- * because it does not do what fd_destroy does when
- * refcount goes to 0.
- * Make sure you only call this when you know there are
- * pending refs on the fd.
- */
- GF_ASSERT (fd->refcount);
- list_del_init (&fd->inode_list);
- }
- UNLOCK (&fd->inode->lock);
-
- return;
-}
-
-
fd_t *
fd_create (inode_t *inode, pid_t pid)
{
diff --git a/libglusterfs/src/fd.h b/libglusterfs/src/fd.h
index d3efeb913..d19b0724f 100644
--- a/libglusterfs/src/fd.h
+++ b/libglusterfs/src/fd.h
@@ -169,6 +169,4 @@ _fd_ref (fd_t *fd);
void
fd_ctx_dump (fd_t *fd, char *prefix);
-extern void
-fd_unref_unbind (fd_t *fd);
#endif /* _FD_H */
diff --git a/xlators/nfs/server/src/nfs3-helpers.c b/xlators/nfs/server/src/nfs3-helpers.c
index cff029d6e..8e4ca3720 100644
--- a/xlators/nfs/server/src/nfs3-helpers.c
+++ b/xlators/nfs/server/src/nfs3-helpers.c
@@ -1775,7 +1775,7 @@ nfs3_dir_open_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
goto err;
}
- cs->fd = fd_ref (fd);
+ cs->fd = fd; /* Gets unrefd when the call state is wiped. */
nfs3_set_inode_opened (cs->nfsx, cs->resolvedloc.inode);
gf_log (GF_NFS3, GF_LOG_TRACE, "FD_REF: %d", fd->refcount);
nfs3_call_resume (cs);
diff --git a/xlators/nfs/server/src/nfs3.c b/xlators/nfs/server/src/nfs3.c
index 7fed84d50..ce5927520 100644
--- a/xlators/nfs/server/src/nfs3.c
+++ b/xlators/nfs/server/src/nfs3.c
@@ -3889,17 +3889,6 @@ nfs3svc_readdir_fstat_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
stat = NFS3_OK;
nfs3err:
-
- /* On end-of-directory, unref the fd to have it removed from the cache
- * and also unbind it from the fd so that any subsequent request on the
- * on the directory do not get this fd when fd_lookup is called in
- * dir open resume path.
- */
- if (is_eof) {
- gf_log (GF_NFS3, GF_LOG_TRACE, "EOF REF: %d", cs->fd->refcount);
- fd_unref_unbind (cs->fd);
- }
-
if (cs->maxcount == 0) {
nfs3_log_readdir_res (nfs_rpcsvc_request_xid (cs->req), stat,
op_errno, (uintptr_t)cs->fd,
@@ -3917,7 +3906,10 @@ nfs3err:
cs->maxcount, is_eof);
}
- gf_log (GF_NFS3, GF_LOG_TRACE, "CS WIPE REF: %d", cs->fd->refcount);
+ if (is_eof) {
+ /* do nothing */
+ }
+
nfs3_call_state_wipe (cs);
return 0;
}
@@ -3968,7 +3960,6 @@ err:
* so that next time the dir is read, we'll get any changed directory
* entries.
*/
- fd_unref_unbind (cs->fd);
nfs3_call_state_wipe (cs);
ret:
return 0;