Bug 1344396

Summary:	fd leak in disperse
Product:	[Community] GlusterFS	Reporter:	Xavi Hernandez <jahernan>
Component:	disperse	Assignee:	Xavi Hernandez <jahernan>
Status:	CLOSED CURRENTRELEASE	QA Contact:
Severity:	medium	Docs Contact:
Priority:	unspecified
Version:	mainline	CC:	aspandey, bugs, jahernan, otira.fuu, pkarampu
Target Milestone:	---	Keywords:	Triaged
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	glusterfs-3.9.0	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1344421 1344422 1344425 (view as bug list)		Environment:
Last Closed:	2016-11-23 07:19:02 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1344421, 1344422, 1344425

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>