[Bugs] [Bug 1211207] New: Use rcu to protect concurrent access to data structures in GlusterD

bugzilla at redhat.com bugzilla at redhat.com
Mon Apr 13 10:25:43 UTC 2015


https://bugzilla.redhat.com/show_bug.cgi?id=1211207

            Bug ID: 1211207
           Summary: Use rcu to protect concurrent access to data
                    structures in GlusterD
           Product: Red Hat Storage
           Version: 3.0
         Component: glusterfs-server
          Severity: medium
          Assignee: rhs-bugs at redhat.com
          Reporter: sasundar at redhat.com
        QA Contact: sasundar at redhat.com
                CC: bugs at gluster.org, gluster-bugs at redhat.com,
                    kaushal at redhat.com, nlevinki at redhat.com,
                    sasundar at redhat.com, vbellur at redhat.com
        Depends On: 1191030, 1205186



+++ This bug was initially created as a clone of Bug #1191030 +++

GlusterD has issues with concurrent access to data structures, which exist even
with the usage of big-lock. To fix these issues, we will use RCU as the
synchronization protection mechanism and use the userspace-rcu library to help
with the implementation.

More details regarding the existing issues and RCU can be found in the mailing
thread archived at
http://www.gluster.org/pipermail/gluster-devel/2014-December/043382.html

--- Additional comment from Anand Avati on 2015-02-10 05:21:06 EST ---

