Bug 763196 (GLUSTER-1464) - fd leak after rename
Summary: fd leak after rename
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: GLUSTER-1464
Product: GlusterFS
Classification: Community
Component: nfs
Version: nfs-alpha
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Shehjar Tikoo
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-08-27 23:45 UTC by Krishna Srinivas
Modified: 2015-12-01 16:45 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
Regression: RTP
Mount Type: nfs
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Krishna Srinivas 2010-08-27 20:52:50 UTC
Shehjar this bug affects vmstor ... so let me know asap if this fix is harmful.

Comment 1 Krishna Srinivas 2010-08-27 23:45:23 UTC
echo junk > test2
touch test1
mv test1 test2

this will cause fd leak:

[root@brick1 testing]# ls -l /proc/30418/fd
lrwx------ 1 root root 64 Aug 26 18:33 20 -> /export/export/testing/test2 (deleted)


This fixed it: (but not sure if this is the "right" way of doing things):

int32_t
nfs3svc_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                    int32_t op_ret, int32_t op_errno, struct iatt *buf,
                    struct iatt *preoldparent, struct iatt *postoldparent,
                    struct iatt *prenewparent, struct iatt *postnewparent)
{
        int                     ret = -EFAULT;
        nfsstat3                stat = NFS3ERR_SERVERFAULT;
        nfs3_call_state_t       *cs = NULL;
        fd_t                    *openfd = NULL;
        struct nfs3_state       *nfs3 = NULL;

        cs = frame->local;
        if (op_ret == -1) {
                stat = nfs3_errno_to_nfsstat3 (op_errno);
                goto nfs3err;
        }

        stat = NFS3_OK;
/* Kris: bug fix for rename fd leak */
        /* Close any cached fds so that when any currently active write                                                                                      
         * finishes, the file is finally removed.                                                                                                            
         */
         openfd = fd_lookup (cs->resolvedloc.inode, 0);
         nfs3 = rpcsvc_request_program_private (cs->req);
         if (openfd) {
                fd_unref (openfd);
                nfs3_fdcache_remove (nfs3, openfd);
         }
         /* do inode_unref(cs->resolvedloc.inode) if needed */
/* Kris: bug fix for rename fd leak */
nfs3err:
        nfs3_log_common_res (rpcsvc_request_xid (cs->req), "RENAME", stat,-ret);

Comment 2 Shehjar Tikoo 2010-08-28 05:09:24 UTC
Your change seems about right if it fixes it, go ahead and use it, i'll make the same change here and take it through some tests to be sure

Comment 3 Shehjar Tikoo 2010-08-28 05:48:43 UTC
the inode_unref  on dst is needed, but (a). i am not sure if nfs is the right place for it, need to look into inode.c code as well, (b) need to think about the exact if {} condition i need to use to unref in rename_cbk because the inode is resolvedloc will also be unref in nfs3_call_state_wipe causing an assertion failure there if the inode has been unrefd previously.

Comment 4 Vijay Bellur 2010-08-31 11:44:53 UTC
PATCH: http://patches.gluster.com/patch/4425 in master (nfs3: Close dst cached fd & unref inode on rename)

Comment 5 Shehjar Tikoo 2010-09-01 02:47:30 UTC
Regression Test:

1. Export a simple posix through gnfs.

2. Mount as:

mount <server>:/posix /mnt

3. Run the commands:

$ echo "asdasdasdads" > /mnt/test2
$ touch /mnt/test1
$ mv /mnt/test1 /mnt/test2

4. Then inspect the nfs server's entry in /proc using the command:

$ ls -l /proc/<GNFS-PID>/fd
lrwx------ 1 root root 64 Aug 26 18:33 20 -> /posixexportdir/test2

If the output includes an entry for test2, there is a regression.

Comment 6 Vijay Bellur 2010-09-02 09:17:22 UTC
PATCH: http://patches.gluster.com/patch/4478 in master (nfs3: Do not unref dst inode on rename cbk)


Note You need to log in before you can comment on or make changes to this bug.