Bug 1344396 - fd leak in disperse
Summary: fd leak in disperse
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: disperse
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
Assignee: Xavi Hernandez
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1344421 1344422 1344425
TreeView+ depends on / blocked
 
Reported: 2016-06-09 14:46 UTC by Xavi Hernandez
Modified: 2016-11-23 07:19 UTC (History)
5 users (show)

Fixed In Version: glusterfs-3.9.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1344421 1344422 1344425 (view as bug list)
Environment:
Last Closed: 2016-11-23 07:19:02 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Xavi Hernandez 2016-06-09 14:46:20 UTC
Description of problem:

The disperse xlator uses __fd_unref() to release references to the fd. However this function doesn't make the cleanup of the fd resources if this was the last reference.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 1 Pranith Kumar K 2016-06-09 15:21:24 UTC
Could you give a reference to the function you are referring to where this cleanup is not happening as we intend it to?

Comment 2 Xavi Hernandez 2016-06-09 15:27:19 UTC
In function ec_lock_update_fd() we choose the newest fd to keep it in the lock structure. If there's already an fd assigned, we release it using __fd_unref(). We use this function because we are already inside a region protected bye inode->lock, so we cannot call fd_unref().

The problem is that this function doesn't do any cleanup. It only decrements the ref counter. All cleanup is made in fd_unref().

Comment 3 Vijay Bellur 2016-06-09 15:35:30 UTC
REVIEW: http://review.gluster.org/14683 (cluster/ec: Fix invalid __fd_unref() call) posted (#1) for review on master by Xavier Hernandez (xhernandez)

Comment 4 Vijay Bellur 2016-06-10 07:23:08 UTC
REVIEW: http://review.gluster.org/14683 (cluster/ec: Fix invalid __fd_unref() call) posted (#2) for review on master by Xavier Hernandez (xhernandez)

Comment 5 Vijay Bellur 2016-06-13 12:14:42 UTC
COMMIT: http://review.gluster.org/14683 committed in master by Jeff Darcy (jdarcy) 
------
commit e0b057cf28abc534f1fc74f4ea921c78a52ab6fe
Author: Xavier Hernandez <xhernandez>
Date:   Thu Jun 9 17:29:26 2016 +0200

    cluster/ec: Fix invalid __fd_unref() call
    
    __fd_unref() doesn't do any cleanup, so it cannot be called to release
    fd references, specially if it's the last reference.
    
    The code has been changed to avoid a call to this function.
    
    In the previous version we always tried to keep the newest fd in the
    ec_lock_t structure. However this is not necessary. We'll always keep
    one reference to an open file on the same inode. It's irrelevant if
    the reference is new or old.
    
    The function __fd_unref() has also been removed from fd.h to avoid being
    used in the future since it's useless as it's defined now.
    
    Change-Id: Ia728777fc8e464758d5ea4d3bf020f0603919039
    BUG: 1344396
    Signed-off-by: Xavier Hernandez <xhernandez>
    Reviewed-on: http://review.gluster.org/14683
    NetBSD-regression: NetBSD Build System <jenkins.org>
    Smoke: Gluster Build System <jenkins.com>
    CentOS-regression: Gluster Build System <jenkins.com>
    Reviewed-by: Ashish Pandey <aspandey>
    Reviewed-by: Jeff Darcy <jdarcy>


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