REVIEW: http://review.gluster.org/9605 (Add liburcu to build system) posted
(#2) for review on master by Kaushal M (kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-10 05:21:09 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#1) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-13 08:42:05 EST ---

REVIEW: http://review.gluster.org/9605 (Add liburcu to build system) posted
(#3) for review on master by Niels de Vos (ndevos at redhat.com)

--- Additional comment from Anand Avati on 2015-02-19 05:09:01 EST ---

REVIEW: http://review.gluster.org/9605 (build, glusterd: Add liburcu to build
system) posted (#4) for review on master by Kaushal M (kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-19 05:09:04 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#2) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-19 05:09:07 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#1) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-20 12:48:46 EST ---

REVIEW: http://review.gluster.org/9605 (build, glusterd: Add liburcu to build
system) posted (#5) for review on master by Kaushal M (kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-20 12:48:49 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#3) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-20 12:48:52 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#2) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-23 07:35:41 EST ---

COMMIT: http://review.gluster.org/9605 committed in master by Krishnan
Parthasarathi (kparthas at redhat.com) 
------
commit 30357bcfcd4a7c92e0e59ff58a26d48c8416c564
Author: Kaushal M <kaushal at redhat.com>
Date:   Tue Jan 6 15:12:59 2015 +0530

    build, glusterd: Add liburcu to build system

    This patch adds liburcu related checks to the build system and updates
    the spec file to require 'userspace-rcu'.

    liburcu >= 0.7 is required to build GlusterFS, but 0.8 and above is
    preferred. For cases when liburcu 0.7.x is the available version, some
    function definitions (currently just one) from liburcu-0.8.6 have been
    made available in /contrib/userspace-rcu/.

    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      a5cd6bd Add userspace-rcu checks to configure.ac
      fe5ced3 Add URCU libs to glusterd libtool flags
      1e43302 Add local definition of cds_list_add_tail_rcu for
              liburcu-0.7
      98da755 Move local definition of cds_list_add_tail_rcu into contrib
      8c44dfd Update spec file to include userspace-rcu0466e33 Rename
      rculist-additional.h to rculist-extra.h
      947c7b3 Add rculist-extra.h to dist
      19f32ad Address review comments 9605/1

    [1]: https://github.com/kshlm/glusterfs/tree/urcu

    Change-Id: Ifbb617d0dacce8fa01214f894badb9d8cdcaf56f
    BUG: 1191030
    Signed-off-by: Kaushal M <kaushal at redhat.com>
    Reviewed-on: http://review.gluster.org/9605
    Tested-by: Gluster Build System <jenkins at build.gluster.com>
    Reviewed-by: Atin Mukherjee <amukherj at redhat.com>
    Reviewed-by: Niels de Vos <ndevos at redhat.com>
    Reviewed-by: Krishnan Parthasarathi <kparthas at redhat.com>
    Tested-by: Krishnan Parthasarathi <kparthas at redhat.com>

--- Additional comment from Anand Avati on 2015-02-24 07:02:51 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#4) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-26 03:54:16 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#5) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-26 03:54:19 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#3) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-02-26 05:01:45 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#6) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Anand Avati on 2015-02-26 05:01:53 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#4) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Anand Avati on 2015-02-26 05:02:04 EST ---

REVIEW: http://review.gluster.org/9755 (glusterd: protect vol_list and volinfo
with RCU) posted (#1) for review on master by Atin Mukherjee
(amukherj at redhat.com)

--- Additional comment from Anand Avati on 2015-03-02 01:45:45 EST ---

REVIEW: http://review.gluster.org/9624 (glusterd: Replace libglusterfs lists
with liburcu lists) posted (#7) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-02 01:45:58 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#5) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-04 02:50:28 EST ---

COMMIT: http://review.gluster.org/9624 committed in master by Krishnan
Parthasarathi (kparthas at redhat.com) 
------
commit 673ba2659cebe22ee30c43f9fb080f330150f55e
Author: Kaushal M <kaushal at redhat.com>
Date:   Tue Jan 6 18:23:41 2015 +0530

    glusterd: Replace libglusterfs lists with liburcu lists

    This patch replaces usage of the libglusterfs lists data structures and
    API in glusterd with the lists data structures and API from liburcu. The
    liburcu data structes and APIs are a drop-in replacement for
    libglusterfs lists.

    All usages have been changed to keep the code consistent, and free from
    confusion.

    NOTE: glusterd_conf_t->xprt_list still uses the libglusterfs data
    structures and API, as it holds rpc_transport_t objects, which is not a
    part of glusterd and is not being changed in this patch.

    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      6dac576 Replace libglusterfs lists with liburcu lists
      a51b5ab Fix compilation issues
      d98a06f Fix merge issues
      a5d918e Remove merge remnant
      1cca113 More style cleanup
      1917be3 Address review comments on 9624/1
      8d10f13 Use cds_lists for glusterd_svc_t
      524ad5d Add rculist header in glusterd-conn-helper.c
      646f294 glusterd: add list_add_order API honouring rcu

    [1]: https://github.com/kshlm/glusterfs/tree/urcu

    Change-Id: Ic613c5b6e496a677b9d3de15fc042a0492109fb0
    BUG: 1191030
    Signed-off-by: Kaushal M <kaushal at redhat.com>
    Signed-off-by: Krishnan Parthasarathi <kparthas at redhat.com>
    Reviewed-on: http://review.gluster.org/9624
    Tested-by: Gluster Build System <jenkins at build.gluster.com>
    Reviewed-by: Atin Mukherjee <amukherj at redhat.com>
    Reviewed-by: Gaurav Kumar Garg <ggarg at redhat.com>
    Reviewed-by: Anand Nekkunti <anekkunt at redhat.com>

--- Additional comment from Anand Avati on 2015-03-05 08:52:15 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#6) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-06 01:04:34 EST ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#7) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Kaushal on 2015-03-09 06:18:18 EDT ---

http://review.gluster.org/9695 implements most of the protection for peerinfos
and the peers list. But it still leaves peerinfo protection during a volume
transaction, especially the transaction peers list, open.

--- Additional comment from Anand Avati on 2015-03-09 07:14:23 EDT ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#8) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-10 05:26:21 EDT ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#9) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Kaushal on 2015-03-10 06:50:38 EDT ---

Need to also address the issue the glusterd_quorum_count macro.

--- Additional comment from Anand Avati on 2015-03-13 06:32:12 EDT ---

REVIEW: http://review.gluster.org/9695 (glusterd: Protect the peer list and
peerinfos with RCU.) posted (#10) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-16 05:19:18 EDT ---

COMMIT: http://review.gluster.org/9695 committed in master by Krishnan
Parthasarathi (kparthas at redhat.com) 
------
commit c7785f78420c94220954eef538ed4698713ebcdb
Author: Kaushal M <kaushal at redhat.com>
Date:   Thu Jan 8 19:24:59 2015 +0530

    glusterd: Protect the peer list and peerinfos with RCU.

    The peer list and the peerinfo objects are now protected using RCU.
    Design patterns described in the Paul McKenney's RCU dissertation [1]
    (sections 5 and 6) have been used to convert existing non-RCU protected
    code to RCU protected code.

    Currently, we are only targetting guaranteeing the existence of the
    peerinfo objects, ie., we are only looking to protect deletes, not all
    updaters. We chose this, as protecting all updates is a much more
    complex task.

    The steps used to accomplish this are,

    1. Remove all long lived direct references to peerinfo objects (apart
    from the peerinfo list). This includes references in glusterd_peerctx_t
    (RPC), glusterd_friend_sm_event_t (friend state machine) and others.
    This way no one has a reference to deleted peerinfo object.

    2. Replace the direct references with indirect references, ie., use
    peer uuid and peer hostname as indirect references to the peerinfo
    object. Any reader or updater now uses the indirect references to get to
    the actual peerinfo object, using glusterd_peerinfo_find. Cases where a
    peerinfo cannot be found are handled gracefully.

    3. The readers get and use the peerinfo object only within a RCU read
    critical section. This prevents the object from being deleted/freed when
    in actual use.

    4. The deletion of a peerinfo object is done in a ordered manner
    (glusterd_peerinfo_destroy). The object is first removed from the
    peerinfo list using an atomic list remove, but the list head is not
    reset to allow existing list readers to complete correctly. We wait for
    readers to complete, before resetting the list head. This removes the
    object from the list completely. After this no new readers can get a
    reference to the object, and it can be freed.

    This change was developed on the git branch at [2]. This commit is a
    combination of the following commits on the development branch.
      d7999b9 Protect the glusterd_conf_t->peers_list with RCU.
      0da85c4 Synchronize before INITing peerinfo list head after removing
              from list.
      32ec28a Add missing rcu_read_unlock
      8fed0b8 Correctly exit read critical section once peer is found.
      63db857 Free peerctx only on rpc destruction
      56eff26 Cleanup style issues
      e5f38b0 Indirection for events and friend_sm
      3c84ac4 In __glusterd_probe_cbk goto unlock only if peer already
              exists
      141d855 Address review comments on 9695/1
      aaeefed Protection during peer updates
      6eda33d Revert "Synchronize before INITing peerinfo list head after
              removing from list."
      f69db96 Remove unneeded line
      b43d2ec Address review comments on 9695/4
      7781921 Address review comments on 9695/5
      eb6467b Add some missing semi-colons
      328a47f Remove synchronize_rcu from
              glusterd_friend_sm_transition_state
      186e429 Run part of glusterd_friend_remove in critical section
      55c0a2e Fix gluster (peer status/ pool list) with no peers
      93f8dcf Use call_rcu to free peerinfo
      c36178c Introduce composite struct, gd_rcu_head

    [1]: http://www.rdrop.com/~paulmck/RCU/RCUdissertation.2004.07.14e1.pdf
    [2]: https://github.com/kshlm/glusterfs/tree/urcu

    Change-Id: Ic1480e59c86d41d25a6a3d159aa3e11fbb3cbc7b
    BUG: 1191030
    Signed-off-by: Kaushal M <kaushal at redhat.com>
    Reviewed-on: http://review.gluster.org/9695
    Tested-by: Gluster Build System <jenkins at build.gluster.com>
    Reviewed-by: Atin Mukherjee <amukherj at redhat.com>
    Reviewed-by: Anand Nekkunti <anekkunt at redhat.com>
    Reviewed-by: Krishnan Parthasarathi <kparthas at redhat.com>
    Tested-by: Krishnan Parthasarathi <kparthas at redhat.com>

--- Additional comment from Anand Avati on 2015-03-18 03:24:15 EDT ---

REVIEW: http://review.gluster.org/9922 (glusterd: Remove compilation warning)
posted (#1) for review on master by Kaushal M (kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-18 05:56:17 EDT ---

COMMIT: http://review.gluster.org/9922 committed in master by Kaushal M
(kaushal at redhat.com) 
------
commit 1250003956bc9fe7f030f0541f1c823cb282a07a
Author: Kaushal M <kaushal at redhat.com>
Date:   Tue Mar 17 12:06:25 2015 +0530

    glusterd: Remove compilation warning

    In glusterd_peerinfo_destroy, cast the passed 'strcut rcu_head *'
    pointer to 'gd_rcu_head *' before use in caa_container_of() to prevent
    the incompatible-pointer compilation warning.

    Also, refactor peerinfo->head to peerinfo->rcu_head to reduce confusion
    when reading code.

    This change was developed on the git branch at [1]. This commit is a
    combination of the following commits on the development branch.
      aa4a0bc Rename peerinfo->head to peerinfo->rcu_head
      c79144b Cast struct rcu_head * to gd_rcu_head * to prevent warning
      1d222c3 More head -> rcu_head renames

    [1]: https://github.com/kshlm/glusterfs/tree/urcu

    BUG: 1191030
    Change-Id: I7ede02090413839563ce44fdf6289697b28777e7
    Signed-off-by: Kaushal M <kaushal at redhat.com>
    Reviewed-on: http://review.gluster.org/9922
    Reviewed-by: Atin Mukherjee <amukherj at redhat.com>
    Tested-by: Gluster Build System <jenkins at build.gluster.com>

--- Additional comment from Anand Avati on 2015-03-24 03:10:45 EDT ---

REVIEW: http://review.gluster.org/9978 (glusterd: Prevent possible deadlock due
to glusterd_quorum_count) posted (#1) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-24 03:30:49 EDT ---

REVIEW: http://review.gluster.org/9978 (glusterd: Prevent possible deadlock due
to glusterd_quorum_count) posted (#2) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Anand Avati on 2015-03-24 03:30:53 EDT ---

REVIEW: http://review.gluster.org/9979 (glusterd: Prevent possible dealock in
glusterd_friend_remove) posted (#1) for review on master by Kaushal M
(kaushal at redhat.com)

--- Additional comment from Kaushal on 2015-03-24 08:26:39 EDT ---

Created a new bug to track the TODOs remaining for GlusterFS-3.7.0 release.
Removing this bug as a blocker on glusterfs-3.7.0.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1191030
[Bug 1191030] Use rcu to protect concurrent access to data structures in
GlusterD
https://bugzilla.redhat.com/show_bug.cgi?id=1205186
[Bug 1205186] RCU changes wrt peers to be done for GlusterFS-3.7.0
-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=lF9TNnuhrz&a=cc_unsubscribe


More information about the Bugs mailing list