Bug 1434274 - BZ for some bugs found while going through synctask code
Summary: BZ for some bugs found while going through synctask code
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: core
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ravishankar N
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1437312 1437313
TreeView+ depends on / blocked
 
Reported: 2017-03-21 07:09 UTC by Ravishankar N
Modified: 2017-05-30 18:48 UTC (History)
1 user (show)

Fixed In Version: glusterfs-3.11.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1437312 1437313 (view as bug list)
Environment:
Last Closed: 2017-05-30 18:48:21 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description Ravishankar N 2017-03-21 07:09:39 UTC
Bugs listed in the patch description.

Comment 1 Worker Ant 2017-03-21 07:13:18 UTC
REVIEW: https://review.gluster.org/16929 (libglusterfs: remove call to sigemptyset) posted (#1) for review on master by Ravishankar N (ravishankar)

Comment 2 Worker Ant 2017-03-21 07:13:21 UTC
REVIEW: https://review.gluster.org/16930 (syncop: fix argc count in call to makecontext()) posted (#1) for review on master by Ravishankar N (ravishankar)

Comment 3 Worker Ant 2017-03-21 07:13:24 UTC
REVIEW: https://review.gluster.org/16931 (syncop:  don't wake task in synctask_wake unless really needed) posted (#1) for review on master by Ravishankar N (ravishankar)

Comment 4 Worker Ant 2017-03-21 10:47:03 UTC
REVIEW: https://review.gluster.org/16929 (libglusterfs: remove call to sigemptyset) posted (#2) for review on master by Ravishankar N (ravishankar)

Comment 5 Worker Ant 2017-03-21 10:48:30 UTC
REVIEW: https://review.gluster.org/16929 (libglusterfs: remove call to sigemptyset) posted (#3) for review on master by Ravishankar N (ravishankar)

Comment 6 Worker Ant 2017-03-21 11:01:49 UTC
REVIEW: https://review.gluster.org/16929 (libglusterfs: Initialize old sigset) posted (#4) for review on master by Ravishankar N (ravishankar)

Comment 7 Worker Ant 2017-03-21 11:31:34 UTC
REVIEW: https://review.gluster.org/16931 (syncop:  don't wake task in synctask_wake unless really needed) posted (#2) for review on master by Ravishankar N (ravishankar)

Comment 8 Worker Ant 2017-03-21 13:18:26 UTC
COMMIT: https://review.gluster.org/16930 committed in master by Jeff Darcy (jeff.us) 
------
commit 2f560dbc78360f0e7fa76a2dfdabd9c92d13e976
Author: Ravishankar N <ravishankar>
Date:   Wed Mar 15 17:18:54 2017 +0530

    syncop: fix argc count in call to makecontext()
    
    We are only passing one argument (a pointer to struct synctask) to the
    function, so argc must be 1 and not 2.
    
    Change-Id: I4eaadd58a76f32327d8bb3efa9c5c435700d7391
    BUG: 1434274
    Signed-off-by: Ravishankar N <ravishankar>
    Reviewed-on: https://review.gluster.org/16930
    Smoke: Gluster Build System <jenkins.org>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Jeff Darcy <jeff.us>

Comment 9 Worker Ant 2017-03-24 07:07:06 UTC
REVIEW: https://review.gluster.org/16931 (syncop:  don't wake task in synctask_wake unless really needed) posted (#3) for review on master by Ravishankar N (ravishankar)

Comment 10 Worker Ant 2017-03-27 03:40:47 UTC
REVIEW: https://review.gluster.org/16951 (syncop: Fix args for makecontext) posted (#1) for review on master by Pranith Kumar Karampuri (pkarampu)

Comment 11 Worker Ant 2017-03-27 05:08:59 UTC
COMMIT: https://review.gluster.org/16929 committed in master by Pranith Kumar Karampuri (pkarampu) 
------
commit 6c14e55377cc0080da88c5b997f5f7ea2374393b
Author: Ravishankar N <ravishankar>
Date:   Wed Mar 15 17:10:41 2017 +0530

    libglusterfs: Initialize old sigset
    
    ...in gf_thread_create(). Technically, since we pass it as an argument
    to pthread_sigmask, initialization is not needed but doing it as a good
    practice.
    
    Change-Id: Ie069af07cb07c1784f3841e1fc628ca13dfdcef4
    BUG: 1434274
    Signed-off-by: Ravishankar N <ravishankar>
    Reviewed-on: https://review.gluster.org/16929
    Smoke: Gluster Build System <jenkins.org>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Ashish Pandey <aspandey>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu>

