On Thu, Jun 5, 2014 at 7:52 PM, Pranith Kumar Karampuri <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.
_______________________________________________ Gluster-devel mailing list Gluster-devel@gluster.org http://supercolony.gluster.org/mailman/listinfo/gluster-devel