features/locks: Avoid race in creating domains

Thread:1                 |       Thread:2
-----------------------------------------
1) Does inodelk on inode1| 1) Does inodelk on inode1
2) It tries to get the   | 2) It tries to get the
dom object for the volume| dom object for the volume
volname-replicate-0. But | volname-replicate-0. But
the domain list is empty.| the domain list is empty.
3) Create the new domain |
dom1 and add it to head  |
of the list of domains.  |
4) inodelk happens on    | 3) Create the new domain
dom1 on range r1.        | dom2 and add it to head
                         | of the list of domains.
                         | 4) inodelk happens on dom2 on range r2
5) unlock for r1 comes   | 5) Unlock on r2 succeeds.
on inode1.               |
6) It tries to get the   |
domain, of the inodelk   |
for volume               |
volname-replicate-0, gets|
dom2 but the lock on     |
range r1 is not present  |
so it fails.             |

subsequent inode lock, unlocks will keep happening on the domain dom2.
The stale lock present in the dom1 on range r1 will live on.
It wont cause any hangs, but the statedump will always be scary.

Change-Id: I9adc120d33febf685b30859cc307768c2fc63ae5
BUG: 770080
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/2993
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>

2 files changed, 16 insertions, 10 deletions

diff --git a/xlators/features/locks/src/common.c b/xlators/features/locks/src/common.c
index 0bc7baa3011..643a7a80db2 100644
--- a/xlators/features/locks/src/common.c
+++ b/xlators/features/locks/src/common.c
@@ -46,7 +46,7 @@ static int
 pl_send_prelock_unlock (xlator_t *this, pl_inode_t *pl_inode,
                         posix_lock_t *old_lock);
 static pl_dom_list_t *
-allocate_domain (const char *volume)
+__allocate_domain (const char *volume)
 {
         pl_dom_list_t *dom = NULL;
 
@@ -88,17 +88,19 @@ get_domain (pl_inode_t *pl_inode, const char *volume)
         GF_VALIDATE_OR_GOTO (POSIX_LOCKS, pl_inode, out);
         GF_VALIDATE_OR_GOTO (POSIX_LOCKS, volume, out);
 
-        list_for_each_entry (dom, &pl_inode->dom_list, inode_list) {
-                if (strcmp (dom->domain, volume) == 0)
-                        goto found;
-
+        pthread_mutex_lock (&pl_inode->mutex);
+        {
+                list_for_each_entry (dom, &pl_inode->dom_list, inode_list) {
+                        if (strcmp (dom->domain, volume) == 0)
+                                goto unlock;
+                }
 
+                dom = __allocate_domain (volume);
+                if (dom)
+                        list_add (&dom->inode_list, &pl_inode->dom_list);
         }
-
-        dom = allocate_domain (volume);
-        if (dom)
-                list_add (&dom->inode_list, &pl_inode->dom_list);
-found:
+unlock:
+        pthread_mutex_unlock (&pl_inode->mutex);
         if (dom) {
                 gf_log (POSIX_LOCKS, GF_LOG_TRACE, "Domain %s found", volume);
         } else {
diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c
index cbc9186aab4..b59f2a280cc 100644
--- a/xlators/features/locks/src/inodelk.c
+++ b/xlators/features/locks/src/inodelk.c
@@ -595,6 +595,10 @@ pl_common_inodelk (call_frame_t *frame, xlator_t *this,
         }
 
         dom = get_domain (pinode, volume);
+        if (!dom) {
+                op_errno = ENOMEM;
+                goto unwind;
+        }
 
         if (frame->root->lk_owner.len == 0) {
                 /*