Comment 12 Worker Ant 2017-03-27 10:48:11 UTC
REVIEW: https://review.gluster.org/16931 (syncop:  don't wake task in synctask_wake unless really needed) posted (#4) for review on master by Ravishankar N (ravishankar)

Comment 13 Worker Ant 2017-03-27 11:27:57 UTC
REVIEW: https://review.gluster.org/16931 (syncop:  don't wake task in synctask_wake unless really needed) posted (#5) for review on master by Ravishankar N (ravishankar)

Comment 14 Worker Ant 2017-03-28 05:55:44 UTC
COMMIT: https://review.gluster.org/16951 committed in master by Pranith Kumar Karampuri (pkarampu) 
------
commit df189a8644d7a805bb3e278f61983c5ba8619188
Author: Pranith Kumar K <pkarampu>
Date:   Mon Mar 27 07:19:57 2017 +0530

    syncop: Fix args for makecontext
    
    Passing 64bit arguments to makecontext is not portable and manpage says the following:
    
    <snip>
    On  architectures where int and pointer types are the same size (e.g., x86-32, where
    both types are 32 bits), you may be able to get away with passing pointers as  argu‐
    ments  to makecontext() following argc.  However, doing this is not guaranteed to be
    portable, is undefined according to the standards, and won't work  on  architectures
    where pointers are larger than ints.  Nevertheless, starting with version 2.8, glibc
    makes some changes to makecontext(), to permit this  on  some  64-bit  architectures
    (e.g., x86-64).
    </snip>
    
    Since we do not depend on the arguments, it is better to change makecontext to not
    take any arguments.
    
    BUG: 1434274
    Change-Id: Ic46c9e9faaeb2f78e4efde353ef861466515b1ec
    Signed-off-by: Pranith Kumar K <pkarampu>
    Reviewed-on: https://review.gluster.org/16951
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: Ravishankar N <ravishankar>
    CentOS-regression: Gluster Build System <jenkins.org>
    NetBSD-regression: NetBSD Build System <jenkins.org>

Comment 15 Worker Ant 2017-03-28 22:34:51 UTC
COMMIT: https://review.gluster.org/16931 committed in master by Pranith Kumar Karampuri (pkarampu) 
------
commit 0f98f5c8070904810252c6fc1df23747afa4b1d7
Author: Ravishankar N <ravishankar>
Date:   Tue Mar 21 11:02:32 2017 +0530

    syncop:  don't wake task in synctask_wake unless really needed
    
    Problem:
    
    In EC and AFR, we launch synctasks during self-heal.
    
    (i) These tasks usually stackwind a FOP to all its children and call
    synctask_yield() which does a swapcontext to synctask_switchto() and puts the
    task in syncenv's waitq by calling __wait(task). This happends as long as the
    FOP ckbs from all children haven't been received.
    
    (ii) For each FOP cbk, we call synctask_wake() which again does a swapcontext
    to synctask_switchto() which now puts the task in syncenv's runq by calling
    __run(task). When the task runs and the conext switches back to the FOP path,
    it puts the task in waitq because we haven't heard from all children as
    explained in (i).
    
    Thus we are unnecessarily using the swapcontext syscalls to just toggle
    the task back and forth between the waitq and runq.
    
    Fix:
    Store the stackwind count in new variable 'syncbarrier->waitfor' before
    winding the fop. In each cbk when we call synctask_wake(),  perform an actual
    wake only if the cbk count == stackwind count.
    
    Change-Id: Id62d3b6ffed5a8c50f8b79267fb34e9470ba5ed5
    BUG: 1434274
    Signed-off-by: Ravishankar N <ravishankar>
    Signed-off-by: Ashish Pandey <aspandey>
    Reviewed-on: https://review.gluster.org/16931
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: Niels de Vos <ndevos>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Pranith Kumar Karampuri <pkarampu>

Comment 16 Shyamsundar 2017-05-30 18:48:21 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.11.0, please open a new bug report.

glusterfs-3.11.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2017-May/000073.html
[2] https://www.gluster.org/pipermail/gluster-users/


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