On 06/06/2014 10:02 AM, Anand Avati wrote:
On Thu, Jun 5, 2014 at 7:52 PM, Pranith Kumar Karampuri
<pkara...@redhat.com <mailto:pkara...@redhat.com>> wrote:
This sounds a bit complicated. I think there is a much simpler
solution:
- First, make update_refkeeper() check for blocked locks (which I
mentioned as "optional" previously)
- Make grant_blocked_locks() double up and do the job of
update_refkeeper() internally.
Something which looks like this:
diff --git a/xlators/features/locks/src/common.c
b/xlators/features/locks/src/common.c
index f6c71c1..38df385 100644
--- a/xlators/features/locks/src/common.c
+++ b/xlators/features/locks/src/common.c
@@ -126,8 +126,14 @@ __pl_inode_is_empty (pl_inode_t *pl_inode)
if (!list_empty (&dom->entrylk_list))
is_empty = 0;
+ if (!list_empty (&dom->blocked_entrylks))
+ is_empty = 0;
+
if (!list_empty (&dom->inodelk_list))
is_empty = 0;
+
+ if (!list_empty (&dom->blocked_inodelks))
+ is_empty = 0;
}
return is_empty;
@@ -944,12 +950,18 @@ grant_blocked_locks (xlator_t *this,
pl_inode_t *pl_inode)
struct list_head granted_list;
posix_lock_t *tmp = NULL;
posix_lock_t *lock = NULL;
+ inode_t *unref = NULL;
INIT_LIST_HEAD (&granted_list);
pthread_mutex_lock (&pl_inode->mutex);
{
__grant_blocked_locks (this, pl_inode, &granted_list);
+
+ if (__pl_inode_is_empty (pl_inode) &&
pl_inode->refkeeper) {
+ unref = pl_inode->refkeeper;
+ pl_inode->refkeeper = NULL;
+ }
}
pthread_mutex_unlock (&pl_inode->mutex);
@@ -965,6 +977,9 @@ grant_blocked_locks (xlator_t *this,
pl_inode_t *pl_inode)
GF_FREE (lock);
}
+ if (unref)
+ inode_unref (unref);
+
return;
}
This should make pl_disconnect_cbk() pretty much race free w.r.t
refkpeer. Thoughts?
Lets say C1 is doing pl_inodelk_client_cleanup. After the second
for-loop(All granted and blocked locks are out of the domain) if
an unlock on the final granted lock on that inode from client C2
completes, refkeeper would be set to NULL and unrefed leading to
zero refs on that inode i.e. pl_forget will also happen. In 3rd
for-loop pl_inode is already freed and leads to free'd memory
access and will crash.
We also need:
diff --git a/xlators/features/locks/src/inodelk.c
b/xlators/features/locks/src/inodelk.c
index c76cb7f..2aceb8a 100644
--- a/xlators/features/locks/src/inodelk.c
+++ b/xlators/features/locks/src/inodelk.c
@@ -494,13 +494,13 @@ pl_inodelk_client_cleanup (xlator_t *this, pl_ctx_t *ctx)
dom = get_domain (pl_inode, l->volume);
- grant_blocked_inode_locks (this, pl_inode, dom);
-
pthread_mutex_lock (&pl_inode->mutex);
{
__pl_inodelk_unref (l);
}
pthread_mutex_unlock (&pl_inode->mutex);
+
+ grant_blocked_inode_locks (this, pl_inode, dom);
}
return 0;
Missed this in the last patch.
It still doesn't solve the problem I described earlier. By the time it
executes this third loop refkeeper is already unreffed when C2 unlocks.
Pranith
_______________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
